-
Notifications
You must be signed in to change notification settings - Fork 313
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
upgrades: failing to release lock on storage #4430
Labels
Comments
Found one missing: diff --git a/crates/bin/pd/src/migrate/testnet76.rs b/crates/bin/pd/src/migrate/testnet76.rs
index b02b2b559..5c8e6ea15 100644
--- a/crates/bin/pd/src/migrate/testnet76.rs
+++ b/crates/bin/pd/src/migrate/testnet76.rs
@@ -121,6 +121,7 @@ pub async fn migrate(
let rocksdb_dir = pd_home.join("rocksdb");
let storage = Storage::load(rocksdb_dir, SUBSTORE_PREFIXES.to_vec()).await?;
let migrated_state = storage.latest_snapshot();
+ storage.release().await;
// The migration is complete, now we need to generate a genesis file. To do this, we need
// to lookup a validator view from the chain, and specify the post-upgrade app hash and Checking for more... |
diff --git a/crates/bin/pd/src/migrate/reset_halt_bit.rs b/crates/bin/pd/src/migrate/reset_halt_bit.rs
index b9877378c..4e0060592 100644
--- a/crates/bin/pd/src/migrate/reset_halt_bit.rs
+++ b/crates/bin/pd/src/migrate/reset_halt_bit.rs
@@ -14,6 +14,7 @@ pub async fn migrate(
let mut delta = StateDelta::new(export_state);
delta.ready_to_start();
let _ = storage.commit_in_place(delta).await?;
+ storage.release().await;
tracing::info!("migration completed: halt bit is turned off, chain is ready to start");
Ok(())
} |
1 task
I had removed it, it must have slipped back in during a rebase of #4137. I will push a pr. |
Those two aren't sufficient to resolve, so I'm still digging around. Happy to pair or review, I've got a slick testing setup to validate. |
conorsch
added a commit
that referenced
this issue
May 21, 2024
conorsch
added a commit
that referenced
this issue
May 21, 2024
1 task
conorsch
pushed a commit
that referenced
this issue
May 22, 2024
conorsch
pushed a commit
that referenced
this issue
May 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
On the latest round of migration-testing for Testnet 76 (#4402), I've started seeing:
This looks like #4344, so it's likely a recent merge forgot to drop a storage handle.
The text was updated successfully, but these errors were encountered: