feat(flatkv): adds per-DB LtHash tracking to the FlatKV commit store#3074
feat(flatkv): adds per-DB LtHash tracking to the FlatKV commit store#3074blindchaser wants to merge 4 commits intomainfrom
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3074 +/- ##
==========================================
+ Coverage 58.42% 58.44% +0.02%
==========================================
Files 2088 2088
Lines 172102 172149 +47
==========================================
+ Hits 100547 100609 +62
+ Misses 62618 62592 -26
- Partials 8937 8948 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58610cf3c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if errorutils.IsNotFound(err) { | ||
| s.perDBCommittedLtHash[dbDir] = lthash.New() | ||
| s.perDBWorkingLtHash[dbDir] = lthash.New() | ||
| continue |
There was a problem hiding this comment.
loadPerDBLtHashes treats missing _meta/hash/* keys as a fresh database and seeds each per-DB hash with zero, but this also happens when opening an existing pre-migration store where global metadata and data already exist. In that upgrade path, the next non-empty ApplyChangeSets computes deltas from zero baselines and then rebuilds workingLtHash from those per-DB hashes, causing the committed global hash to diverge from actual on-disk state. Missing per-DB keys need a migration/backfill path (or guarded error) when the store is not truly empty.
Useful? React with 👍 / 👎.
blindchaser
changed the title
| func (s *CommitStore) loadPerDBLtHashes() error { | ||
| for dbDir, metaKey := range perDBLtHashKeys { | ||
| data, err := s.metadataDB.Get([]byte(metaKey)) | ||
| if errorutils.IsNotFound(err) { |
There was a problem hiding this comment.
Would it make sense to log a warning level log if we don't find the hash? Something like this:
No lattice hash found for %s DB, initializing to fresh hash.
That way, if hashes diverge because a hash is missing from the metadataDB then the cause of failure is more obvious.
|
|
||
| imp.store.committedVersion = imp.version | ||
| imp.store.committedLtHash = imp.store.workingLtHash.Clone() | ||
| imp.store.snapshotLtHashes() |
There was a problem hiding this comment.
- We don't need the individual perDB committed LT hash
- We only need a single committedLtHash, so we don't need this function
There was a problem hiding this comment.
agreed, removed snapshotLtHashes(). per-db lthashes are now persisted inside each db's LocalMeta atomically via commitBatches(), no separate snapshot step needed.
sei-db/state_db/sc/flatkv/store.go
Outdated
| // Metadata DB keys | ||
| MetaGlobalVersion = "_meta/version" // Global committed version watermark (8 bytes) | ||
| MetaGlobalLtHash = "_meta/hash" // Global LtHash (2048 bytes) | ||
| MetaAccountLtHash = "_meta/hash/account" |
There was a problem hiding this comment.
Question, shall we store these per DB hash + version into its corresponding DB ? And metadata only store the GlobalHash?
There was a problem hiding this comment.
per-db lthash + version are now stored in each db's own LocalMeta
blindchaser
force-pushed
the
yiren/lthash-per-db
branch
from
970933a to
b6c9641
Compare
3 participants
Summary
Adds per-DB LtHash tracking to each of the four FlatKV data databases (account, code, storage, legacy) alongside the existing global LtHash. The global LtHash is now derived from per-DB hashes via the homomorphic property (sum of per-DB = global), eliminating an independent computation path and making the invariant structural.
Persistence model: authoritative per-DB hashes live in
metadataDB(written atomically with the global hash incommitGlobalMetadata); secondary copies are embedded in each DB'sLocalMetafor per-DB integrity verification.keys.go: ExtendsLocalMetawith an optional*lthash.LtHashfield.MarshalLocalMeta/UnmarshalLocalMetaaccept both 8-byte (version-only, old format) and 2056-byte (version + LtHash) formats. Rejects any other length.store.go: AddsperDBCommittedLtHashandperDBWorkingLtHashmaps toCommitStore.loadGlobalMetadatanow callsloadPerDBLtHashesto read per-DB hashes frommetadataDB; missing keys initialize to zero (fresh start).store_meta.go:commitGlobalMetadataatomically writes per-DB hashes in the same batch as global version and global hash.snapshotLtHashesclones all working hashes (global + per-DB) to committed state.loadPerDBLtHashesreads per-DB keys frommetadataDB, initializing to zero on not-found.store_write.go:ApplyChangeSetscomputes per-DB hashes independently using the already-separated pair slices, then derives the global hash viaReset()+MixIn().commitBatchesembeds the per-DB hash into each DB'sLocalMeta. Uses a fixed-size[4]dbPairsarray to avoid per-block map allocation.store_catchup.go: WAL replay snapshots per-DB hashes viasnapshotLtHashes()after each replayed version.importer.go: Snapshot import usessnapshotLtHashes()to persist per-DB committed state.Test plan
perdb_lthash_test.go(11 tests):LocalMetato version V-1, reopens, verifies catchup produces correct per-DB hashes via full scan.Importer, verifies per-DB hashes match full scan and committed == working.metadataDBafter commit, verifies they match in-memory committed hashes.ApplyChangeSetswith 20 pairs, verifies per-DB hashes match full scan.keys_test.go:LocalMetaround-trip serialization with embedded LtHash.LtHash == nilfor old 8-byte format.