Skip to content

Commit

Permalink
Added implicit PRUNE mode warning & Handling invalid data-storage-mod…
Browse files Browse the repository at this point in the history
…e file (#8369)
  • Loading branch information
lucassaldanha authored Jun 18, 2024
1 parent 0060e81 commit 437da16
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<StateStorageMode> 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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -267,39 +264,36 @@ public StorageConfiguration build() {
private void determineDataStorageMode() {
if (dataConfig != null) {
final DataDirLayout dataDirLayout = DataDirLayout.createFrom(dataConfig);
final Path beaconDataDirectory = dataDirLayout.getBeaconDataDirectory();

Optional<StateStorageMode> 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;
}
}
}

private Optional<StateStorageMode> 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);
Expand All @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<StateStorageMode> 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arguments> getStateStorageDefaultScenarios() {
ArrayList<Arguments> args = new ArrayList<>();
args.add(Arguments.of(false, Optional.empty(), NOT_SET, MINIMAL));
Expand Down Expand Up @@ -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));
}
}

0 comments on commit 437da16

Please sign in to comment.