feat(flatkv): add read-only LoadVersion for state sync#3039
feat(flatkv): add read-only LoadVersion for state sync#3039blindchaser merged 9 commits intomainfrom
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c72f10db2d
ℹ️ 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 cs.evmCommitter != nil { | ||
| evmStore, err := cs.evmCommitter.LoadVersion(targetVersion, true) | ||
| if err != nil { |
There was a problem hiding this comment.
Avoid requiring FlatKV for every read-only LoadVersion call
In the read-only branch, LoadVersion now always opens FlatKV when evmCommitter is configured, and returns an error if that load fails; this makes all historical scStore.LoadVersion(version, true) callers fail (including non-EVM queries) whenever FlatKV cannot serve that height (for example, due to different retention/pruning or local FlatKV issues), even though CompositeCommitStore.GetChildStoreByName still serves reads from Cosmos only. This turns previously successful historical reads into hard failures in mixed-backend deployments.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3039 +/- ##
=======================================
Coverage 58.37% 58.37%
=======================================
Files 2080 2080
Lines 171944 171892 -52
=======================================
- Hits 100366 100350 -16
+ Misses 62636 62603 -33
+ Partials 8942 8939 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| if cs.evmCommitter != nil { | ||
| evmStore, err := cs.evmCommitter.LoadVersion(targetVersion, true) | ||
| if err != nil { | ||
| _ = cosmosCommitter.Close() |
There was a problem hiding this comment.
Would it be worth logging if this error is non-nil?
sei-db/state_db/sc/flatkv/store.go
Outdated
|
|
||
| defer func() { | ||
| if retErr != nil { | ||
| _ = ro.Close() |
There was a problem hiding this comment.
Perhaps log if this error is non-nil
| phaseTimer *metrics.PhaseTimer | ||
|
|
||
| readOnly bool // Set by readonly LoadVersion; guards all write methods. | ||
| readOnlyWorkDir string // Temp working dir for readonly store; removed by Close. |
There was a problem hiding this comment.
What if there are multiple goroutines all opening for the same height, but only one got closed? Will it also remove on close? What about other opened DB
There was a problem hiding this comment.
Each loadVersionReadOnly instance creates a unique temporary directory via os.MkdirTemp, complete isolation between multiple instances. Closing a instance only deletes its own directory.
| } | ||
|
|
||
| if s.readOnlyWorkDir != "" { | ||
| _ = os.RemoveAll(s.readOnlyWorkDir) |
There was a problem hiding this comment.
os.MkdirTemp does not automatically delete the directory.
The directory will remain on disk until something explicitly removes it. if a read-only store is not properly closed (e.g., process crash), these readonly-* directories will linger forever.
There's no cleanup for orphaned readonly-* dirs on startup.
There was a problem hiding this comment.
this is a good point
…eadonly-* working directories left by a previous process crash
Looks like this PR added them back in by accident: * #3039
Looks like this PR added them back in by accident: * #3039
This commit was inadvertently reverted by the squash-merge of #3039. Restores the tx ordering check (firstCosmosSeen), len(evmEntries)>0 and len(v2Entries)>0 guards in ProcessTXsWithOCCGiga. Conflicts with #3050's slog migration resolved by using package-level logger instead of ctx.Logger(). Made-with: Cursor
This commit was inadvertently reverted by the squash-merge of #3039. Restores ReceiptStoreConfig in app.toml, readReceiptStoreConfig(), BackendTypeName(), receipt config tests, and init_test.go. Conflicts resolved: - app_config.go: kept both ReceiptStore (from #3035) and Admin (from #3062) - receipt_store.go: kept BackendTypeName but used #3050's slog convention - parquet_store_test.go: kept #3053's deterministic pruning test fix Made-with: Cursor
This commit was inadvertently reverted by the squash-merge of #3039. Restores LatticeHashEnabled config option in StateCommitConfig, composite store lattice hash support, and related tests. Conflict resolved: store_test.go needed all three imports (metrics from HEAD, evm+logger from #3043). Made-with: Cursor
This commit was inadvertently reverted by the squash-merge of #3039. Restores the benchmark config changes (BlocksPerCommit=1, SnapshotInterval=1000, SnapshotMinTimeInterval=60). The consoleLogger addition from #3046 is dropped because #3050 replaced the logger package with slog, making it unnecessary. Made-with: Cursor
## Summary The squash-merge of #3039 (`feat(flatkv): add read-only LoadVersion for state sync`) inadvertently reverted changes from 5 previously-merged PRs. The `yiren/flatkv-readonly` branch had accumulated stale versions of files through merge-from-main commits, and when the final squash-merge landed, those stale versions overwrote the newer code on `main`. ### PRs reverted by #3039 and restored in this PR | PR | Title | Key changes lost | |---|---|---| | **#2810** | fix(giga): check whether txs follow Giga ordering | `firstCosmosSeen` tx ordering check, `len(evmEntries) > 0` / `len(v2Entries) > 0` guards in `ProcessTXsWithOCCGiga` | | **#3035** | Add receiptdb config option in app.toml | `ReceiptStoreConfig` in app.toml, `readReceiptStoreConfig()`, `BackendTypeName()`, receipt config tests | | **#3043** | Add config to enable lattice hash | `EnableLatticeHash` config, composite store lattice hash support | | **#3021** | Background Transaction Generation | `block.go`, `block_builder.go` for cryptosim benchmark | | **#3046** | Add console logger and fix memiavl config for benchmark | `BlocksPerCommit=1`, `SnapshotInterval=1000`, `SnapshotMinTimeInterval=60` | ### Conflict resolutions Since several PRs landed after #3039 (notably #3050 slog migration, #3053 flaky test fix, #3062 admin service), cherry-picks required manual conflict resolution: - **`app/app.go`** (#2810): Kept #2810's structural changes, used `logger.Error` (from #3050) instead of `ctx.Logger().Error` - **`cmd/seid/cmd/app_config.go`** (#3035): Kept both `ReceiptStore` (from #3035) and `Admin` (from #3062) - **`sei-db/ledger_db/receipt/receipt_store.go`** (#3035): Restored `BackendTypeName` but dropped logger param (superseded by #3050 slog) - **`sei-db/ledger_db/receipt/parquet_store_test.go`** (#3035): Kept #3053's deterministic pruning test fix - **`sei-db/state_db/sc/composite/store_test.go`** (#3043): Merged all three needed imports - **`sei-db/common/logger/logger.go`** (#3046): Kept deletion from #3050; `consoleLogger` is no longer needed with slog - **`sei-db/state_db/bench/wrappers/db_implementations.go`** (#3046): Kept #3050's no-logger-param API --------- Signed-off-by: Cody Littley <cody.littley@seinetwork.io> Co-authored-by: Cody Littley <56973212+cody-littley@users.noreply.github.com> Co-authored-by: Cody Littley <cody.littley@seinetwork.io>
* main: fix(giga): match v2 correctness checks (#3071) Added clone method to canned random (#3076) Helper files for the flatKV cache implementation (#3072) fix: restore PRs inadvertently reverted by #3039 squash-merge (#3070) Refine logging to avoid printing expensive objects on hot path (#3066) Fix flaky tendermint syncer test (#3065) Add runtime log level control via gRPC admin service (#3062) chore: dcoument run RPC suite on legacy vs giga (#3041) chore: self-contained revert tests, contract reorg, and failure analysis (#3033)
Cherry-picked from main PR #3039 (6 non-merge commits). - LoadVersion now accepts a readOnly flag. When true, it creates an isolated CommitStore in a temporary working directory under flatkv/, replays the WAL to the requested version, then closes the WAL so the returned store has no coupling to the parent. This enables opening any historical height for import/export without affecting the live writable store, which is required for state sync over the network. - Write methods (ApplyChangeSets, Commit, WriteSnapshot, Rollback, Importer) return errReadOnly on readonly stores. Close removes the temporary directory. - CompositeCommitStore.LoadVersion soft-fails FlatKV for readonly loads so Cosmos queries still work when FlatKV is unavailable at that height. - CleanupCrashArtifacts removes orphaned readonly-* directories left by a previous process crash, called at startup in rootmulti.NewStore. Made-with: Cursor
Describe your changes and provide context
LoadVersion now accepts a readOnly flag. When true, it creates an isolated CommitStore in a temporary working directory under flatkv/, replays the WAL to the requested version, then closes the WAL so the returned store has no coupling to the parent. This enables opening any historical height for import/export without affecting the live writable store, which is required for state sync over the network.
Write methods (ApplyChangeSets, Commit, WriteSnapshot, Rollback, Importer) return errReadOnly on readonly stores. Close removes the temporary directory.
Testing performed to validate your change