revert 6499 flat-db-healing-enabled false by default (hyperledger#6642)

Signed-off-by: Sally MacFarlane <[email protected]> Signed-off-by: amsmota <[email protected]>
Citi · Apr 16, 2024 · 00ca00a · 00ca00a
1 parent 0201b84
commit 00ca00a
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,7 +37,6 @@
 
 ### Deprecations
 - X_SNAP and X_CHECKPOINT are marked for deprecation and will be removed in 24.4.0 in favor of SNAP and CHECKPOINT [#6405](https://github.com/hyperledger/besu/pull/6405)
-- `--Xsnapsync-synchronizer-flat-db-healing-enabled` is deprecated (always enabled). [#6499](https://github.com/hyperledger/besu/pull/6499)
 - `--Xp2p-peer-lower-bound` is deprecated. [#6501](https://github.com/hyperledger/besu/pull/6501)
 
 ### Upcoming Breaking Changes

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -1639,6 +1639,14 @@ && isOptionSet(commandLine, "--sync-min-peers")) {
         SyncMode.isCheckpointSync(getDefaultSyncModeIfNotSet()),
         singletonList("--Xcheckpoint-post-merge-enabled"));
 
+    CommandLineUtils.failIfOptionDoesntMeetRequirement(
+        commandLine,
+        "--Xsnapsync-synchronizer-flat option can only be used when -Xsnapsync-synchronizer-flat-db-healing-enabled is true",
+        unstableSynchronizerOptions.isSnapsyncFlatDbHealingEnabled(),
+        asList(
+            "--Xsnapsync-synchronizer-flat-account-healed-count-per-request",
+            "--Xsnapsync-synchronizer-flat-slot-healed-count-per-request"));
+
     if (!securityModuleName.equals(DEFAULT_SECURITY_MODULE)
         && nodePrivateKeyFileOption.getNodePrivateKeyFile() != null) {
       logger.warn(

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java
@@ -292,8 +292,7 @@ public void parseBlockPropagationRange(final String arg) {
       names = SNAP_FLAT_DB_HEALING_ENABLED_FLAG,
       hidden = true,
       paramLabel = "<Boolean>",
-      description =
-          "(Deprecated) Always enabled: Snap sync flat db healing enabled (default: ${DEFAULT-VALUE})")
+      description = "Snap sync flat db healing enabled (default: ${DEFAULT-VALUE})")
   private Boolean snapsyncFlatDbHealingEnabled =
       SnapSyncConfiguration.DEFAULT_IS_FLAT_DB_HEALING_ENABLED;
 
@@ -306,6 +305,15 @@ public void parseBlockPropagationRange(final String arg) {
 
   private SynchronizerOptions() {}
 
+  /**
+   * Flag to know whether the flat db healing feature is enabled or disabled.
+   *
+   * @return true is the flat db healing is enabled
+   */
+  public boolean isSnapsyncFlatDbHealingEnabled() {
+    return snapsyncFlatDbHealingEnabled;
+  }
+
   /**
    * Create synchronizer options.
    *
@@ -441,11 +449,15 @@ public List<String> getCLIOptions() {
             SNAP_BYTECODE_COUNT_PER_REQUEST_FLAG,
             OptionParser.format(snapsyncBytecodeCountPerRequest),
             SNAP_TRIENODE_COUNT_PER_REQUEST_FLAG,
-            OptionParser.format(snapsyncTrieNodeCountPerRequest),
-            SNAP_FLAT_ACCOUNT_HEALED_COUNT_PER_REQUEST_FLAG,
-            OptionParser.format(snapsyncFlatAccountHealedCountPerRequest),
-            SNAP_FLAT_STORAGE_HEALED_COUNT_PER_REQUEST_FLAG,
-            OptionParser.format(snapsyncFlatStorageHealedCountPerRequest));
+            OptionParser.format(snapsyncTrieNodeCountPerRequest));
+    if (isSnapsyncFlatDbHealingEnabled()) {
+      value.addAll(
+          Arrays.asList(
+              SNAP_FLAT_ACCOUNT_HEALED_COUNT_PER_REQUEST_FLAG,
+              OptionParser.format(snapsyncFlatAccountHealedCountPerRequest),
+              SNAP_FLAT_STORAGE_HEALED_COUNT_PER_REQUEST_FLAG,
+              OptionParser.format(snapsyncFlatStorageHealedCountPerRequest)));
+    }
     return value;
   }
 }
diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -3498,12 +3498,29 @@ public void txpoolTxPoolMinGasPriceMustNotBeGreaterThanMinGasPriceZero() {
   }
 
   @Test
-  public void snapsyncForHealingFeaturesShouldFailWhenHealingIsNotSet_EnabledByDefault() {
+  public void snapsyncHealingOptionShouldBeDisabledByDefault() {
+    final TestBesuCommand besuCommand = parseCommand();
+    assertThat(besuCommand.unstableSynchronizerOptions.isSnapsyncFlatDbHealingEnabled()).isFalse();
+  }
+
+  @Test
+  public void snapsyncHealingOptionShouldWork() {
+    final TestBesuCommand besuCommand =
+        parseCommand("--Xsnapsync-synchronizer-flat-db-healing-enabled", "true");
+    assertThat(besuCommand.unstableSynchronizerOptions.isSnapsyncFlatDbHealingEnabled()).isTrue();
+  }
+
+  @Test
+  public void snapsyncForHealingFeaturesShouldFailWhenHealingIsNotEnabled() {
     parseCommand("--Xsnapsync-synchronizer-flat-account-healed-count-per-request", "100");
-    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8))
+        .contains(
+            "--Xsnapsync-synchronizer-flat option can only be used when -Xsnapsync-synchronizer-flat-db-healing-enabled is true");
 
     parseCommand("--Xsnapsync-synchronizer-flat-slot-healed-count-per-request", "100");
-    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8))
+        .contains(
+            "--Xsnapsync-synchronizer-flat option can only be used when -Xsnapsync-synchronizer-flat-db-healing-enabled is true");
   }
 
   @Test

diff --git a/.../src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapSyncConfiguration.java b/.../src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/SnapSyncConfiguration.java
@@ -36,7 +36,7 @@ public class SnapSyncConfiguration {
   public static final int DEFAULT_LOCAL_FLAT_STORAGE_COUNT_TO_HEAL_PER_REQUEST =
       1024; // The default number of flat slots entries to verify and heal per request.
 
-  public static final Boolean DEFAULT_IS_FLAT_DB_HEALING_ENABLED = Boolean.TRUE;
+  public static final Boolean DEFAULT_IS_FLAT_DB_HEALING_ENABLED = Boolean.FALSE;
 
   public static SnapSyncConfiguration getDefault() {
     return ImmutableSnapSyncConfiguration.builder().build();