Allow database migration upon adding new database column family to schema #1851

simrankedia · 2024-04-21T23:55:02Z

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.

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests - Tests introduced by Added a test for the case of adding new columns into the existing RocksDB database #1853

Before requesting review

I have reviewed the code myself

CLAassistant · 2024-04-21T23:55:07Z

All committers have signed the CLA.

xgreenx · 2024-04-22T08:47:02Z

Answering your comment in the issue:

@Voxelot I have tried to raise a PR to fix this issue #1851 - please let me know if the direction is correct - I will accordingly complete and release a new PR with tests etc.

You are welcome to contribute, and yeah, it is the right direction.

I created a test case #1853 to prove that the fix works. Could you rebase your PR to use this branch with the test case, please?

crates/fuel-core/src/state/rocks_db.rs

xgreenx

Could you fix CI please?=) You can run locally ./ci_checks.sh to perform all CI checks

simrankedia · 2024-04-22T23:21:49Z

Could you fix CI please?=) You can run locally ./ci_checks.sh to perform all CI checks

@xgreenx I am seeing the following issue upon running the checks, which seems unrelated.

xgreenx · 2024-04-23T12:19:19Z

The change fails most of tests=)

xgreenx · 2024-04-23T12:19:39Z

I am seeing the following issue upon running the checks, which seems unrelated.

I would suggest you to use rust 1.75=)

simrankedia · 2024-04-23T23:22:34Z

The change fails most of tests=)

Yes, I will look into it and get back. Didn't get a chance today.

…hema

simrankedia · 2024-04-25T01:16:14Z

request as draft April 24, 2024 14:14

I have fixed the original issue and the failing tests have been fixed. However, upon further running the checks I see some 5 more tests failing:

    poa::p2p::test_poa_multiple_producers
    sync::test_multiple_producers_different_keys
    sync::test_partitions_larger_groups::partition_with_100_txs_8_validators_4_partitions
    sync::test_partitions_larger_groups::partition_with_10_txs_8_validators_4_partitions
    sync::test_partitions_larger_groups::partition_with_1_tx_3_validators_3_partitions

These are not directly related to my changes atleast - I will investigate more tomorrow. Let me know if you have any idea till then. Thanks

simrankedia · 2024-04-25T09:17:48Z

request as draft April 24, 2024 14:14

I have fixed the original issue and the failing tests have been fixed. However, upon further running the checks I see some 5 more tests failing:
    poa::p2p::test_poa_multiple_producers
    sync::test_multiple_producers_different_keys
    sync::test_partitions_larger_groups::partition_with_100_txs_8_validators_4_partitions
    sync::test_partitions_larger_groups::partition_with_10_txs_8_validators_4_partitions
    sync::test_partitions_larger_groups::partition_with_1_tx_3_validators_3_partitions
These are not directly related to my changes atleast - I will investigate more tomorrow. Let me know if you have any idea till then. Thanks

@xgreenx Slightly by increasing this time_limit (https://github.com/FuelLabs/fuel-core/blob/master/crates/fuel-core/src/p2p_test_helpers.rs#L383) all test passes - I am running an experiment to understand what kind of limit to use - but wanted to check if there are any specific constraints that needs to be followed here?

xgreenx · 2024-04-25T11:01:11Z

crates/fuel-core/src/state/rocks_db.rs

+                for (name, opt) in cf_descriptors_to_create {
+                    db.create_cf(name, &opt)
+                        .map_err(|e| DatabaseError::Other(e.into()))?;
+                }
+                Ok(db)


Maybe it makes sense to move this loop after db is opened(below of the match statement). In this case you also will handle the case of the error with repair.

xgreenx · 2024-04-25T11:01:26Z

crates/fuel-core/src/state/rocks_db.rs

+                        Self::cf_opts(i, &block_opts),
+                    )
+                });
+                DB::open_cf_descriptors(&opts, &path, cf_descriptors)


Suggested change

DB::open_cf_descriptors(&opts, &path, cf_descriptors)

DB::open_cf_descriptors(&opts, &path, cf_descriptors_to_open)

