[consensus] only wait to replay until highest consumer processed commit #22854

mwtian · 2025-07-23T17:09:37Z

Description

Currently, CommitConsumerMonitor::replay_complete() waits until all local commits found during initialization gets processed through handle_consensus_commit(). But a suffix of local commits may not have been finalized before. These commits can only be finalized with new commits, after consensus starts running. So the waiter can wait for a long time.

This change makes the waiter only wait until the last commit index handle_consensus_commit() finished processing, which is guaranteed to be finalized in local DAG. Then the waiter should not wait for too long on restart.

Also, record finalized commit refs and their rejected transactions. They will be used during recovery to identify the commits which have enough blocks in local DAG to finalize them. In future, they will be used to verify rejected transactions are computed correctly during recovery.

Test plan

CI

TRANSACTION_DRIVER=50 python ./scripts/simtest/seed-search.py test_simulated_load_reconfig_with_crashes_and_delays --test simtest --num-seeds 500

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

vercel · 2025-07-23T17:09:42Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sui-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 25, 2025 5:26am

2 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
multisig-toolkit	⬜️ Ignored (Inspect)	Visit Preview		Jul 25, 2025 5:26am
sui-kiosk	⬜️ Ignored (Inspect)	Visit Preview		Jul 25, 2025 5:26am

consensus/core/src/commit_consumer.rs

consensus/core/src/authority_node.rs

mwtian · 2025-07-25T01:58:29Z

consensus/core/src/commit_observer.rs

