ledger: generic kv trackers backend implementation #5488
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5488 +/- ##
==========================================
- Coverage 55.41% 54.87% -0.54%
==========================================
Files 446 460 +14
Lines 63289 64231 +942
==========================================
+ Hits 35069 35245 +176
- Misses 25826 26610 +784
+ Partials 2394 2376 -18
... and 36 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
algorandskiy
changed the title
| return ret | ||
| } | ||
|
|
||
| // resourceKey: 4-byte prefix + 32-byte address + 8-byte big-endian uint64 |
There was a problem hiding this comment.
these 32 bytes per resource looks as a huge waste
| } | ||
|
|
||
| // TODO: lifted from sql.go, we might want to refactor it | ||
| func keyPrefixIntervalPreprocessing(prefix []byte) ([]byte, []byte) { |
There was a problem hiding this comment.
it has to be a shared function, not a copy
| } | ||
|
|
||
| // RunMigrations runs the migrations on the store up to the target version. | ||
| func RunMigrations(ctx context.Context, db dbForMigrations, params trackerdb.Params, targetVersion int32) (mgr trackerdb.InitParams, err error) { |
There was a problem hiding this comment.
consider rewriting by using utils/db's migration/init code - see and staterproof as an example
| require.Equal(t, trackerdb.ErrNotFound, err) | ||
| } | ||
|
|
||
| func CustomTestOnlineAccountsDelete(t *customT) { |
There was a problem hiding this comment.
there is a bigger test TestOnlineAccountsDeletion and I do think the new implementation have to at least pass it but ideally extend it to 3+ accounts and more rounds.
There was a problem hiding this comment.
Looks good but there are couple primary concerns:
- new
OnlineAccountsDeleteneeds more test scenarios. The original SQL code was written for continuous address sequence like AAAABBBCCCC and not ABCABAACC. - keys creation has unneeded allocation overhead, need to fix.
|
Going to merge after the 3.16.3 cut and have follow up ticket for tasks. |
Summary
We are breaking down #5212 in several smaller PRs to make it more digestible.
This is PRs "numero 3" on the KV epic.
Content:
Test Plan
Adding cross-backend db testing.