From 437da16de0d7a4ef05fde778c0e3d4a343514877 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Tue, 18 Jun 2024 18:27:01 +1200 Subject: [PATCH] Added implicit PRUNE mode warning & Handling invalid data-storage-mode file (#8369) --- CHANGELOG.md | 1 + .../infrastructure/logging/StatusLogger.java | 9 +++ .../server/DatabaseStorageModeFileHelper.java | 48 ++++++++++++++ .../storage/server/StorageConfiguration.java | 60 +++++++++-------- .../server/VersionedDatabaseFactory.java | 25 +------ .../DatabaseStorageModeFileHelperTest.java | 65 +++++++++++++++++++ .../server/StorageConfigurationTest.java | 52 +++++++++++++++ 7 files changed, 210 insertions(+), 50 deletions(-) create mode 100644 storage/src/main/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelper.java create mode 100644 storage/src/test/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelperTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 967670c2592..a9c017d9e20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ We recommend all users of the `Teku - Detailed` dashboard to upgrade to version [Revision 12](https://grafana.com/api/dashboards/16737/revisions/12/download) as soon as possible. Documentation with all metrics that have been renamed will be provided. - Next release will require Java 21. The current release is compatible, please consider upgrading before the next release. +- From the next release, you will need to explicitly set `--data-storage-mode=(prune|archive)` unless you're using minimal data-storage-mode (which is the default behaviour). ## Current Releases diff --git a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java index 1083cfa1f0d..863e7d4af1e 100644 --- a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java +++ b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java @@ -532,6 +532,15 @@ public void warnIgnoringWeakSubjectivityPeriod() { Color.YELLOW)); } + public void warnUsageOfImplicitPruneDataStorageMode() { + log.warn( + print( + "Prune mode being used as default without a explicit --data-storage-mode option. This will NOT be " + + "supported in future Teku versions. Please add --data-storage-mode=prune to your CLI arguments" + + " or config file if you want to keep using PRUNE.", + Color.YELLOW)); + } + private void logWithColorIfLevelGreaterThanInfo( final Level level, final String msg, final ColorConsolePrinter.Color color) { final boolean useColor = level.compareTo(Level.INFO) < 0; diff --git a/storage/src/main/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelper.java b/storage/src/main/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelper.java new file mode 100644 index 00000000000..56147650ec8 --- /dev/null +++ b/storage/src/main/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelper.java @@ -0,0 +1,48 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.storage.server; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +public class DatabaseStorageModeFileHelper { + + private static final Logger LOG = LogManager.getLogger(); + + public static Optional readStateStorageMode(final Path path) { + if (!Files.exists(path)) { + return Optional.empty(); + } + + try { + final StateStorageMode dbStorageMode = + StateStorageMode.valueOf(Files.readString(path).trim()); + LOG.debug("Read previous storage mode as {}", dbStorageMode); + return Optional.of(dbStorageMode); + } catch (final IllegalArgumentException ex) { + throw DatabaseStorageException.unrecoverable( + "Invalid database storage mode file (" + + path + + "). Run your node using '--data-storage-mode' option to configure the correct storage mode.", + ex); + } catch (final IOException ex) { + throw new UncheckedIOException("Failed to read storage mode from file", ex); + } + } +} 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 a573f6d522d..b19d00db4cb 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 @@ -13,14 +13,12 @@ package tech.pegasys.teku.storage.server; +import static tech.pegasys.teku.infrastructure.logging.StatusLogger.STATUS_LOG; import static tech.pegasys.teku.storage.server.StateStorageMode.MINIMAL; import static tech.pegasys.teku.storage.server.StateStorageMode.NOT_SET; import static tech.pegasys.teku.storage.server.StateStorageMode.PRUNE; import static tech.pegasys.teku.storage.server.VersionedDatabaseFactory.STORAGE_MODE_PATH; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; import java.util.Optional; @@ -33,7 +31,6 @@ import tech.pegasys.teku.spec.Spec; public class StorageConfiguration { - public static final boolean DEFAULT_STORE_NON_CANONICAL_BLOCKS_ENABLED = false; public static final int DEFAULT_STATE_REBUILD_TIMEOUT_SECONDS = 120; public static final long DEFAULT_STORAGE_FREQUENCY = 2048L; @@ -267,11 +264,24 @@ public StorageConfiguration build() { private void determineDataStorageMode() { if (dataConfig != null) { final DataDirLayout dataDirLayout = DataDirLayout.createFrom(dataConfig); + final Path beaconDataDirectory = dataDirLayout.getBeaconDataDirectory(); + + Optional storageModeFromStoredFile; + try { + storageModeFromStoredFile = + DatabaseStorageModeFileHelper.readStateStorageMode( + beaconDataDirectory.resolve(STORAGE_MODE_PATH)); + } catch (final DatabaseStorageException e) { + if (dataStorageMode == NOT_SET) { + throw e; + } else { + storageModeFromStoredFile = Optional.empty(); + } + } + this.dataStorageMode = determineStorageDefault( - dataDirLayout.getBeaconDataDirectory().toFile().exists(), - getStorageModeFromPersistedDatabase(dataDirLayout), - dataStorageMode); + beaconDataDirectory.toFile().exists(), storageModeFromStoredFile, dataStorageMode); } else { if (dataStorageMode.equals(NOT_SET)) { dataStorageMode = PRUNE; @@ -279,27 +289,11 @@ private void determineDataStorageMode() { } } - private Optional getStorageModeFromPersistedDatabase( - final DataDirLayout dataDirLayout) { - final Path dbStorageModeFile = - dataDirLayout.getBeaconDataDirectory().resolve(STORAGE_MODE_PATH); - if (!Files.exists(dbStorageModeFile)) { - return Optional.empty(); - } - try { - final StateStorageMode dbStorageMode = - StateStorageMode.valueOf(Files.readString(dbStorageModeFile).trim()); - LOG.debug("Read previous storage mode as {}", dbStorageMode); - return Optional.of(dbStorageMode); - } catch (final IOException ex) { - throw new UncheckedIOException("Failed to read storage mode from file", ex); - } - } - public Builder stateRebuildTimeoutSeconds(final int stateRebuildTimeoutSeconds) { if (stateRebuildTimeoutSeconds < 10 || stateRebuildTimeoutSeconds > 300) { LOG.warn( - "State rebuild timeout is set outside of sensible defaults of 10 -> 300, {} was defined. Cannot be below 1, will allow the value to exceed 300.", + "State rebuild timeout is set outside of sensible defaults of 10 -> 300, {} was defined. Cannot be below " + + "1, will allow the value to exceed 300.", stateRebuildTimeoutSeconds); } this.stateRebuildTimeoutSeconds = Math.max(stateRebuildTimeoutSeconds, 1); @@ -315,6 +309,20 @@ static StateStorageMode determineStorageDefault( if (modeRequested != NOT_SET) { return modeRequested; } - return maybeHistoricStorageMode.orElse(isExistingStore ? PRUNE : MINIMAL); + + if (maybeHistoricStorageMode.isPresent()) { + final StateStorageMode stateStorageMode = maybeHistoricStorageMode.get(); + if (stateStorageMode == PRUNE) { + STATUS_LOG.warnUsageOfImplicitPruneDataStorageMode(); + } + return stateStorageMode; + } + + if (isExistingStore) { + STATUS_LOG.warnUsageOfImplicitPruneDataStorageMode(); + return PRUNE; + } else { + return MINIMAL; + } } } diff --git a/storage/src/main/java/tech/pegasys/teku/storage/server/VersionedDatabaseFactory.java b/storage/src/main/java/tech/pegasys/teku/storage/server/VersionedDatabaseFactory.java index 429be0d0053..16cac554b7a 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/server/VersionedDatabaseFactory.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/server/VersionedDatabaseFactory.java @@ -20,7 +20,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; -import java.util.Optional; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes; @@ -82,29 +81,7 @@ public VersionedDatabaseFactory( this.dbStorageModeFile = this.dataDirectory.toPath().resolve(STORAGE_MODE_PATH).toFile(); dbSettingFileSyncDataAccessor = SyncDataAccessor.create(dataDirectory.toPath()); - this.stateStorageMode = - getStateStorageModeFromConfigOrDisk(Optional.of(config.getDataStorageMode())); - } - - private StateStorageMode getStateStorageModeFromConfigOrDisk( - final Optional maybeConfiguredStorageMode) { - try { - final File storageModeFile = this.dataDirectory.toPath().resolve(STORAGE_MODE_PATH).toFile(); - if (storageModeFile.exists() && maybeConfiguredStorageMode.isPresent()) { - final StateStorageMode storageModeFromFile = - StateStorageMode.valueOf(Files.readString(storageModeFile.toPath()).trim()); - if (!storageModeFromFile.equals(maybeConfiguredStorageMode.get())) { - LOG.info( - "Storage mode that was persisted differs from the command specified option; file: {}, CLI: {}", - () -> storageModeFromFile, - maybeConfiguredStorageMode::get); - } - } - } catch (IOException e) { - LOG.error("Failed to read storage mode file", e); - } - - return maybeConfiguredStorageMode.orElse(StateStorageMode.DEFAULT_MODE); + this.stateStorageMode = config.getDataStorageMode(); } @Override diff --git a/storage/src/test/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelperTest.java b/storage/src/test/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelperTest.java new file mode 100644 index 00000000000..d0bd3c2c88d --- /dev/null +++ b/storage/src/test/java/tech/pegasys/teku/storage/server/DatabaseStorageModeFileHelperTest.java @@ -0,0 +1,65 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.storage.server; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.fail; +import static tech.pegasys.teku.storage.server.VersionedDatabaseFactory.STORAGE_MODE_PATH; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class DatabaseStorageModeFileHelperTest { + + @Test + public void shouldReadValidStorageModeFile(@TempDir final Path tempDir) { + final Path dbStorageModeFile = + createDatabaseStorageModeFile(tempDir, StateStorageMode.MINIMAL.toString()); + assertThat(DatabaseStorageModeFileHelper.readStateStorageMode(dbStorageModeFile)) + .hasValue(StateStorageMode.MINIMAL); + } + + @Test + public void shouldReadEmptyWhenFileDoesNotExist(@TempDir final Path tempDir) { + final Path dbStorageModeFile = tempDir.resolve("foo"); + assertThat(DatabaseStorageModeFileHelper.readStateStorageMode(dbStorageModeFile)).isEmpty(); + } + + @Test + public void shouldThrowErrorIfFileHasInvalidValue(@TempDir final Path tempDir) { + final Path dbStorageModeFile = createDatabaseStorageModeFile(tempDir, "hello"); + assertThatThrownBy(() -> DatabaseStorageModeFileHelper.readStateStorageMode(dbStorageModeFile)) + .isInstanceOf(DatabaseStorageException.class); + } + + @Test + public void shouldThrowErrorIfFileIsEmpty(@TempDir final Path tempDir) { + final Path dbStorageModeFile = createDatabaseStorageModeFile(tempDir, ""); + assertThatThrownBy(() -> DatabaseStorageModeFileHelper.readStateStorageMode(dbStorageModeFile)) + .isInstanceOf(DatabaseStorageException.class); + } + + private Path createDatabaseStorageModeFile(final Path path, final String value) { + try { + return Files.writeString(path.resolve(STORAGE_MODE_PATH), value); + } catch (IOException e) { + fail(e); + return null; + } + } +} 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 63de8a8ca57..b766dfa3995 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 @@ -13,21 +13,36 @@ package tech.pegasys.teku.storage.server; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; import static tech.pegasys.teku.storage.server.StateStorageMode.ARCHIVE; import static tech.pegasys.teku.storage.server.StateStorageMode.MINIMAL; import static tech.pegasys.teku.storage.server.StateStorageMode.NOT_SET; import static tech.pegasys.teku.storage.server.StateStorageMode.PRUNE; +import static tech.pegasys.teku.storage.server.VersionedDatabaseFactory.STORAGE_MODE_PATH; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Optional; import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import tech.pegasys.teku.ethereum.execution.types.Eth1Address; +import tech.pegasys.teku.service.serviceutils.layout.DataConfig; +import tech.pegasys.teku.spec.Spec; +import tech.pegasys.teku.spec.TestSpecFactory; public class StorageConfigurationTest { + final Spec spec = TestSpecFactory.createMinimalPhase0(); + final Eth1Address eth1Address = + Eth1Address.fromHexString("0x77f7bED277449F51505a4C54550B074030d989bC"); + public static Stream getStateStorageDefaultScenarios() { ArrayList args = new ArrayList<>(); args.add(Arguments.of(false, Optional.empty(), NOT_SET, MINIMAL)); @@ -58,4 +73,41 @@ void shouldDetermineCorrectStorageModeGivenInputs( isExistingStore, maybePreviousStorageMode, requestedMode)) .isEqualTo(expectedResult); } + + @Test + public void shouldFailIfDatabaseStorageModeFileIsInvalidAndNoExplicitOptionIsSet( + @TempDir final Path dir) throws IOException { + createInvalidStorageModeFile(dir); + final DataConfig dataConfig = DataConfig.builder().beaconDataPath(dir).build(); + + final StorageConfiguration.Builder storageConfigBuilder = + StorageConfiguration.builder() + .specProvider(spec) + .dataConfig(dataConfig) + .eth1DepositContract(eth1Address); + + assertThatThrownBy(storageConfigBuilder::build).isInstanceOf(DatabaseStorageException.class); + } + + @Test + public void shouldSucceedIfDatabaseStorageModeFileIsInvalidAndExplicitOptionIsSet( + @TempDir final Path dir) throws IOException { + createInvalidStorageModeFile(dir); + final DataConfig dataConfig = DataConfig.builder().beaconDataPath(dir).build(); + + final StorageConfiguration storageConfig = + StorageConfiguration.builder() + .specProvider(spec) + .dataConfig(dataConfig) + .eth1DepositContract(eth1Address) + .dataStorageMode(ARCHIVE) + .build(); + + assertThat(storageConfig.getDataStorageMode()).isEqualTo(ARCHIVE); + } + + private static void createInvalidStorageModeFile(final Path dir) throws IOException { + // An empty storage mode path is invalid + Files.createFile(dir.resolve(STORAGE_MODE_PATH)); + } }