Skip to content

Bonsai to archive migration#9997

Merged
jframe merged 80 commits intobesu-eth:mainfrom
jframe:bonsai_archive_migrate
Apr 7, 2026
Merged

Bonsai to archive migration#9997
jframe merged 80 commits intobesu-eth:mainfrom
jframe:bonsai_archive_migrate

Conversation

@jframe
Copy link
Copy Markdown
Contributor

@jframe jframe commented Mar 9, 2026

PR description

fixes #9608

This improves the sync of Bonsai archive to sync as Bonsai and then migration to Bonsai archive. This significantly improves the time taken to sync a Bonsai archive node. It does this by waiting until the node is fully in sync and then processing trie logs sequentially.

There is an impact to the engine API performance during the migration. But this is part of the syncing so some degradation is acceptable and the nodes are able to stay in sync during the migration.

  • Uses the Synchronizer.subscribeInSync instead being an additional sync step as the initial sync complete event is never fired for full sync and I want to use to the same mechanism for all networks. Placing a step in the Full sync pipeline wouldn't work for post-merge testnets that use backward sync
  • Low priority transactions are used for the migrations so that RocksDB throttles migration writes under compaction pressure rather than impacting the engine API. This made the most difference.
  • There is a possible edge case with finishing the migration where a block could be missed switching to archive mode. I have deliberately a fix out of this PR as this will be irrelevant once hybrid mode is implemented in the next PR which will be able to handle missing archive data.

Migration Test Results

Network Test Run 1 Run 2 Run 3
Hoodi Migration time 3h 12m 3h 40m
Mainnet Migration time 7d 13h 51m 7d 4h 27m 7d 4h 27m

Migration Approaches to mitigate engine API times

engine_newPayloadV4 — quantiles during migration (seconds)

p50 mean p50 peak p95 mean p95 peak p99 mean p99 peak
control (no migration) 0.28 s 0.81 s 0.81 s 2.88 s 1.06 s 2.95 s
migrate-10 — no protection 1.75 s 4.40 s 4.52 s 48.07 s 6.08 s 54.98 s
migrate-11 — pause on engine API 1.46 s 17.44 s 3.15 s 39.64 s 3.59 s 47.67 s
migrate-12 — low-priority writes 1.22 s 1.80 s 2.47 s 4.94 s 2.82 s 5.71 s

engine_forkchoiceUpdatedV3 — quantiles during migration (seconds)

p50 mean p50 peak p95 mean p95 peak p99 mean p99 peak
control (no migration) 0.16 s 0.23 s 0.33 s 0.57 s 0.38 s 0.76 s
migrate-10 — no protection 1.46 s 4.09 s 5.65 s 11.96 s 7.30 s 13.32 s
migrate-11 — pause on engine API 0.80 s 2.93 s 3.64 s 14.54 s 4.54 s 17.82 s
migrate-12 — low-priority writes 0.26 s 0.52 s 0.57 s 3.68 s 0.73 s 4.60 s

Fixed Issue(s)

fixes #9608

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests
  • hive tests: Engine or other RPCs modified?

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
…sync

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
When migration approaches the chain head (within TAIL_THRESHOLD blocks),
tail mode activates: the block observer writes archive keys for new blocks
directly and the target is frozen. This eliminates the race window between
the last loop iteration and upgradeToFullFlatDbMode() where blocks could
arrive in FULL format. The switchover ordering is also corrected to call
upgradeToFullFlatDbMode() before removing the observer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Move try/catch in tail mode observer to wrap the entire branch
including getTrieLogLayer(), ensuring any exception is caught and
logged rather than propagating to the block import thread. Also
remove duplicate removeObserver call since finally block handles it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
The observer fires synchronously during appendBlock, so the throwing
mock must be configured before the block is appended. Pre-generate
the block to get its hash, set up the mock, then append.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
…Block

Using endBlock (chain head at migration start) caused the tail zone to
grow with every block imported during the multi-hour migration. Now uses
target.get() so tail mode only activates within TAIL_THRESHOLD of the
current chain head.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
The strategy field is written on the archive-migrator thread during
upgradeToFullFlatDbMode but read on the vert.x-worker thread during
block processing. Without volatile, the JMM does not guarantee the
vert.x thread sees the updated strategy reference promptly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
…d storage

