Sync block body #8242

pinges · 2025-02-04T06:13:57Z

PR description

This PR changes the SNAP and CHECKPOINT sync to reduce the memory requirements by not decoding the block bodies requested from peers. This results in less memory and CPU usage during the sync.
This PR does make sure that the properties of the syncs are the same as before, e.g. transaction indexing and block added events haven't changed.

Note: this PR does not significantly reduce the sync time on it's own, because the bottleneck is the import step of the download pipeline. It will however allow us to increase the parallelism of the pipeline because of the reduced memory and CPU usage. Once the import step is improved that increase in parallelism will allow us to reduce the sync time.

Note: Not decoding receipts during the sync does improve the memory and CPU usage as well. This has been shown in a POC.

First screenshot:

CPU% GC young generation during sync with this PR:

CPU% GC young generation during sync without this PR:

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadSyncBodiesStep.java

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/SyncBlockBody.java

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java

jframe · 2025-02-06T05:06:58Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java

+                  () -> new IllegalStateException("Blockchain is missing total difficulty data."));
+      difficultyForSyncing = parentTotalDifficulty.add(blockHeader.getDifficulty());
+    } else {
+      difficultyForSyncing = difficultyForSyncing.add(blockHeader.getDifficulty());


Is this just an optimised path to avoid db lookup for the parent header?

Yes, as long as we are importing the blocks in order we shouldn't have to ask the DB what the last blocks td was.

I think the parent header is already cached as we cache blocks when importing so this might not be needed

...m/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteSyncBlocksTask.java

.../src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetSyncBlocksFromPeerTask.java

.../src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/DownloadSyncReceiptsStep.java

.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/ImportSyncBlocksStep.java

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

jframe

Are all the sync tasks using the legacy peer task system? implementing the new peer task system for sync blocks could be done in another PR but we need at least the legacy version for now.

jframe · 2025-02-13T05:42:47Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java

+                  () -> new IllegalStateException("Blockchain is missing total difficulty data."));
+      difficultyForSyncing = parentTotalDifficulty.add(blockHeader.getDifficulty());
+    } else {
+      difficultyForSyncing = difficultyForSyncing.add(blockHeader.getDifficulty());


I think the parent header is already cached as we cache blocks when importing so this might not be needed

jframe · 2025-02-13T05:53:36Z

.../src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetSyncBlocksFromPeerTask.java

+    if ((header.getWithdrawalsRoot().isEmpty() && body.getWithdrawalsRoot() != null)
+        || (header.getWithdrawalsRoot().isPresent() && body.getWithdrawalsRoot() == null)) {
+      return false;
+    } else if ((header.getWithdrawalsRoot().isPresent()
+        && !header.getWithdrawalsRoot().get().equals(body.getWithdrawalsRoot()))) {
+      return false;


Think this could be simpler, we should be able to do the equality check on the optional to simplify this
i.e. header.getWithdrawalsRoot().equals(body.getWithdrawalsRoot()) ?

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Matilda-Clerke · 2025-02-20T23:01:51Z

...va/org/hyperledger/besu/ethereum/api/jsonrpc/internal/filter/FilterManagerLogFilterTest.java

-    final Hash blockAddedHash = recordBlockEvents(1).get(0).getBlock().getHash();
+    final Hash blockAddedHash = recordBlockEvents(1).get(0).getHeader().getHash();


Probably would have made for cleaner reviews if this kind of change was done separately

Matilda-Clerke · 2025-02-20T23:10:17Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java

+
+    return BlockAddedEvent.createForSyncHeadAdvancement(
+        newBlock.getHeader(),
+        () -> new Block(newBlock.getHeader(), newBlock.getBody().getBodySupplier().get()),


What's the benefit of delaying execution of getting the block body here?

Matilda-Clerke · 2025-02-20T23:18:34Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/SyncBlockBody.java

+    final SyncBlockBody body =
+        new SyncBlockBody(
+            bytesCurrentBody, transactionBytes, ommersListBytes, withdrawalBytes, protocolSchedule);
+    return body;


Any reason not to just return new SyncBlockBody(...?

Matilda-Clerke · 2025-02-20T23:44:23Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BodyValidation.java

+    final ArrayList<Bytes> bytesList = new ArrayList<>(withdrawals.size());
+    withdrawals.forEach(w -> bytesList.add(WithdrawalEncoder.encodeOpaqueBytes(w)));

-    return Hash.wrap(trie.getRootHash());
+    return Util.getRootFromListOfBytes(bytesList);


Could alternatively do return Util.getRootFromListOfBytes(withdrawals.stream().map(WithdrawalEncoder::encodeOpaqueBytes).toList()); to make it a simple one liner.

Likewise with the other changed methods in BodyValidation

Matilda-Clerke · 2025-02-21T01:25:14Z

.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBodiesFromPeerTask.java

+  boolean bodyMatchesHeader(final BlockBody body, final BlockHeader header) {
+    if ((header.getWithdrawalsRoot().isEmpty() && body.getWithdrawals().isPresent())
+        || (header.getWithdrawalsRoot().isPresent() && body.getWithdrawals().isEmpty())) {
+      return false;
+    } else if ((header.getWithdrawalsRoot().isPresent()
+        && !header
+            .getWithdrawalsRoot()
+            .get()
+            .equals(BodyValidation.withdrawalsRoot(body.getWithdrawals().get())))) {
+      return false;
+    } else {
+      return BodyValidation.transactionsRoot(body.getTransactions())
+              .equals(header.getTransactionsRoot())
+          && BodyValidation.ommersHash(body.getOmmers()).equals(header.getOmmersHash());


Not worth utilising the BlockBodyIdentifier class like in the other implementation?

Matilda-Clerke · 2025-02-21T01:28:18Z

.../main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/AbstractDownloadReceiptsStep.java

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public abstract class AbstractDownloadReceiptsStep<B, BWR>


Probably worth adding some comments about exactly what B and BWR are expected to represent

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

pinges added 19 commits December 19, 2024 17:23

first try

f1e1113

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

fix NPE

9abc4dd

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

no logsWithMetadata available during sync with SyncBlock. Fixes NPE

b0d15fc

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

add logging when importing

f2acc7e

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

merge main

da9cff8

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

added transaction indexing and log meta data

c133616

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

make last steps in pipeline not ordered and convert ImportStep

cece7a4

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

increase import step parallelism

15aa667

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

make all ordered

ec796ef

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

make import shorter

faa1b1b

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

87b5eb4

replace Body with SyncBody in Fastsync

1e50b17

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

cd2beef

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

4a0ccd5

add some tests and some minor changes

baf668f

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

709ff3a

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

1b93d01

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

6521fb2

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

a89f402

pinges requested a review from jframe February 6, 2025 04:07

jframe reviewed Feb 6, 2025

View reviewed changes

pinges added 7 commits February 7, 2025 14:11

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

b2be1ec

clean up after review

4cdb549

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

reduce header request size to 120

995f74a

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

f682d48

fix AGBFPT

c61feb6

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

undo change of header request size

b347e1e

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

b5ed59d

jframe reviewed Feb 13, 2025

View reviewed changes

minor changes for readability

5a34ec8

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

pinges added 6 commits February 14, 2025 09:44

merge conflicts

b4e7898

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

7bdc04b

add usage of legacy bodies download

424d27e

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

5769c0a

add some logging

1840bb3

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Merge branch 'main' of github.com:hyperledger/besu into SyncBlockBody

e3e1c2f

Matilda-Clerke reviewed Feb 21, 2025

View reviewed changes

add logging

9f1b6d3

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync block body #8242

Sync block body #8242

pinges commented Feb 4, 2025 •

edited

Loading

jframe Feb 6, 2025

pinges Feb 6, 2025 •

edited

Loading

jframe Feb 13, 2025 •

edited

Loading

jframe left a comment

jframe Feb 13, 2025 •

edited

Loading

jframe Feb 13, 2025

Matilda-Clerke Feb 20, 2025

Matilda-Clerke Feb 20, 2025

Matilda-Clerke Feb 20, 2025

Matilda-Clerke Feb 20, 2025

Matilda-Clerke Feb 21, 2025

Matilda-Clerke Feb 21, 2025

		final Hash blockAddedHash = recordBlockEvents(1).get(0).getBlock().getHash();
		final Hash blockAddedHash = recordBlockEvents(1).get(0).getHeader().getHash();

Sync block body #8242

Are you sure you want to change the base?

Sync block body #8242

Conversation

pinges commented Feb 4, 2025 • edited Loading

PR description

Choose a reason for hiding this comment

pinges Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

jframe Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

jframe left a comment

Choose a reason for hiding this comment

jframe Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinges commented Feb 4, 2025 •

edited

Loading

pinges Feb 6, 2025 •

edited

Loading

jframe Feb 13, 2025 •

edited

Loading

jframe Feb 13, 2025 •

edited

Loading