Use pipelined execution in CheckpointExecutor #21538
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
mystenmark
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
| /// A collection of watches for each stage. These are the synchronization points | ||
| /// for the pipeline. | ||
| pub(super) struct PipelineStages { | ||
| stages: [SequenceWatch; PipelineStage::End as usize], |
There was a problem hiding this comment.
possibly not worth mentioning, but since you're already using strum, would it make sense to use strum::EnumCount here?
There was a problem hiding this comment.
hmm, but then it would be COUNT - 1 which seems possibly more confusing?
| @@ -25,6 +26,9 @@ pub struct CheckpointExecutorMetrics { | |||
| // TODO: delete once users are migrated to non-Mysten histogram. | |||
| pub last_executed_checkpoint_age_ms: MystenHistogram, | |||
| pub checkpoint_executor_validator_path: IntGauge, | |||
|
|
|||
| pub stage_wait_duration_ns: IntCounterVec, | |||
There was a problem hiding this comment.
Would it make more sense for these to be histograms?
There was a problem hiding this comment.
I'm not sure that would useful? We can always add them later
| @@ -917,7 +917,7 @@ impl ExpensiveSafetyCheckConfig { | |||
| } | |||
|
|
|||
| fn default_checkpoint_execution_max_concurrency() -> usize { | |||
| 40 | |||
| 4 | |||
There was a problem hiding this comment.
What's the reason to lower this?
There was a problem hiding this comment.
in the new system, you cannot have more concurrency than there are stages, since only one thread can be in each stage at a time. So the max possible value would be 8 or so. But in practice almost all the time is consumed by execution, building db batch, and committing batch. So 3 + 1 more core to handle all the other small stages seems sensible.
if we eventually need more throughput we will probably have to shard the object writes.
There was a problem hiding this comment.
hmm does that mean there can be at most 8 transaction executing at the same time?
There was a problem hiding this comment.
no - this is checkpoint parallelism not tx parallelism. the ExecuteTransaction stage schedules all transactions in the checkpoint concurrently.
mystenmark
force-pushed
the
mlogan-ckpt-opt-pr-1
branch
from
58bc6f9 to
84b393c
Compare
mystenmark
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
mystenmark
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
This allows us to have parallelism throughout the entire CheckpointExecution process, rather than just during the transaction-execution phase.
Additionally, by imposing stricter-than-necessary ordering constraints on the execution phase (i.e. that we enqueue all transactions from checkpoint N before enqueueing those from N+1) we achieve lower latency.
Throughput should also be improved here, by running the batch-building and batch-writing steps in different stages of the pipeline, which allows us to start building the batch for seq N+1 while we are still committing the batch for seq N.