Skip to content

upgrade redb file format to v3 #7917

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 14 commits into
base: unstable
Choose a base branch
from
Open

upgrade redb file format to v3 #7917

wants to merge 14 commits into from

Conversation

owanikin
Copy link

Issue Addressed

#7911

Proposed Changes

  • Add migration v29 for Redb file-format upgrade.
  • Introduce migration_schema_v29.rs with:
    • Upgrade path gated under the redb feature.
    • Explicit irreversible downgrade (returns error).
  • Register migration in schema_change.rs.

eserilev
eserilev reviewed Aug 21, 2025
View reviewed changes
beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs Outdated
pub fn downgrade_from_v29<T: BeaconChainTypes>(
_d: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
) -> Result<Vec<KeyValueStoreOp>, Error> {
Err(Error::MigrationError("Cannot downgrade from v29: Redb file format upgrade is irreversible".to_string()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the downgrade should only fail if redb is enabled and the database backend is redb

In all other cases it should just be a no-op

@eserilev
Copy link
Member

Just wanted to document here that redb v2.6.2 creates the db using the v3 format by default

https://docs.rs/redb/2.6.2/src/redb/db.rs.html#1202-1220

So once we push this change, new redb databases will be using the correct v3 format as well

@eserilev eserilev marked this pull request as ready for review September 8, 2025 21:44
eserilev
eserilev requested changes Sep 8, 2025
View reviewed changes
Copy link
Member

@eserilev eserilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sticking through with this! This looks really good, mostly just have some things to remove

beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs Outdated
pub fn upgrade_to_v29<T: BeaconChainTypes>(
db: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
) -> Result<Vec<KeyValueStoreOp>, Error> {
if db.is_redb() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was nice for testing, but we no longer need this, you can just call upgrade

beacon_node/store/src/database/interface.rs Outdated
}
#[cfg(feature = "redb")]
BeaconNodeBackend::Redb(redb) => {
info!("Running Redb v29 migration");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update to this log to "Updating Redb file format to v3"

beacon_node/store/src/database/interface.rs Outdated
match self {
#[cfg(feature = "leveldb")]
BeaconNodeBackend::LevelDb(_) => {
info!("LevelDB upgrade: no migration needed");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update this log to "LevelDB, no upgrade required" and downgrade this log to debug

beacon_node/store/src/hot_cold_store.rs Outdated
Comment on lines 273 to 277
#[cfg(feature = "redb")]
let _database_backend = DatabaseBackend::Redb;
#[cfg(feature = "leveldb")]
let database_backend = DatabaseBackend::LevelDb;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, this must be something I pushed up, we can remove this

beacon_node/store/src/hot_cold_store.rs Outdated
@@ -82,6 +84,8 @@ pub struct HotColdDB<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> {
historic_state_cache: Mutex<HistoricStateCache<E>>,
/// Chain spec.
pub spec: Arc<ChainSpec>,
/// Database backend type
pub database_backend: Option<DatabaseBackend>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this as well

beacon_node/store/src/hot_cold_store.rs Outdated
@@ -1,3 +1,5 @@
#[cfg(feature = "redb")]
use crate::config::DatabaseBackend;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed

beacon_node/store/src/hot_cold_store.rs Outdated
@@ -242,6 +246,7 @@ impl<E: EthSpec> HotColdDB<E, MemoryStore<E>, MemoryStore<E>> {
config,
hierarchy,
spec,
database_backend: None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed

beacon_node/store/src/hot_cold_store.rs Outdated
@@ -294,6 +304,7 @@ impl<E: EthSpec> HotColdDB<E, BeaconNodeBackend<E>, BeaconNodeBackend<E>> {
config,
hierarchy,
spec,
database_backend: Some(database_backend),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove as well

lighthouse/Cargo.toml Outdated
@@ -12,7 +12,7 @@ rust-version = "1.88.0"
normal = ["target_check"]

[features]
default = ["slasher-lmdb", "beacon-node-leveldb"]
default = ["slasher-lmdb", "beacon-node-redb"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set default back to leveldb (sorry I think I pushed up this change)

eserilev
eserilev reviewed Sep 9, 2025
View reviewed changes
beacon_node/store/src/database/interface.rs
#[cfg(feature = "redb")]
BeaconNodeBackend::Redb(redb) => {
info!("Updating Redb file format to v3");
redb.upgrade()?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note thta upgrade returns true is successful and false if its already in the v3 format

https://docs.rs/redb/2.6.2/redb/struct.Database.html#method.upgrade

In all other failure cases it will raise an error

eserilev
eserilev approved these changes Sep 9, 2025
View reviewed changes
Copy link
Member

@eserilev eserilev left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@michaelsproul whenever you have a chance, would be great to get your review as well. Thanks!

@eserilev eserilev added the ready-for-review The code is ready for review label Sep 9, 2025
@mergify Mergify
Copy link

mergify bot commented Sep 9, 2025

Some required checks have failed. Could you please take a look @owanikin? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 9, 2025
@eserilev
Copy link
Member

eserilev commented Sep 9, 2025

@owanikin we have a small ci failure, can you run cargo sort --workspace and push those changes up when you get the chance

@owanikin owanikin mentioned this pull request Sep 15, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eserilev eserilev eserilev approved these changes

@michaelsproul michaelsproul Awaiting requested review from michaelsproul michaelsproul is a code owner

Assignees
No one assigned
Labels
database waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@owanikin @eserilev @jimmygchen