@MitchTurner

## 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

simrankedia mentioned this pull request Apr 21, 2024

Make RocksDB more upgradeable #162

Closed

xgreenx marked this pull request as draft April 22, 2024 09:24

simrankedia changed the base branch from master to feature/support-new-tables-rocksdb April 22, 2024 22:05

simrankedia marked this pull request as ready for review April 22, 2024 22:13

xgreenx reviewed Apr 22, 2024

View reviewed changes

crates/fuel-core/src/state/rocks_db.rs Outdated Show resolved Hide resolved

crates/fuel-core/src/state/rocks_db.rs Outdated Show resolved Hide resolved

crates/fuel-core/src/state/rocks_db.rs Outdated Show resolved Hide resolved

xgreenx reviewed Apr 22, 2024

View reviewed changes

simrankedia force-pushed the fix_rocksdb branch from 967710b to 5b4f560 Compare April 22, 2024 22:56

simrankedia force-pushed the fix_rocksdb branch 3 times, most recently from 6334d6a to da71786 Compare April 22, 2024 23:46

simrankedia changed the title ~~Allow database migration upon adding new database column family to sc…~~ Allow database migration upon adding new database column family to schema Apr 22, 2024

simrankedia force-pushed the fix_rocksdb branch from da71786 to 66e0071 Compare April 23, 2024 00:03

xgreenx marked this pull request as draft April 24, 2024 13:14

simrankedia force-pushed the fix_rocksdb branch from 66e0071 to aa7535d Compare April 25, 2024 00:50

simrankedia added 2 commits April 25, 2024 01:57

Allow database migration upon adding new database column family to sc…

6d41092

…hema

Updated CHANGELOG.md

ccf6511

simrankedia force-pushed the fix_rocksdb branch from aa7535d to ccf6511 Compare April 25, 2024 01:02

simrankedia marked this pull request as ready for review April 25, 2024 09:13

Merge branch 'feature/support-new-tables-rocksdb' into fix_rocksdb

a74a4e6

xgreenx reviewed Apr 25, 2024

View reviewed changes

xgreenx merged commit 4165449 into FuelLabs:feature/support-new-tables-rocksdb Apr 29, 2024
30 of 33 checks passed

xgreenx mentioned this pull request Apr 30, 2024

Release v0.26.0 #1874

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow database migration upon adding new database column family to schema #1851

Allow database migration upon adding new database column family to schema #1851

simrankedia commented Apr 21, 2024 •

edited by xgreenx

CLAassistant commented Apr 21, 2024 •

edited

xgreenx commented Apr 22, 2024

xgreenx left a comment •

edited

simrankedia commented Apr 22, 2024

xgreenx commented Apr 23, 2024

xgreenx commented Apr 23, 2024

simrankedia commented Apr 23, 2024

simrankedia commented Apr 25, 2024

simrankedia commented Apr 25, 2024

xgreenx Apr 25, 2024

xgreenx Apr 25, 2024

	DB::open_cf_descriptors(&opts, &path, cf_descriptors)
	DB::open_cf_descriptors(&opts, &path, cf_descriptors_to_open)

Allow database migration upon adding new database column family to schema #1851

Allow database migration upon adding new database column family to schema #1851

Conversation

simrankedia commented Apr 21, 2024 • edited by xgreenx

Before requesting review

CLAassistant commented Apr 21, 2024 • edited

xgreenx commented Apr 22, 2024

xgreenx left a comment • edited

Choose a reason for hiding this comment

simrankedia commented Apr 22, 2024

xgreenx commented Apr 23, 2024

xgreenx commented Apr 23, 2024

simrankedia commented Apr 23, 2024

simrankedia commented Apr 25, 2024

simrankedia commented Apr 25, 2024

xgreenx Apr 25, 2024

Choose a reason for hiding this comment

xgreenx Apr 25, 2024

Choose a reason for hiding this comment

simrankedia commented Apr 21, 2024 •

edited by xgreenx

CLAassistant commented Apr 21, 2024 •

edited

xgreenx left a comment •

edited