The migrator previously only wrote archive entries when getUpdated()
was non-null, silently skipping deletions. This meant getNearestBefore
would return the pre-deletion state for accounts/storage that were
deleted, since no tombstone existed to indicate the removal.

Now writes DELETED_ACCOUNT_VALUE and DELETED_STORAGE_VALUE markers
consistent with BonsaiArchiveFlatDbStrategy.removeFlatAccount and
removeFlatAccountStorageValueByStorageSlotHash.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
After upgradeToFullFlatDbMode() switches the flat DB strategy from
PARTIAL to ARCHIVE, cached RocksDB snapshots still share the same
FlatDbStrategyProvider and immediately begin using ARCHIVE read paths.
These snapshots were taken before migration completed, so they lack
archive-keyed entries for blocks processed after the snapshot time.
The ARCHIVE getNearestBefore could then return stale historical entries
instead of the correct current state from PARTIAL keys, causing invalid
block validation (e.g. nonce mismatches).

Notify subscribers via onClearFlatDatabaseStorage so the cached world
storage manager drops stale snapshots and forces fresh reads from the
live DB which has complete migration data.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
…once the migration is complete

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Add context-accepting overloads to BonsaiArchiveFlatDbStrategy (putFlatAccount,
removeFlatAccount, putFlatAccountStorageValueByStorageSlotHash,
removeFlatAccountStorageValueByStorageSlotHash) that take an explicit
BonsaiContext. Existing storage-derived @OverRide methods now delegate to these
new overloads. BonsaiFlatDbToArchiveMigrator uses the strategy methods instead
of constructing archive keys directly, eliminating duplicated write logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Copilot AI review requested due to automatic review settings March 9, 2026 01:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for migrating an existing Bonsai node from flat DB mode to archive mode without a full resync, and wires it into controller startup so the archiver is enabled only after migration completes.

Changes:

  • Introduces BonsaiFlatDbToArchiveMigrator to backfill archive-keyed flat DB entries from trie logs and persist migration progress.
  • Extends BonsaiArchiveFlatDbStrategy with overloads that accept an explicit BonsaiContext for archive writes/removals.
  • Updates storage/controller flow: notify subscribers on strategy upgrade, mark strategy pointer volatile, and start migration after the node is in sync.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/pathbased/bonsaiarchive/BonsaiFlatDbToArchiveMigrator.java New migrator that replays trie logs into archive-keyed flat DB entries and records progress.
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/pathbased/bonsai/storage/flat/BonsaiArchiveFlatDbStrategy.java Adds context-based overloads used by the migrator to write/delete archive entries at a given block context.
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/pathbased/bonsai/storage/BonsaiWorldStateKeyValueStorage.java Notifies subscribers to clear caches after switching flat DB strategy to avoid stale snapshot reads.
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/pathbased/common/storage/flat/FlatDbStrategyProvider.java Makes the strategy reference volatile for cross-thread visibility when switching strategies.
app/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java Starts migration post-sync (when in FULL mode) and enables archiver only after successful migration.
ethereum/core/src/test/java/org/hyperledger/besu/ethereum/trie/pathbased/bonsaiarchive/BonsaiFlatDbToArchiveMigratorTest.java New unit tests covering migrator behavior (progress, concurrency, failures, block catch-up).
ethereum/core/src/test/java/org/hyperledger/besu/ethereum/trie/pathbased/bonsai/storage/BonsaiWorldStateKeyValueStorageTest.java Adds test ensuring subscribers are notified to clear caches on strategy upgrade.

jframe added 17 commits March 27, 2026 14:43
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Previously showed startBlock (first block of current run) as the
resumed-from value. Now passes lastProcessedBlock (-1 for a fresh
start) and only shows the resumed-from message when progress
actually existed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
… needed when upgrading to archive mode as the flatDbStrategy is incompatible

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Covers commit persists, rollback discards, and post-commit put throws.
Tests run for both OptimisticRocksDB and TransactionDB via the base class.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Covers commit persists, rollback discards, and post-commit put throws.
Tests run for both OptimisticRocksDB and TransactionDB via the base class.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
@jframe jframe marked this pull request as ready for review March 31, 2026 03:12
jframe added 2 commits April 2, 2026 10:26
…gration triggered

