From 56a746b26366bff58c38da25213e7eb7800ea77f Mon Sep 17 00:00:00 2001 From: Gabriel Fukushima Date: Wed, 24 Jul 2024 11:38:00 +1000 Subject: [PATCH] Tide up state pruner validation (#8459) * Remove 0 from the possible range of state pruner retention * Add validation of Database version * Replace warn log by InvalidConfigurationException * Add test for invalid slots retained Signed-off-by: Gabriel Fukushima --------- Signed-off-by: Gabriel Fukushima --- .../services/chainstorage/StorageService.java | 43 +++++++++++-------- .../storage/server/StorageConfiguration.java | 4 +- .../server/StorageConfigurationTest.java | 15 +++++-- 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/services/chainstorage/src/main/java/tech/pegasys/teku/services/chainstorage/StorageService.java b/services/chainstorage/src/main/java/tech/pegasys/teku/services/chainstorage/StorageService.java index fc9ca9f5137..6a1e4e9bb39 100644 --- a/services/chainstorage/src/main/java/tech/pegasys/teku/services/chainstorage/StorageService.java +++ b/services/chainstorage/src/main/java/tech/pegasys/teku/services/chainstorage/StorageService.java @@ -24,6 +24,7 @@ import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.async.eventthread.AsyncRunnerEventThread; import tech.pegasys.teku.infrastructure.events.EventChannels; +import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; import tech.pegasys.teku.infrastructure.metrics.SettableLabelledGauge; import tech.pegasys.teku.infrastructure.metrics.TekuMetricCategory; import tech.pegasys.teku.service.serviceutils.Service; @@ -36,6 +37,7 @@ import tech.pegasys.teku.storage.server.ChainStorage; import tech.pegasys.teku.storage.server.CombinedStorageChannelSplitter; import tech.pegasys.teku.storage.server.Database; +import tech.pegasys.teku.storage.server.DatabaseVersion; import tech.pegasys.teku.storage.server.DepositStorage; import tech.pegasys.teku.storage.server.RetryingStorageUpdateChannel; import tech.pegasys.teku.storage.server.StorageConfiguration; @@ -117,24 +119,29 @@ protected SafeFuture doStart() { pruningActiveLabelledGauge)); } if (config.getDataStorageMode().storesFinalizedStates() - && config.getRetainedSlots() > -1) { - LOG.info( - "State pruner will run every: {} minute(s), retaining states for the last {} finalized slots. Limited to {} state prune per execution. ", - config.getStatePruningInterval().toMinutes(), - config.getRetainedSlots(), - config.getStatePruningLimit()); - statePruner = - Optional.of( - new StatePruner( - config.getSpec(), - database, - storagePrunerAsyncRunner, - config.getStatePruningInterval(), - config.getRetainedSlots(), - config.getStatePruningLimit(), - "state", - pruningTimingsLabelledGauge, - pruningActiveLabelledGauge)); + && config.getRetainedSlots() > 0) { + if (config.getDataStorageCreateDbVersion() == DatabaseVersion.LEVELDB_TREE) { + throw new InvalidConfigurationException( + "State pruning is not supported with leveldb_tree database."); + } else { + LOG.info( + "State pruner will run every: {} minute(s), retaining states for the last {} finalized slots. Limited to {} state prune per execution. ", + config.getStatePruningInterval().toMinutes(), + config.getRetainedSlots(), + config.getStatePruningLimit()); + statePruner = + Optional.of( + new StatePruner( + config.getSpec(), + database, + storagePrunerAsyncRunner, + config.getStatePruningInterval(), + config.getRetainedSlots(), + config.getStatePruningLimit(), + "state", + pruningTimingsLabelledGauge, + pruningActiveLabelledGauge)); + } } if (config.getSpec().isMilestoneSupported(SpecMilestone.DENEB)) { blobsPruner = diff --git a/storage/src/main/java/tech/pegasys/teku/storage/server/StorageConfiguration.java b/storage/src/main/java/tech/pegasys/teku/storage/server/StorageConfiguration.java index 846b3d4df93..a413e6e66c7 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/server/StorageConfiguration.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/server/StorageConfiguration.java @@ -39,7 +39,7 @@ public class StorageConfiguration { public static final int DEFAULT_BLOCK_PRUNING_LIMIT = 5000; public static final Duration DEFAULT_BLOBS_PRUNING_INTERVAL = Duration.ofMinutes(1); public static final Duration DEFAULT_STATE_PRUNING_INTERVAL = Duration.ofMinutes(5); - public static final long DEFAULT_STORAGE_RETAINED_SLOTS = -1; + public static final long DEFAULT_STORAGE_RETAINED_SLOTS = 0; public static final int DEFAULT_STATE_PRUNING_LIMIT = 1; // 60/12 = 5 blocks per minute * 6 max blobs per block = 30 blobs per minute at maximum, 15 as @@ -275,7 +275,7 @@ public Builder blobsPruningLimit(final int blobsPruningLimit) { } public Builder retainedSlots(final long retainedSlots) { - if (retainedSlots < -1) { + if (retainedSlots < 0) { throw new InvalidConfigurationException( "Invalid number of slots to retain finalized states for"); } diff --git a/storage/src/test/java/tech/pegasys/teku/storage/server/StorageConfigurationTest.java b/storage/src/test/java/tech/pegasys/teku/storage/server/StorageConfigurationTest.java index 6c788c7f904..f6755b87bfa 100644 --- a/storage/src/test/java/tech/pegasys/teku/storage/server/StorageConfigurationTest.java +++ b/storage/src/test/java/tech/pegasys/teku/storage/server/StorageConfigurationTest.java @@ -109,7 +109,16 @@ public void shouldSucceedIfDatabaseStorageModeFileIsInvalidAndExplicitOptionIsSe } @Test - public void shouldFailIfUserConfiguresStatePrunerIntervalTooLow() { + public void shouldFailIfUserConfiguresStatePruneSlotsRetainedLowerThanAllowed() { + + assertThatThrownBy( + () -> StorageConfiguration.builder().dataStorageMode(ARCHIVE).retainedSlots(-1)) + .isInstanceOf(InvalidConfigurationException.class) + .hasMessageContaining("Invalid number of slots to retain finalized states for"); + } + + @Test + public void shouldFailIfUserConfiguresStatePrunerLowerThanAllowed() { assertThatThrownBy( () -> @@ -122,7 +131,7 @@ public void shouldFailIfUserConfiguresStatePrunerIntervalTooLow() { } @Test - public void shouldFailIfUserConfiguresStatePrunerIntervalTooHigh() { + public void shouldFailIfUserConfiguresStatePrunerIntervalGreaterThanAllowed() { assertThatThrownBy( () -> @@ -135,7 +144,7 @@ public void shouldFailIfUserConfiguresStatePrunerIntervalTooHigh() { } @Test - public void shouldFailIfUserConfiguresStatePrunerLimitTooHigh() { + public void shouldFailIfUserConfiguresStatePrunerLimitGreaterThanAllowed() { assertThatThrownBy( () -> StorageConfiguration.builder().dataStorageMode(ARCHIVE).statePruningLimit(101))