+                if committed_sub_dag.commit_ref.index
+                    > commit_consumer.consumer_last_processed_commit_index
+                {
+                    committed_sub_dag.local_dag_has_finalization_blocks = false;


This is needed to mitigate incorrectly finalize transactions in recovered commits.

I am not sure how this fixes the issue? We only persist finalized commits to store so recovering finalized commits should be able to be reprocessed correctly. What information is missing to be able to properly finalize transactions in recovered commits?

sui/consensus/core/src/commit_finalizer.rs

Lines 118 to 124 in 6fe223c

if !finalized_commits.is_empty() {

// Commits and committed blocks must be persisted to storage before sending them to Sui

// to execute their finalized transactions.

// Commit metadata and uncommitted blocks can be persisted more lazily because they are recoverable.

// But for simplicity, all unpersisted commits and blocks are flushed to storage.

self.dag_state.write().flush();

}

Chatted offline. Today for simplicity both finalized and non-finalized commits are persisted. But a suffix of commits may not have been finalized, and they don't have enough info in local dag to optimistically finalize transactions.

arun-koshy

Approving to unblock fixing the original issue of reducing delay in rebuilding checkpoints which looks good, but I still have an open question on how this fixes incorrect finalization

consensus/core/src/commit_consumer.rs

arun-koshy · 2025-07-25T02:33:41Z

consensus/core/src/commit_observer.rs

+                if committed_sub_dag.commit_ref.index
+                    > commit_consumer.consumer_last_processed_commit_index
+                {
+                    committed_sub_dag.local_dag_has_finalization_blocks = false;


I am not sure how this fixes the issue? We only persist finalized commits to store so recovering finalized commits should be able to be reprocessed correctly. What information is missing to be able to properly finalize transactions in recovered commits?

sui/consensus/core/src/commit_finalizer.rs

Lines 118 to 124 in 6fe223c

if !finalized_commits.is_empty() {

// Commits and committed blocks must be persisted to storage before sending them to Sui

// to execute their finalized transactions.

// Commit metadata and uncommitted blocks can be persisted more lazily because they are recoverable.

// But for simplicity, all unpersisted commits and blocks are flushed to storage.

self.dag_state.write().flush();

}

…ndex

akichidis

💯

akichidis · 2025-07-25T10:34:09Z

consensus/core/src/commit.rs

+    /// and is false when the commit is received through commit sync.
+    /// When this is false, CommitFinalizer will not assume there are enough blocks
+    /// to optimistically finalize transactions in the commit.
+    pub local_dag_has_finalization_blocks: bool,


nit: maybe this could be something more directly with how this sub dag has been committed? Ex:

committed_with_local_certificate

commit_certificate_exists_locally

leader_certificate_exists_locally

Good ideas.

akichidis · 2025-07-25T12:53:50Z

consensus/core/src/dag_state.rs

    // & last_committed_rounds from the commits as needed.
    commit_info_to_write: Vec<(CommitRef, CommitInfo)>,

+    // Buffers finalized commits and their rejected transactions to be written to storage.
+    finalized_commits_to_write: Vec<(CommitRef, BTreeMap<BlockRef, Vec<TransactionIndex>>)>,


Do we intend to use somewhere the rejected transaction indexes?

Yes, let me follow up with verification. Also they can be used for debugging if needed.

akichidis · 2025-07-25T13:03:23Z

crates/sui-core/src/consensus_manager/mysticeti_manager.rs

@@ -154,10 +154,10 @@ impl ConsensusManagerTrait for MysticetiManager {

        let num_prior_commits = protocol_config.consensus_num_requested_prior_commits_at_startup();
        let last_processed_commit = consensus_handler.last_processed_subdag_index() as CommitIndex;
-        let starting_commit = last_processed_commit.saturating_sub(num_prior_commits);
+        let restart_after_commit = last_processed_commit.saturating_sub(num_prior_commits);


nit: restart_after_commit -> replay_after_commit ?

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 23, 2025 17:09 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 23, 2025 17:10 View deployment

mwtian force-pushed the tmw/consensus-replay branch from b51d012 to ab47b1a Compare July 24, 2025 02:24

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 24, 2025 02:24 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 24, 2025 02:27 View deployment

mwtian force-pushed the tmw/consensus-replay branch from ab47b1a to 920a29a Compare July 24, 2025 04:13

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 24, 2025 04:13 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 24, 2025 04:14 View deployment

mwtian force-pushed the tmw/consensus-replay branch from 920a29a to 84af149 Compare July 24, 2025 04:15

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 24, 2025 04:15 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 24, 2025 04:17 View deployment

mwtian force-pushed the tmw/consensus-replay branch from 84af149 to f7ca8b6 Compare July 24, 2025 04:45

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 24, 2025 04:45 — with GitHub Actions Inactive

mwtian marked this pull request as ready for review July 24, 2025 04:45

mwtian requested a review from a team as a code owner July 24, 2025 04:45

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 24, 2025 04:45 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 24, 2025 04:47 View deployment

mwtian force-pushed the tmw/consensus-replay branch from f7ca8b6 to 104aa61 Compare July 24, 2025 04:59

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 24, 2025 04:59 — with GitHub Actions Inactive

mwtian changed the title ~~[consensus] only wait to replay until highest consumer observed commit~~ [consensus] only wait to replay until highest consumer processed commit Jul 24, 2025

vercel bot deployed to Preview – sui-docs July 24, 2025 05:01 View deployment

mwtian commented Jul 24, 2025

View reviewed changes

consensus/core/src/commit_consumer.rs Show resolved Hide resolved

mwtian requested a review from lxfind July 24, 2025 18:46

mwtian commented Jul 24, 2025

View reviewed changes

consensus/core/src/authority_node.rs Show resolved Hide resolved

mwtian force-pushed the tmw/consensus-replay branch from 104aa61 to 9b91e75 Compare July 25, 2025 01:45

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 25, 2025 01:45 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 25, 2025 01:47 View deployment

mwtian force-pushed the tmw/consensus-replay branch from 9b91e75 to 6fe223c Compare July 25, 2025 02:04

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 25, 2025 02:04 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 25, 2025 02:06 View deployment

mwtian commented Jul 25, 2025

View reviewed changes

lxfind requested a review from mystenmark July 25, 2025 02:14

arun-koshy approved these changes Jul 25, 2025

View reviewed changes

mwtian force-pushed the tmw/consensus-replay branch from 6fe223c to 5882b35 Compare July 25, 2025 03:08

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 25, 2025 03:09 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 25, 2025 03:10 View deployment

[consensus] fix commit recovery with consumer last processed commit i…

89667c3

…ndex

mwtian force-pushed the tmw/consensus-replay branch from 5882b35 to 710a9d9 Compare July 25, 2025 05:08

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 25, 2025 05:08 — with GitHub Actions Inactive

vercel bot had a problem deploying to Preview – sui-docs July 25, 2025 05:09 Failure

persists finalized commit refs and their rejected transactions

d2b91e7

mwtian force-pushed the tmw/consensus-replay branch from 710a9d9 to d2b91e7 Compare July 25, 2025 05:20

mwtian temporarily deployed to sui-typescript-aws-kms-test-env July 25, 2025 05:20 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs July 25, 2025 05:26 View deployment

mwtian merged commit bb47ac8 into main Jul 25, 2025
56 checks passed

mwtian deleted the tmw/consensus-replay branch July 25, 2025 06:27

akichidis reviewed Jul 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[consensus] only wait to replay until highest consumer processed commit #22854

[consensus] only wait to replay until highest consumer processed commit #22854

Uh oh!

mwtian commented Jul 23, 2025 •

edited

Loading

Uh oh!

vercel bot commented Jul 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

mwtian Jul 25, 2025

Uh oh!

arun-koshy Jul 25, 2025

Uh oh!

mwtian Jul 25, 2025

Uh oh!

arun-koshy left a comment

Uh oh!

Uh oh!

arun-koshy Jul 25, 2025

Uh oh!

Uh oh!

akichidis left a comment

Uh oh!

akichidis Jul 25, 2025

Uh oh!

mwtian Jul 25, 2025

Uh oh!

akichidis Jul 25, 2025

Uh oh!

mwtian Jul 25, 2025

Uh oh!

akichidis Jul 25, 2025

Uh oh!

Uh oh!

	if !finalized_commits.is_empty() {
	// Commits and committed blocks must be persisted to storage before sending them to Sui
	// to execute their finalized transactions.
	// Commit metadata and uncommitted blocks can be persisted more lazily because they are recoverable.
	// But for simplicity, all unpersisted commits and blocks are flushed to storage.
	self.dag_state.write().flush();
	}

[consensus] only wait to replay until highest consumer processed commit #22854

[consensus] only wait to replay until highest consumer processed commit #22854

Uh oh!

Conversation

mwtian commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test plan

Release notes

Uh oh!

vercel bot commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arun-koshy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akichidis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mwtian commented Jul 23, 2025 •

edited

Loading

vercel bot commented Jul 23, 2025 •

edited

Loading