Skip to content

feat: parallelize save_blocks #20993

Merged
joshieDo merged 13 commits intomainfrom
joshie/par-save-blocks
Jan 15, 2026
Merged

feat: parallelize save_blocks #20993
joshieDo merged 13 commits intomainfrom
joshie/par-save-blocks

Conversation

@joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Jan 13, 2026

  • parallelize save_blocks
  • insert_block now calls save_blocks

PR Stack

#21045 (metrics)    #21003 (rocksdb)
         \           /
          \         /
           #21012 (account changesets)
              ↓
           #20993 (parallelize save_blocks) ◀
              ↓
            main

@joshieDo joshieDo self-assigned this Jan 13, 2026
@joshieDo joshieDo added A-db Related to the database A-static-files Related to static files labels Jan 13, 2026
@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Jan 13, 2026
@joshieDo joshieDo changed the base branch from main to joshie/split-commit-sf January 13, 2026 13:03
@joshieDo joshieDo force-pushed the joshie/split-commit-sf branch from 9bd7808 to 4629f21 Compare January 13, 2026 14:57
@joshieDo joshieDo force-pushed the joshie/par-save-blocks branch 2 times, most recently from 01196a8 to f101e09 Compare January 13, 2026 15:48
@joshieDo joshieDo force-pushed the joshie/split-commit-sf branch from 4629f21 to 6275ac6 Compare January 13, 2026 15:48
@joshieDo joshieDo force-pushed the joshie/par-save-blocks branch from f101e09 to 868f884 Compare January 13, 2026 15:58
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some suggestions, but afaict the flow makes sense, in parallelizes as much as possible


// finally, write the receipts
provider.write_state(&execution_outcome, OriginalValuesKnown::Yes)?;
provider.write_state(&execution_outcome, OriginalValuesKnown::Yes, true)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this a config struct because i assume well add more settings eventually?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#21012 added here

Comment on lines -3039 to -3059
/// Inserts the block into the database, always modifying the following static file segments and
/// tables:
/// * [`StaticFileSegment::Headers`]
/// * [`tables::HeaderNumbers`]
/// * [`tables::BlockBodyIndices`]
///
/// If there are transactions in the block, the following static file segments and tables will
/// be modified:
/// * [`StaticFileSegment::Transactions`]
/// * [`tables::TransactionBlocks`]
/// Inserts the block into the database, writing to both static files and MDBX.
///
/// If ommers are not empty, this will modify [`BlockOmmers`](tables::BlockOmmers).
/// If withdrawals are not empty, this will modify
/// [`BlockWithdrawals`](tables::BlockWithdrawals).
///
/// If the provider has __not__ configured full sender pruning, this will modify either:
/// * [`StaticFileSegment::TransactionSenders`] if senders are written to static files
/// * [`tables::TransactionSenders`] if senders are written to the database
///
/// If the provider has __not__ configured full transaction lookup pruning, this will modify
/// [`TransactionHashNumbers`](tables::TransactionHashNumbers).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this no longer accurate?

/// [`TransactionHashNumbers`](tables::TransactionHashNumbers).
/// This is a convenience method primarily used in tests. For production use,
/// prefer [`Self::save_blocks`] which handles execution output and trie data.
fn insert_block(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when do we even call this?

separately

appears to be unused?

///
/// If the provider has __not__ configured full transaction lookup pruning, this will modify
/// [`TransactionHashNumbers`](tables::TransactionHashNumbers).
/// This is a convenience method primarily used in tests. For production use,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah nvm, I think I understand now

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Jan 13, 2026
@joshieDo joshieDo force-pushed the joshie/par-save-blocks branch 2 times, most recently from b271a28 to 4fdb472 Compare January 13, 2026 16:25
@joshieDo joshieDo marked this pull request as ready for review January 13, 2026 16:28
@joshieDo joshieDo force-pushed the joshie/par-save-blocks branch 2 times, most recently from 8f2cbc4 to 76c93c6 Compare January 13, 2026 16:30
Base automatically changed from joshie/split-commit-sf to main January 13, 2026 16:38
@joshieDo joshieDo force-pushed the joshie/par-save-blocks branch 2 times, most recently from 705619b to c38dbb8 Compare January 13, 2026 16:47
@joshieDo joshieDo marked this pull request as draft January 13, 2026 18:18
@joshieDo joshieDo marked this pull request as ready for review January 13, 2026 18:35
@joshieDo joshieDo requested a review from mattsse January 13, 2026 18:44
Copy link
Member

@pepyakin pepyakin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've went through the code and it looks reasonable to me.

for (i, block) in blocks.iter().enumerate() {
let recovered_block = block.recovered_block();

let start = Instant::now();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have the instant::now() outside the loop? right now in this scope we have a couple of this ~5

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems fine, I think we want this per block

Copy link
Member

@yongkangc yongkangc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshieDo left some comments, overall flow lgtm

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checks out

for (i, block) in blocks.iter().enumerate() {
let recovered_block = block.recovered_block();

let start = Instant::now();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems fine, I think we want this per block

@joshieDo joshieDo added this pull request to the merge queue Jan 15, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 15, 2026
@joshieDo joshieDo enabled auto-merge January 15, 2026 14:23
@joshieDo joshieDo added this pull request to the merge queue Jan 15, 2026
Merged via the queue into main with commit f012b33 Jan 15, 2026
44 checks passed
@joshieDo joshieDo deleted the joshie/par-save-blocks branch January 15, 2026 15:17
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Jan 15, 2026
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 22, 2026
Co-authored-by: Sergei Shulepov <s.pepyakin@gmail.com>
Co-authored-by: Sergei Shulepov <pep@tempo.xyz>
Co-authored-by: Brian Picciano <me@mediocregopher.com>
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Feb 11, 2026
Co-authored-by: Sergei Shulepov <s.pepyakin@gmail.com>
Co-authored-by: Sergei Shulepov <pep@tempo.xyz>
Co-authored-by: Brian Picciano <me@mediocregopher.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-db Related to the database A-static-files Related to static files

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants