Flat db heal - deprecate CLI option and set to always true (hyperledg…

…er#6499) * flat db heal always set to true Signed-off-by: Sally MacFarlane <[email protected]> * changelog Signed-off-by: Sally MacFarlane <[email protected]> --------- Signed-off-by: Sally MacFarlane <[email protected]>
shemnon · Jan 31, 2024 · 8fb503e · 8fb503e
1 parent 38161c9
commit 8fb503e
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 70 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 
 
 ### Deprecations
+- `--Xsnapsync-synchronizer-flat-db-healing-enabled` is deprecated (always enabled). [#6499](https://github.com/hyperledger/besu/pull/6499)
 
 ### Additions and Improvements
 - Upgrade Prometheus and Opentelemetry dependencies [#6422](https://github.com/hyperledger/besu/pull/6422)

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -1677,14 +1677,6 @@ && isOptionSet(commandLine, "--sync-min-peers")) {
         SyncMode.X_CHECKPOINT.equals(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
@@ -106,7 +106,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = DOWNLOADER_CHANGE_TARGET_THRESHOLD_BY_HEIGHT_FLAG,
       hidden = true,
-      defaultValue = "200",
       paramLabel = "<LONG>",
       description =
           "Minimum height difference before switching fast sync download peers (default: ${DEFAULT-VALUE})")
@@ -116,7 +115,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = DOWNLOADER_CHANGE_TARGET_THRESHOLD_BY_TD_FLAG,
       hidden = true,
-      defaultValue = "1000000000000000000",
       paramLabel = "<UINT256>",
       description =
           "Minimum total difficulty difference before switching fast sync download peers (default: ${DEFAULT-VALUE})")
@@ -126,7 +124,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = DOWNLOADER_HEADER_REQUEST_SIZE_FLAG,
       hidden = true,
-      defaultValue = "200",
       paramLabel = "<INTEGER>",
       description = "Number of headers to request per packet (default: ${DEFAULT-VALUE})")
   private int downloaderHeaderRequestSize =
@@ -135,7 +132,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = DOWNLOADER_CHECKPOINT_TIMEOUTS_PERMITTED_FLAG,
       hidden = true,
-      defaultValue = "5",
       paramLabel = "<INTEGER>",
       description =
           "Number of tries to attempt to download checkpoints before stopping (default: ${DEFAULT-VALUE})")
@@ -145,7 +141,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = DOWNLOADER_CHAIN_SEGMENT_SIZE_FLAG,
       hidden = true,
-      defaultValue = "200",
       paramLabel = "<INTEGER>",
       description = "Distance between checkpoint headers (default: ${DEFAULT-VALUE})")
   private int downloaderChainSegmentSize =
@@ -154,15 +149,13 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = DOWNLOADER_PARALLELISM_FLAG,
       hidden = true,
-      defaultValue = "4",
       paramLabel = "<INTEGER>",
       description = "Number of threads to provide to chain downloader (default: ${DEFAULT-VALUE})")
   private int downloaderParallelism = SynchronizerConfiguration.DEFAULT_DOWNLOADER_PARALLELISM;
 
   @CommandLine.Option(
       names = TRANSACTIONS_PARALLELISM_FLAG,
       hidden = true,
-      defaultValue = "2",
       paramLabel = "<INTEGER>",
       description =
           "Number of threads to commit to transaction processing (default: ${DEFAULT-VALUE})")
@@ -179,7 +172,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = PIVOT_DISTANCE_FROM_HEAD_FLAG,
       hidden = true,
-      defaultValue = "50",
       paramLabel = "<INTEGER>",
       description =
           "Distance from initial chain head to fast sync target (default: ${DEFAULT-VALUE})")
@@ -188,15 +180,13 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = FULL_VALIDATION_RATE_FLAG,
       hidden = true,
-      defaultValue = "0.1",
       paramLabel = "<FLOAT>",
       description = "Fraction of headers fast sync will fully validate (default: ${DEFAULT-VALUE})")
   private float fastSyncFullValidationRate = SynchronizerConfiguration.DEFAULT_FULL_VALIDATION_RATE;
 
   @CommandLine.Option(
       names = WORLD_STATE_HASH_COUNT_PER_REQUEST_FLAG,
       hidden = true,
-      defaultValue = "384",
       paramLabel = "<INTEGER>",
       description = "Fast sync world state hashes queried per request (default: ${DEFAULT-VALUE})")
   private int worldStateHashCountPerRequest =
@@ -205,7 +195,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = WORLD_STATE_REQUEST_PARALLELISM_FLAG,
       hidden = true,
-      defaultValue = "10",
       paramLabel = "<INTEGER>",
       description =
           "Number of concurrent requests to use when downloading fast sync world state (default: ${DEFAULT-VALUE})")
@@ -215,7 +204,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = WORLD_STATE_MAX_REQUESTS_WITHOUT_PROGRESS_FLAG,
       hidden = true,
-      defaultValue = "1000",
       paramLabel = "<INTEGER>",
       description =
           "Number of world state requests accepted without progress before considering the download stalled (default: ${DEFAULT-VALUE})")
@@ -225,7 +213,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = WORLD_STATE_MIN_MILLIS_BEFORE_STALLING_FLAG,
       hidden = true,
-      defaultValue = "300000",
       paramLabel = "<LONG>",
       description =
           "Minimum time in ms without progress before considering a world state download as stalled (default: ${DEFAULT-VALUE})")
@@ -235,7 +222,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = WORLD_STATE_TASK_CACHE_SIZE_FLAG,
       hidden = true,
-      defaultValue = "1000000",
       paramLabel = "<INTEGER>",
       description =
           "The max number of pending node data requests cached in-memory during fast sync world state download. (default: ${DEFAULT-VALUE})")
@@ -245,7 +231,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_PIVOT_BLOCK_WINDOW_VALIDITY_FLAG,
       hidden = true,
-      defaultValue = "126",
       paramLabel = "<INTEGER>",
       description =
           "The size of the pivot block window before having to change it (default: ${DEFAULT-VALUE})")
@@ -255,7 +240,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_PIVOT_BLOCK_DISTANCE_BEFORE_CACHING_FLAG,
       hidden = true,
-      defaultValue = "60",
       paramLabel = "<INTEGER>",
       description =
           "The distance from the head before loading a pivot block into the cache to have a ready pivot block when the window is finished (default: ${DEFAULT-VALUE})")
@@ -265,7 +249,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_STORAGE_COUNT_PER_REQUEST_FLAG,
       hidden = true,
-      defaultValue = "384",
       paramLabel = "<INTEGER>",
       description = "Snap sync storage queried per request (default: ${DEFAULT-VALUE})")
   private int snapsyncStorageCountPerRequest =
@@ -274,7 +257,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_BYTECODE_COUNT_PER_REQUEST_FLAG,
       hidden = true,
-      defaultValue = "84",
       paramLabel = "<INTEGER>",
       description = "Snap sync bytecode queried per request (default: ${DEFAULT-VALUE})")
   private int snapsyncBytecodeCountPerRequest =
@@ -283,7 +265,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_TRIENODE_COUNT_PER_REQUEST_FLAG,
       hidden = true,
-      defaultValue = "384",
       paramLabel = "<INTEGER>",
       description = "Snap sync trie node queried per request (default: ${DEFAULT-VALUE})")
   private int snapsyncTrieNodeCountPerRequest =
@@ -292,7 +273,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_FLAT_ACCOUNT_HEALED_COUNT_PER_REQUEST_FLAG,
       hidden = true,
-      defaultValue = "128",
       paramLabel = "<INTEGER>",
       description =
           "Snap sync flat accounts verified and healed per request (default: ${DEFAULT-VALUE})")
@@ -302,7 +282,6 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_FLAT_STORAGE_HEALED_COUNT_PER_REQUEST_FLAG,
       hidden = true,
-      defaultValue = "1024",
       paramLabel = "<INTEGER>",
       description =
           "Snap sync flat slots verified and healed per request (default: ${DEFAULT-VALUE})")
@@ -312,9 +291,9 @@ public void parseBlockPropagationRange(final String arg) {
   @CommandLine.Option(
       names = SNAP_FLAT_DB_HEALING_ENABLED_FLAG,
       hidden = true,
-      defaultValue = "false",
       paramLabel = "<Boolean>",
-      description = "Snap sync flat db healing enabled (default: ${DEFAULT-VALUE})")
+      description =
+          "(Deprecated) Always enabled: Snap sync flat db healing enabled (default: ${DEFAULT-VALUE})")
   private Boolean snapsyncFlatDbHealingEnabled =
       SnapSyncConfiguration.DEFAULT_IS_FLAT_DB_HEALING_ENABLED;
 
@@ -336,15 +315,6 @@ public static SynchronizerOptions create() {
     return new 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 from Synchronizer Configuration.
    *
@@ -471,15 +441,11 @@ public List<String> getCLIOptions() {
             SNAP_BYTECODE_COUNT_PER_REQUEST_FLAG,
             OptionParser.format(snapsyncBytecodeCountPerRequest),
             SNAP_TRIENODE_COUNT_PER_REQUEST_FLAG,
-            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)));
-    }
+            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));
     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
@@ -3588,29 +3588,12 @@ public void txpoolTxPoolMinGasPriceMustNotBeGreaterThanMinGasPriceZero() {
   }
 
   @Test
-  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() {
+  public void snapsyncForHealingFeaturesShouldFailWhenHealingIsNotSet_EnabledByDefault() {
     parseCommand("--Xsnapsync-synchronizer-flat-account-healed-count-per-request", "100");
-    assertThat(commandErrorOutput.toString(UTF_8))
-        .contains(
-            "--Xsnapsync-synchronizer-flat option can only be used when -Xsnapsync-synchronizer-flat-db-healing-enabled is true");
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
 
     parseCommand("--Xsnapsync-synchronizer-flat-slot-healed-count-per-request", "100");
-    assertThat(commandErrorOutput.toString(UTF_8))
-        .contains(
-            "--Xsnapsync-synchronizer-flat option can only be used when -Xsnapsync-synchronizer-flat-db-healing-enabled is true");
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
   }
 
   @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.FALSE;
+  public static final Boolean DEFAULT_IS_FLAT_DB_HEALING_ENABLED = Boolean.TRUE;
 
   public static SnapSyncConfiguration getDefault() {
     return ImmutableSnapSyncConfiguration.builder().build();