-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Added a test for the case of adding new columns into the existing RocksDB database #1853
Conversation
…hema (#1851) Adds migration capabilities (allows adding new column family to existing database) to RocksDB instance. This change makes sure RocksDB open is always requested with the existing set of column families and all new column families are later created once opened. - [x] Breaking changes are clearly marked as such in the PR description and changelog - [x] New behavior is reflected in tests - Tests introduced by #1853 ### Before requesting review - [x] I have reviewed the code myself --------- Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
vec![Column::Coins, Column::Messages, Column::UploadedBytecodes]; | ||
let database_with_old_columns = | ||
RocksDb::<OnChain>::open(tmp_dir.path(), old_columns.clone(), None) | ||
.expect("Failed to open database with old columns"); |
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.
Nit: I'm okay with using unwrap
in tests, if you'd prefer expect
, it's more standard to explain the reason it should be Ok
, rather than just telling the user it failed:
https://doc.rust-lang.org/std/result/enum.Result.html#recommended-message-style
DB::open_cf_descriptors(&opts, &path, cf_descriptors) | ||
} | ||
} | ||
let existing_column_families = DB::list_cf(&opts, &path).unwrap_or_else(|err| { |
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.
This is a pretty big wall of imperative code. Could you break it down into a series of declarative helper functions?
## Version v0.26.0 ### Fixed #### Breaking - [#1868](#1868): Include the `event_inbox_root` in the header hash. Changed types of the `transactions_count` to `u16` and `message_receipt_count` to `u32` instead of `u64`. Updated the application hash root calculation to not pad numbers. - [#1866](#1866): Fixed a runtime panic that occurred when restarting a node. The panic happens when the relayer database is already populated, and the relayer attempts an empty commit during start up. This invalid commit is removed in this PR. - [#1871](#1871): Fixed `block` endpoint to return fetch the blocks from both databases after regenesis. - [#1856](#1856): Replaced instances of `Union` with `Enum` for GraphQL definitions of `ConsensusParametersVersion` and related types. This is needed because `Union` does not support multiple `Version`s inside discriminants or empty variants. - [#1870](#1870): Fixed benchmarks for the `0.25.3`. - [#1870](#1870): Improves the performance of getting the size of the contract from the `InMemoryTransaction`. - [#1851](#1851): Provided migration capabilities (enabled addition of new column families) to RocksDB instance. ### Added - [#1853](#1853): Added a test case to verify the database's behavior when new columns are added to the RocksDB database. - [#1860](#1860): Regenesis now preserves `FuelBlockIdsToHeights` off-chain table. ### Changed - [#1847](#1847): Simplify the validation interface to use `Block`. Remove `Validation` variant of `ExecutionKind`. - [#1832](#1832): Snapshot generation can be cancelled. Progress is also reported. - [#1837](#1837): Refactor the executor and separate validation from the other use cases ## What's Changed * Weekly `cargo update` by @github-actions in #1850 * Refactor/separate validation from other executions by @MitchTurner in #1837 * fix: Use `Enum` for `ConsensusParametersVersion` and related types by @bvrooman in #1856 * feat: snapshot generation graceful shutdown by @segfault-magnet in #1832 * regenesis: migrate FuelBlockIdsToHeights by @Dentosal in #1860 * Weekly `cargo update` by @github-actions in #1869 * Refactor/Simplify validation logic by @MitchTurner in #1847 * Fixed `block` endpoint to return fetch the blocks from both databases after regenesis by @xgreenx in #1871 * Add Eq and Partial Eq to tx response and status by @MujkicA in #1872 * test: restart with relayer data by @bvrooman in #1866 * Fix `BlockHeader` hash by @MitchTurner in #1868 * Added a test for the case of adding new columns into the existing RocksDB database by @xgreenx in #1853 * Fixed benchmarks for the `0.25.3` by @xgreenx in #1870 **Full Changelog**: v0.25.3...v0.26.0
Closes #162
Added a test case to verify the database's behavior when new columns are added to the RocksDB database.
Before requesting review