At startup, wait until all previously-observed commits have been sent to the consensus handler. #19344
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
| ) { | ||
| let highest_handled_commit = self.highest_handled_commit(); | ||
| assert!( | ||
| highest_observed_commit_at_startup >= highest_handled_commit, |
There was a problem hiding this comment.
This may not be the case if we want to revert and recreate latest consensus commits, but somehow doesn't need to revert state on the Sui side. But it seems unlikely.
There was a problem hiding this comment.
Yeah I'm confused how that would be possible, but we can always fix the assert later if that comes up
| if highest_handled >= highest_observed_commit_at_startup { | ||
| return; | ||
| } | ||
| rx.changed().await.unwrap(); |
There was a problem hiding this comment.
Maybe exit the loop instead of panic in case of shutdown order issues?
There was a problem hiding this comment.
I think hitting the unwrap is impossible - this method has a &self reference, so the watch::Sender cannot possibly be dropped while we are in this method.
mystenmark
force-pushed
the
mlogan-consensus-info
branch
from
03d2e68
to
ae772d9
Compare
mystenmark
force-pushed
the
mlogan-consensus-info
branch
from
ae772d9
to
5b6475a
Compare
|
|
||
| assert!( | ||
| *commit == 0, | ||
| "highest_known_commit_at_startup can only be set once" |
There was a problem hiding this comment.
s/highest_known_commit_at_startup/highest_observed_commit_at_startup
| @@ -203,6 +212,9 @@ where | |||
| let store_path = context.parameters.db_path.as_path().to_str().unwrap(); | |||
| let store = Arc::new(RocksDBStore::new(store_path)); | |||
| let dag_state = Arc::new(RwLock::new(DagState::new(context.clone(), store.clone()))); | |||
|
|
|||
| let highest_known_commit_at_startup = dag_state.read().last_commit_index(); | |||
There was a problem hiding this comment.
nit, optional: highest_observed_commit, and maybe set it closer to its usage?
… to the consensus handler.
mystenmark
force-pushed
the
mlogan-consensus-info
branch
from
5b6475a
to
da62f19
Compare
… to the consensus handler. (#19344) This is necessary for proper crash recovery with data quarantining, as we will need to wait until all previously-built checkpoints have been rebuilt before starting CheckpointExecutor
This is necessary for proper crash recovery with data quarantining, as we will need to wait until all previously-built checkpoints have been rebuilt before starting CheckpointExecutor