sov-db: Track DB commit status #2272
Conversation
bkolad
force-pushed
the
blaze/fix_db_commit
branch
from
1662beb to
d804845
Compare
bkolad
force-pushed
the
blaze/fix_db_commit
branch
3 times, most recently
from
0ea1d6e to
811a9ba
Compare
bkolad
force-pushed
the
blaze/fix_db_commit
branch
from
811a9ba to
600ba4a
Compare
|
|
||
| pub fn log_reset_instruction(&self) { | ||
| tracing::error!( | ||
| "To reset commit flag, please remove commit flag file: `rm {}`", |
There was a problem hiding this comment.
This was useful if user knows that it just wants to remove flag, they can just copy command and execute it. Why we removed it?
There was a problem hiding this comment.
Why would we ever want to reset the flag manually? The flow has changed and now we save the file before committing the nomt.
| debug_assert_eq!(self.commit_flag.read_status()?, in_progress_commit_status); | ||
| commit_flag.save_commit_status(&CommitStatus::CommittingKernelNomt( | ||
| self.kernel.root().into_inner(), | ||
| ))?; |
There was a problem hiding this comment.
What happens if NOMT has commited successfully, but writing flag has failed?
There was a problem hiding this comment.
We set the flag before committing to Nomt, so if saving the flag fails, we won’t attempt the Nomt commit
| tracing::warn!( | ||
| "Detected in-progress commit {commit_status:?}. Rolling back kernel & user DBs." | ||
| ); | ||
| // User & Kernel commit was sucefull but we don't see `Success`. We rollback both User & Kernek. |
There was a problem hiding this comment.
| // User & Kernel commit was sucefull but we don't see `Success`. We rollback both User & Kernek. | |
| // User & Kernel commit was successful but we don't see `Success`. We rollback both User & Kernel. |
citizen-stig
left a comment
citizen-stig
left a comment
There was a problem hiding this comment.
Overall is good, left couple of comments
| let flat_state = FlatStateDb::new(path, state_cache_size, separate_archival_state)?; | ||
|
|
||
| // After validation override the commit flag to Success. | ||
| commit_flag.write_status(&CommitStatus::Success)?; |
There was a problem hiding this comment.
What happens if we commited live-db successfully, but failed to write commit status Success ?
There was a problem hiding this comment.
We discussed this with Preston when you ware on holiday:
- We will rollback both the
userandkernelso they remain in a consistent state. - The flat-db and ledger will temporarily be ahead of Nomt,
- On startup, the rollup will start from the latest height committed to nomt. After processing the first block, we will override the flat-db/ledger
bkolad
force-pushed
the
blaze/fix_db_commit
branch
2 times, most recently
from
1c158f2 to
af72874
Compare
| ?commit_status, | ||
| "Detected in-progress commit. Rolling back kernel & user DBs." | ||
| ); | ||
| // User & Kernel commit was sucefull but we don't see `Success`. We rollback both User & Kernel. |
There was a problem hiding this comment.
| // User & Kernel commit was sucefull but we don't see `Success`. We rollback both User & Kernel. | |
| // User & Kernel commit was successful but we don't see `Success`. We rollback both User & Kernel. |
| self.user.rollback(1)?; | ||
| } | ||
| CommitStatus::Success => {} | ||
| } |
There was a problem hiding this comment.
Do we need to update flag to success at the end, so it is consistent on the next restart?
There was a problem hiding this comment.
Meaning if it accidentally shut down immediately before starting new block
There was a problem hiding this comment.
Added the save in DbGroup::new
| Ok(Self { user, kernel }) | ||
| } | ||
|
|
||
| pub(crate) fn validate_commit_flag_and_rollback_if_necssesary( |
There was a problem hiding this comment.
| pub(crate) fn validate_commit_flag_and_rollback_if_necssesary( | |
| pub(crate) fn validate_commit_flag_and_rollback_if_necessesary( |
Description
This PR adds tracking for DB commit status.
Previously, the
CommitFlagmechanism saved theCommitStatusto a file, but it had two major limitations:The
CommitStatusis now written to a file in theDbGroup::commitmethod, and recovery is performed inNomtStateDb::validate_commit_flag_and_rollback_if_necssesary.Unit tests will be included in a follow-up PR.
I have updatedCHANGELOG.mdwith a new entry if my PR makes any breaking changes or fixes a bug. If my PR removes a feature or changes its behavior, I provide help for users on how to migrate to the new behavior.Cargo.tomlchanges before opening the PRs. (Are all new dependencies necessary? Is any module dependency leaked into the full-node (hint: it shouldn't)?)