feat: support using badgerDB as a repository for data regulation suppressions #2619
Conversation
atzoum
force-pushed
the
feat.badgerSuppressions
branch
from
2f3d556 to
3ff091e
Compare
atzoum
force-pushed
the
feat.badgerSuppressions
branch
3 times, most recently
from
37e514a to
ef2c282
Compare
Codecov ReportBase: 45.55% // Head: 45.87% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2619 +/- ##
==========================================
+ Coverage 45.55% 45.87% +0.31%
==========================================
Files 290 295 +5
Lines 48066 48499 +433
==========================================
+ Hits 21895 22247 +352
- Misses 24781 24852 +71
- Partials 1390 1400 +10
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
atzoum
force-pushed
the
feat.badgerSuppressions
branch
11 times, most recently
from
66c8cc7 to
cbbd856
Compare
|
SonarCloud Quality Gate failed.
|
atzoum
force-pushed
the
feat.badgerSuppressions
branch
3 times, most recently
from
d5bd120 to
aa5c179
Compare
atzoum
changed the title
| path, | ||
| log, | ||
| WithSeederSource(seederSource), | ||
| WithMaxSeedWait(config.GetDuration("BackendConfig.Regulations.maxSeedWait", 5, time.Second))) |
There was a problem hiding this comment.
Explain: if restore doesn't complete in 5 seconds, the gateway container will startup. Only after restore completes will the gateway be able to successfully perform suppressions, in the meantime, no suppressions will be performed.
atzoum
force-pushed
the
feat.badgerSuppressions
branch
3 times, most recently
from
bce1865 to
d42344d
Compare
atzoum
force-pushed
the
feat.badgerSuppressions
branch
2 times, most recently
from
bcfe3f7 to
283f588
Compare
atzoum
force-pushed
the
feat.badgerSuppressions
branch
from
283f588 to
b5f683a
Compare
| Backup(w io.Writer) error | ||
|
|
||
| // Restore restores the repository from the given reader | ||
| Restore(r io.Reader) error |
There was a problem hiding this comment.
Can you please explain why do we need to expose Backup & Restore func. Given that Restore happens implicitly when we call NewRepository.
And, for Backup I am assuming that it will be implemented in next PR. So, I am not sure of the implementation that you are going to have. But, still with an assumption that it will be called implicitly from Stop() function. Since, we might want to take a backup when stoping or maybe periodically using some go routine. In either case, I feel it will be implicit inside the badgerdb package itself. So, here also won't it make sense to not expose it?
In short, if we associate NewRepository() with Restore() & Stop() with Backup(), then won't need to expose backup & restore.
There was a problem hiding this comment.
Backup & Restore are both used now by tests
There was a problem hiding this comment.
yes, I saw that, but isn't it a bad practice to export something just to test? Instead, won't it be better to have tests in the same package for these internal function. WDYT?
There was a problem hiding this comment.
Backup & Restore are not something you wouldn't expect from a repository & we are going to use them in the future in non-testing code too.
| log: log, | ||
| path: path.Join(basePath, "badgerdbv3"), | ||
| maxGoroutines: 1, | ||
| maxSeedWait: 10 * time.Second, |
There was a problem hiding this comment.
Since, we have WithMaxSeedWait as Opt. Should we check for it, before setting a default value?
There was a problem hiding this comment.
this is the default value, opts are applied later in line 70, i.e. can override the default values.
atzoum
force-pushed
the
feat.badgerSuppressions
branch
from
b5f683a to
e3d78dd
Compare
atzoum
force-pushed
the
feat.badgerSuppressions
branch
6 times, most recently
from
b999439 to
94b7b37
Compare
atzoum
force-pushed
the
feat.badgerSuppressions
branch
from
94b7b37 to
49cb34b
Compare
Description
It is already possible to have millions of data regulation suppressions for a given workspace/namespace, thus keeping them in-memory is an non-viable solution. For that purpose, we are introducing support for using badgerDB as a backing repository to store and query suppressions at the gateway component.
Other improvements
suppressionpackage, so that some of its modules can be reused for other purposes:suppression.Repository: stores and queries suppressions, we support both an in-memory and a badgerdb-backed repositorysuppression.Syncer: polls the data regulation service and syncs suppressions to thesuppression.Repositorysuppression.handler: implementstypes.UserSuppressionenterprise feature and uses asuppression.Repositorysuppression.Factory: creates and glues all the modules above according to the environment's configurationBenchmarks
As expected, the in-memory repository can achieve 5x faster write times and 5.5x faster read times.
Nevertheless, badgerdb's performance is satisfactory, as it exhibits
500K wpsand500Mil rps.go test -run=^$ -bench ^BenchmarkAddAndSuppress$ github.com/rudderlabs/rudder-server/enterprise/suppress-user -v -count=1 goos: darwin goarch: arm64 pkg: github.com/rudderlabs/rudder-server/enterprise/suppress-user BenchmarkAddAndSuppress BenchmarkAddAndSuppress/badger BenchmarkAddAndSuppress/badger-10 1 42729787333 ns/op 518757 suppressions/s(add) 445789123 suppressions/s(read) BenchmarkAddAndSuppress/memory BenchmarkAddAndSuppress/memory-10 1 12530184667 ns/op 2858211 suppressions/s(add) 2564874328 suppressions/s(read) PASS ok github.com/rudderlabs/rudder-server/enterprise/suppress-user 56.053sFurther Work
To speed up suppressions' sync time in gateway containers, we are investigating a solution which involves a periodic backup of the badgerdb repo, so that any new gateway container can startup in a hot state after performing a short restore of its badgerdb data.
Notion Ticket
Link
Security