flatkv migration testings test#3462
Conversation
PR SummaryHigh Risk Overview Refactors Expands migration and recovery coverage: adds comprehensive Go tests for MemiavlOnly -> Reviewed by Cursor Bugbot for commit b3b6cc2. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
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 #3462 +/- ##
========================================
Coverage 59.04% 59.05%
========================================
Files 2188 2189 +1
Lines 182088 182455 +367
========================================
+ Hits 107517 107750 +233
- Misses 64933 65040 +107
- Partials 9638 9665 +27
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75ffaf1428
ℹ️ 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".
| HistoricalProofBurst int `mapstructure:"historical-proof-burst"` | ||
|
|
||
| // The number of keys to migrate from memiavl to flatkv per block. Ignored if not in a migration mode. | ||
| KeysToMigratePerBlock int `mapstructure:"keys-to-migrate-per-block"` |
There was a problem hiding this comment.
Wire the migration batch size into app config
Adding this field does not make it configurable at runtime: the generated app.toml template has no matching key and app/seidb.go only reads the existing FlagSC* options, while the new migration script writes keys-to-migrate-per-block expecting it to take effect. As a result, migrate_* runs configured from app.toml keep using the hard-coded default (and server/config.GetConfig leaves the field at zero in its literal), so operators/tests cannot throttle batches and the 0->1 test silently skips the intended small-batch resume coverage. Please add the TOML key/flag and parse it into StateCommitConfig.
Useful? React with 👍 / 👎.
75ffaf1 to
bc103a8
Compare
6370d22 to
ad6ce4a
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ad6ce4a. Configure here.
49fa7a3 to
2a5e537
Compare
2a5e537 to
2ec63e0
Compare
2364ea4 to
152df79
Compare
152df79 to
abbf2ec
Compare

Describe your changes and provide context
Testing performed to validate your change