-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(storage): save enum indices in RocksDB #162
feat(storage): save enum indices in RocksDB #162
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #162 +/- ##
==========================================
+ Coverage 31.77% 31.87% +0.10%
==========================================
Files 481 481
Lines 25593 25688 +95
==========================================
+ Hits 8132 8189 +57
- Misses 17461 17499 +38
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, this functionality should be tested. One approach that comes to mind is syncing RocksdbStorage
from Postgres (you can test enum indices at this point) and then manually removing enum indices for all keys in the state CF (by directly using RocksDB
operations), so that they can be synced again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice test! 👍 Other than some nits, looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than minor nits, looks good. Please update the PR description; it describes an outdated implementation.
…ocksdb-migration-for-enum-indices
🤖 I have created a release *beep* *boop* --- ## [16.0.0](core-v15.1.1...core-v16.0.0) (2023-10-11) ### ⚠ BREAKING CHANGES * **vm:** Update Refund model ([#181](#181)) ### Features * change chainId to u64 ([#167](#167)) ([f14bf68](f14bf68)) * **merkle tree:** Provide Merkle proofs for tree entries and entry ranges ([#119](#119)) ([1e30d0b](1e30d0b)) * **storage:** save enum indices in RocksDB ([#162](#162)) ([bab099d](bab099d)) * **vm:** Update Refund model ([#181](#181)) ([92b6f59](92b6f59)) ### Bug Fixes * **db:** drop constraint prover_jobs_fri_l1_batch_number_fkey ([#173](#173)) ([fa71650](fa71650)) * **vm:** Make execution status and stop reason public ([#169](#169)) ([f98c4fa](f98c4fa)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
What ❔
Enumeration indices now are saved along with values in the same column family. Indices are added gradually for old DB entries. The number of keys processed each L1 batch is configurable.
Why ❔
Enumeration indices in storage are necessary for boojum upgrade.
Checklist
zk fmt
andzk lint
.