Signed-off-by: Jason Frame <jason.frame@consensys.net>
…x a possible window where another migration could trigger another upgrade

Signed-off-by: Jason Frame <jason.frame@consensys.net>
@usmansaleem
Copy link
Copy Markdown
Contributor

Code review

Found 1 issue:

  1. Resource leak: transaction not rolled back on exception in migrateBlocks()

In migrateBlocks(), a SegmentedKeyValueStorageTransaction is opened via startLowPriorityTransaction() on each loop iteration, but there is no try/finally or try-with-resources to guarantee rollback or cleanup if processBlock() throws or the IllegalStateException for a missing trie log is raised. The transaction is neither committed nor rolled back on the exception path, leaking the underlying native RocksDB transaction object. Since the migration runs over millions of blocks on mainnet (7+ days), any transient failure in processBlock() will leak native memory. Wrapping the transaction usage in try { ... tx.commit(); } catch (...) { tx.rollback(); throw; } (or making the transaction AutoCloseable and using try-with-resources) would fix this.

final SegmentedKeyValueStorageTransaction tx =
worldStateStorage.getComposedWorldStateStorage().startLowPriorityTransaction();
if (maybeTrieLog.isPresent()) {
processBlock(maybeTrieLog.get(), blockNumber, tx);
migratedBlockNumber.incrementAndGet();
} else if (blockNumber > 0) {
throw new IllegalStateException("No trie log found for block " + blockNumber);
}
saveProgress(blockNumber, tx);
tx.commit();

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@jframe
Copy link
Copy Markdown
Contributor Author

jframe commented Apr 2, 2026

Code review

Found 1 issue:

  1. Resource leak: transaction not rolled back on exception in migrateBlocks()

In migrateBlocks(), a SegmentedKeyValueStorageTransaction is opened via startLowPriorityTransaction() on each loop iteration, but there is no try/finally or try-with-resources to guarantee rollback or cleanup if processBlock() throws or the IllegalStateException for a missing trie log is raised. The transaction is neither committed nor rolled back on the exception path, leaking the underlying native RocksDB transaction object. Since the migration runs over millions of blocks on mainnet (7+ days), any transient failure in processBlock() will leak native memory. Wrapping the transaction usage in try { ... tx.commit(); } catch (...) { tx.rollback(); throw; } (or making the transaction AutoCloseable and using try-with-resources) would fix this.

final SegmentedKeyValueStorageTransaction tx =
worldStateStorage.getComposedWorldStateStorage().startLowPriorityTransaction();
if (maybeTrieLog.isPresent()) {
processBlock(maybeTrieLog.get(), blockNumber, tx);
migratedBlockNumber.incrementAndGet();
} else if (blockNumber > 0) {
throw new IllegalStateException("No trie log found for block " + blockNumber);
}
saveProgress(blockNumber, tx);
tx.commit();

🤖 Generated with Claude Code

  • If this code review was useful, please react with 👍. Otherwise, react with 👎.

The general throws case is handled already by RocksDbTransaction. While I don't think it's much of an issue we could create an empty transaction since we create that before trying to get the trielog. So I'll change it to only create the transaction if we aren't going to exit early with the IllegalStateException

jframe added 2 commits April 2, 2026 18:20
… a transaction and never use it due an early exit

Signed-off-by: Jason Frame <jason.frame@consensys.net>
Copy link
Copy Markdown
Contributor

@usmansaleem usmansaleem left a comment

Choose a reason for hiding this comment

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

LGTM

jframe added 2 commits April 7, 2026 09:56
Signed-off-by: Jason Frame <jason.frame@consensys.net>
@jframe jframe enabled auto-merge (squash) April 7, 2026 01:04
@jframe jframe merged commit 15082ba into besu-eth:main Apr 7, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bonsai Archive: Sync as Bonsai then migrate to archive

4 participants