Re-execution of pending certs must happen concurrently with consensus handling, since there may be dependencies in either direction. #21000
Conversation
mystenmark
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
| @@ -436,7 +436,7 @@ impl CheckpointExecutor { | |||
|
|
|||
| /// Post processing and plumbing after we executed a checkpoint. This function is guaranteed | |||
| /// to be called in the order of checkpoint sequence number. | |||
| #[instrument(level = "debug", skip_all)] | |||
| #[instrument(level = "info", skip_all, fields(seq = ?checkpoint.sequence_number()))] | |||
There was a problem hiding this comment.
did you mean to merge this "info" level bump?
There was a problem hiding this comment.
yes, once-per-checkpoint spans are good candidates for info level imo
| "state-sync should have ensured that transaction with digest {:?} exists for checkpoint: {checkpoint:?}", | ||
| digests.transaction, | ||
| "state-sync should have ensured that transaction with digests {:?} exists for checkpoint: {checkpoint:?}", | ||
| digests |
There was a problem hiding this comment.
if you like - inline "digests" into the format str?
| components.consensus_adapter.submit_recovered(&epoch_store); | ||
|
|
||
| // Start the gRPC server |
There was a problem hiding this comment.
this seems subtle and not sure I get it, why does this now have to get pulled out and started earlier here?
There was a problem hiding this comment.
its actually getting started later - we need to wait until all previously-executed txns have been re-executed before accepting user requests, so that we don't appear to have forgotten about a transaction
crates/sui-node/src/lib.rs
Outdated
| @@ -2042,6 +2051,30 @@ impl SuiNode { | |||
| } | |||
| } | |||
|
|
|||
| enum ValidatorGrpcServerHandle { | |||
There was a problem hiding this comment.
this enum seems to have nothing (internally) to do with being a validator gRPC server? is this just to make starting idempotent? I am confused by its name being so much more specific than what it appears to do
There was a problem hiding this comment.
Fair... but i'm not sure what else to call it? its not really a general purpose thing?
There was a problem hiding this comment.
maybe OnceStarter or SpawnOnce?
or maybe you can use this instead?
https://docs.rs/once_cell/latest/once_cell/sync/struct.Lazy.html
There was a problem hiding this comment.
I like SpawnOnce
… handling, since there may be dependencies in either direction.
mystenmark
force-pushed
the
mlogan-rexecution-fix
branch
from
8a8dc4d to
02fb8de
Compare
mystenmark
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
No description provided.