Persist backward sync status to support resuming across restarts

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
hyperledger · Mar 7, 2023 · 667e82a · 667e82a
1 parent 42f9569
commit 667e82a
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,7 @@
 ### Additions and Improvements
 
 ### Bug Fixes
-- Fix for backward sync wrongly thinking it is done after a restart [#5182](https://github.com/hyperledger/besu/pull/5182)
+- Persist backward sync status to support resuming across restarts [#5182](https://github.com/hyperledger/besu/pull/5182)
 
 ## 23.1.1
 This update is required for the Goerli Shanghai/Capella upgrade and recommended for all Mainnet users. If you use Besu on Goerli, update to 23.1.1. If you previously used 23.1.1-RC1, update to test 23.1.1 on Goerli. 

diff --git a/.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardChain.java b/.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardChain.java
@@ -36,8 +36,8 @@
 public class BackwardChain {
   private static final Logger LOG = getLogger(BackwardChain.class);
 
-  private final static String FIRST_STORED_ANCESTOR_KEY = "firstStoredAncestor";
-  private final static String LAST_STORED_PIVOT_KEY = "lastStoredPivot";
+  private static final String FIRST_STORED_ANCESTOR_KEY = "firstStoredAncestor";
+  private static final String LAST_STORED_PIVOT_KEY = "lastStoredPivot";
 
   private final GenericKeyValueStorageFacade<Hash, BlockHeader> headers;
   private final GenericKeyValueStorageFacade<Hash, Block> blocks;
@@ -56,8 +56,28 @@ public BackwardChain(
     this.blocks = blocksStorage;
     this.chainStorage = chainStorage;
     this.variablesStorage = variablesStorage;
-    firstStoredAncestor = variablesStorage.get(FIRST_STORED_ANCESTOR_KEY);
-    lastStoredPivot = variablesStorage.get(LAST_STORED_PIVOT_KEY);
+    firstStoredAncestor =
+        variablesStorage
+            .get(FIRST_STORED_ANCESTOR_KEY)
+            .map(
+                header -> {
+                  LOG.atDebug()
+                      .setMessage(FIRST_STORED_ANCESTOR_KEY + " loaded from storage with value {}")
+                      .addArgument(header::toLogString)
+                      .log();
+                  return header;
+                });
+    lastStoredPivot =
+        variablesStorage
+            .get(LAST_STORED_PIVOT_KEY)
+            .map(
+                header -> {
+                  LOG.atDebug()
+                      .setMessage(LAST_STORED_PIVOT_KEY + " loaded from storage with value {}")
+                      .addArgument(header::toLogString)
+                      .log();
+                  return header;
+                });
   }
 
   public static BackwardChain from(
@@ -78,12 +98,11 @@ public static BackwardChain from(
             new HashConvertor(),
             storageProvider.getStorageBySegmentIdentifier(
                 KeyValueSegmentIdentifier.BACKWARD_SYNC_CHAIN)),
-            new GenericKeyValueStorageFacade<>(
-                    key -> key.getBytes(StandardCharsets.UTF_8),
-                    BlocksHeadersConvertor.of(blockHeaderFunctions),
-                    storageProvider.getStorageBySegmentIdentifier(
-                            KeyValueSegmentIdentifier.BACKWARD_SYNC_CHAIN))
-            );
+        new GenericKeyValueStorageFacade<>(
+            key -> key.getBytes(StandardCharsets.UTF_8),
+            BlocksHeadersConvertor.of(blockHeaderFunctions),
+            storageProvider.getStorageBySegmentIdentifier(
+                KeyValueSegmentIdentifier.BACKWARD_SYNC_CHAIN)));
   }
 
   public synchronized Optional<BlockHeader> getFirstAncestorHeader() {
@@ -103,8 +122,10 @@ public synchronized List<BlockHeader> getFirstNAncestorHeaders(final int size) {
   public synchronized void prependAncestorsHeader(final BlockHeader blockHeader) {
     prependAncestorsHeader(blockHeader, false);
   }
-  public synchronized void prependAncestorsHeader(final BlockHeader blockHeader, final boolean alreadyStored) {
-    if(!alreadyStored) {
+
+  public synchronized void prependAncestorsHeader(
+      final BlockHeader blockHeader, final boolean alreadyStored) {
+    if (!alreadyStored) {
       headers.put(blockHeader.getHash(), blockHeader);
     }
 
@@ -114,25 +135,27 @@ public synchronized void prependAncestorsHeader(final BlockHeader blockHeader, f
       final BlockHeader firstHeader = firstStoredAncestor.get();
       chainStorage.put(blockHeader.getHash(), firstHeader.getHash());
       LOG.atDebug()
-              .setMessage("Added header {} to backward chain led by pivot {} on height {}")
-              .addArgument(blockHeader::toLogString)
-              .addArgument(() -> lastStoredPivot.orElseThrow().toLogString())
-              .addArgument(firstHeader::getNumber)
-              .log();
+          .setMessage("Added header {} to backward chain led by pivot {} on height {}")
+          .addArgument(blockHeader::toLogString)
+          .addArgument(() -> lastStoredPivot.orElseThrow().toLogString())
+          .addArgument(firstHeader::getNumber)
+          .log();
     }
 
     updateFirstStoredAncestor(Optional.of(blockHeader));
   }
 
   private void updateFirstStoredAncestor(final Optional<BlockHeader> maybeHeader) {
-    maybeHeader.ifPresentOrElse(header ->
-    variablesStorage.put(FIRST_STORED_ANCESTOR_KEY, header), () -> variablesStorage.drop(FIRST_STORED_ANCESTOR_KEY));
+    maybeHeader.ifPresentOrElse(
+        header -> variablesStorage.put(FIRST_STORED_ANCESTOR_KEY, header),
+        () -> variablesStorage.drop(FIRST_STORED_ANCESTOR_KEY));
     firstStoredAncestor = maybeHeader;
   }
 
   private void updateLastStorePivot(final Optional<BlockHeader> maybeHeader) {
-    maybeHeader.ifPresentOrElse(header ->
-            variablesStorage.put(LAST_STORED_PIVOT_KEY, header), () -> variablesStorage.drop(LAST_STORED_PIVOT_KEY));
+    maybeHeader.ifPresentOrElse(
+        header -> variablesStorage.put(LAST_STORED_PIVOT_KEY, header),
+        () -> variablesStorage.drop(LAST_STORED_PIVOT_KEY));
     lastStoredPivot = maybeHeader;
   }
 

diff --git a/.../main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncAlgorithm.java b/.../main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncAlgorithm.java
@@ -64,7 +64,10 @@ public CompletableFuture<Void> pickNextStep() {
       return executeSyncStep(firstHash.get())
           .thenAccept(
               result -> {
-                LOG.atDebug().setMessage("Backward sync target block is {}").addArgument(result::toLogString).log();
+                LOG.atDebug()
+                    .setMessage("Backward sync target block is {}")
+                    .addArgument(result::toLogString)
+                    .log();
                 context.getBackwardChain().removeFromHashToAppend(firstHash.get());
                 context.getStatus().updateTargetHeight(result.getHeader().getNumber());
               });

diff --git a/...th/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/StringConvertor.java b/...th/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/StringConvertor.java
diff --git a/...rc/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncAlgSpec.java b/...rc/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncAlgSpec.java
@@ -66,8 +66,7 @@ public class BackwardSyncAlgSpec {
   @Captor ArgumentCaptor<BesuEvents.TTDReachedListener> ttdCaptor;
   @Captor ArgumentCaptor<BesuEvents.InitialSyncCompletionListener> completionCaptor;
 
-  @InjectMocks
-  BackwardSyncAlgorithm algorithm;
+  @InjectMocks BackwardSyncAlgorithm algorithm;
   @Mock private Hash hash;
 
   private static final BlockDataGenerator blockDataGenerator = new BlockDataGenerator();