From 739b3382f43e2fc07cf67a75dc9102e028b74480 Mon Sep 17 00:00:00 2001 From: "Ratan (Rai) Sur" Date: Tue, 16 Jun 2020 10:36:52 -0400 Subject: [PATCH] Unsupport Misplaced Database Metadata (#1060) Way back in the pantheon days, I put the DATABASE_METADATA.json file in the database directory. This was a bad idea because rocksdb owns that directory. We changed to putting it in the directory we own, data, but we needed to support the old way of doing things. The time to support that old location has well and truly past so now we're safe to stop looking in that directory. Signed-off-by: Ratan Rai Sur --- .../DatabaseMigrationAcceptanceTest.java | 6 +-- .../RocksDBKeyValuePrivacyStorageFactory.java | 10 ++-- .../RocksDBKeyValueStorageFactory.java | 6 +-- .../configuration/DatabaseMetadata.java | 42 +++++------------ ...ksDBKeyValuePrivacyStorageFactoryTest.java | 47 ++++--------------- .../RocksDBKeyValueStorageFactoryTest.java | 18 ++----- .../configuration/DatabaseMetadataTest.java | 26 +--------- 7 files changed, 36 insertions(+), 119 deletions(-) diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/database/DatabaseMigrationAcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/database/DatabaseMigrationAcceptanceTest.java index 0ebd5feefa..8b201a14ae 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/database/DatabaseMigrationAcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/database/DatabaseMigrationAcceptanceTest.java @@ -16,6 +16,7 @@ package org.hyperledger.besu.tests.acceptance.database; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; +import static java.util.Collections.singletonList; import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.core.Wei; @@ -34,7 +35,6 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.stream.Stream; @@ -79,7 +79,7 @@ public class DatabaseMigrationAcceptanceTest extends AcceptanceTestBase { "Before versioning was enabled", "version0", 0xA, - Arrays.asList( + singletonList( new AccountData( "0xd1aeb42885a43b72b518182ef893125814811048", BigInteger.valueOf(0xA), @@ -89,7 +89,7 @@ public class DatabaseMigrationAcceptanceTest extends AcceptanceTestBase { "After versioning was enabled and using multiple RocksDB columns", "version1", 0xA, - Arrays.asList( + singletonList( new AccountData( "0xd1aeb42885a43b72b518182ef893125814811048", BigInteger.valueOf(0xA), diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactory.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactory.java index 13ec7c317f..b368e50830 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactory.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactory.java @@ -86,15 +86,12 @@ public class RocksDBKeyValuePrivacyStorageFactory implements PrivacyKeyValueStor * use the default version */ private int readDatabaseVersion(final BesuConfiguration commonConfiguration) throws IOException { - final Path privacyDatabaseDir = - commonConfiguration.getStoragePath().resolve(PRIVATE_DATABASE_PATH); - final Path databaseDir = commonConfiguration.getStoragePath(); final Path dataDir = commonConfiguration.getDataPath(); - final boolean privacyDatabaseExists = privacyDatabaseDir.resolve("IDENTITY").toFile().exists(); + final boolean privacyDatabaseExists = + commonConfiguration.getStoragePath().resolve(PRIVATE_DATABASE_PATH).toFile().exists(); final int privacyDatabaseVersion; if (privacyDatabaseExists) { - privacyDatabaseVersion = - DatabaseMetadata.lookUpFrom(databaseDir, dataDir).maybePrivacyVersion().orElse(0); + privacyDatabaseVersion = DatabaseMetadata.lookUpFrom(dataDir).maybePrivacyVersion().orElse(0); LOG.info( "Existing private database detected at {}. Version {}", dataDir, privacyDatabaseVersion); } else { @@ -103,7 +100,6 @@ public class RocksDBKeyValuePrivacyStorageFactory implements PrivacyKeyValueStor "No existing private database detected at {}. Using version {}", dataDir, privacyDatabaseVersion); - Files.createDirectories(privacyDatabaseDir); Files.createDirectories(dataDir); new DatabaseMetadata(publicFactory.getDefaultVersion(), privacyDatabaseVersion) .writeToDirectory(dataDir); diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java index 570317976b..21e05c4cbe 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java @@ -154,17 +154,15 @@ public class RocksDBKeyValueStorageFactory implements KeyValueStorageFactory { } private int readDatabaseVersion(final BesuConfiguration commonConfiguration) throws IOException { - final Path databaseDir = commonConfiguration.getStoragePath(); final Path dataDir = commonConfiguration.getDataPath(); - final boolean databaseExists = databaseDir.resolve("IDENTITY").toFile().exists(); + final boolean databaseExists = commonConfiguration.getStoragePath().toFile().exists(); final int databaseVersion; if (databaseExists) { - databaseVersion = DatabaseMetadata.lookUpFrom(databaseDir, dataDir).getVersion(); + databaseVersion = DatabaseMetadata.lookUpFrom(dataDir).getVersion(); LOG.info("Existing database detected at {}. Version {}", dataDir, databaseVersion); } else { databaseVersion = defaultVersion; LOG.info("No existing database detected at {}. Using version {}", dataDir, databaseVersion); - Files.createDirectories(databaseDir); Files.createDirectories(dataDir); new DatabaseMetadata(databaseVersion).writeToDirectory(dataDir); } diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java index 3ef5e124ba..63214c7cbb 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadata.java @@ -72,46 +72,26 @@ public class DatabaseMetadata { return privacyVersion; } - public static DatabaseMetadata lookUpFrom(final Path databaseDir, final Path dataDir) - throws IOException { + public static DatabaseMetadata lookUpFrom(final Path dataDir) throws IOException { LOG.info("Lookup database metadata file in data directory: {}", dataDir.toString()); - File metadataFile = getDefaultMetadataFile(dataDir); - final boolean shouldLookupInDatabaseDir = !metadataFile.exists(); - if (shouldLookupInDatabaseDir) { - LOG.info( - "Database metadata file not found in data directory. Lookup in database directory: {}", - databaseDir.toString()); - metadataFile = getDefaultMetadataFile(databaseDir); - } - final DatabaseMetadata databaseMetadata = resolveDatabaseMetadata(metadataFile); - if (shouldLookupInDatabaseDir) { - LOG.warn( - "Database metadata file has been copied from old location (database directory). Be aware that the old file might be removed in future release."); - writeToDirectory(databaseMetadata, dataDir); - } - return databaseMetadata; - } - - public void writeToDirectory(final Path databaseDir) throws IOException { - writeToDirectory(this, databaseDir); + return resolveDatabaseMetadata(getDefaultMetadataFile(dataDir)); } - private static void writeToDirectory( - final DatabaseMetadata databaseMetadata, final Path databaseDir) throws IOException { + public void writeToDirectory(final Path dataDir) throws IOException { try { - final DatabaseMetadata curremtMetadata = - MAPPER.readValue(getDefaultMetadataFile(databaseDir), DatabaseMetadata.class); - if (curremtMetadata.maybePrivacyVersion().isPresent()) { - databaseMetadata.setPrivacyVersion(curremtMetadata.getPrivacyVersion()); + final DatabaseMetadata currentMetadata = + MAPPER.readValue(getDefaultMetadataFile(dataDir), DatabaseMetadata.class); + if (currentMetadata.maybePrivacyVersion().isPresent()) { + setPrivacyVersion(currentMetadata.getPrivacyVersion()); } - MAPPER.writeValue(getDefaultMetadataFile(databaseDir), databaseMetadata); + MAPPER.writeValue(getDefaultMetadataFile(dataDir), this); } catch (FileNotFoundException fnfe) { - MAPPER.writeValue(getDefaultMetadataFile(databaseDir), databaseMetadata); + MAPPER.writeValue(getDefaultMetadataFile(dataDir), this); } } - private static File getDefaultMetadataFile(final Path databaseDir) { - return databaseDir.resolve(METADATA_FILENAME).toFile(); + private static File getDefaultMetadataFile(final Path dataDir) { + return dataDir.resolve(METADATA_FILENAME).toFile(); } private static DatabaseMetadata resolveDatabaseMetadata(final File metadataFile) diff --git a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactoryTest.java b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactoryTest.java index 17948ded45..220594249e 100644 --- a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactoryTest.java +++ b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValuePrivacyStorageFactoryTest.java @@ -37,7 +37,6 @@ import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) public class RocksDBKeyValuePrivacyStorageFactoryTest { - // private static final String METADATA_FILENAME = "DATABASE_METADATA.json"; private static final int DEFAULT_VERSION = 1; private static final int DEFAULT_PRIVACY_VERSION = 1; @@ -55,8 +54,6 @@ public class RocksDBKeyValuePrivacyStorageFactoryTest { final Path tempPrivateDatabaseDir = tempDatabaseDir.resolve("private"); Files.createDirectories(tempPrivateDatabaseDir); Files.createDirectories(tempDataDir); - Files.createFile(tempPrivateDatabaseDir.resolve("IDENTITY")); - Files.createFile(tempDatabaseDir.resolve("IDENTITY")); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); @@ -70,21 +67,14 @@ public class RocksDBKeyValuePrivacyStorageFactoryTest { // Side effect is creation of the Metadata version file storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .maybePrivacyVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion()) .isNotEmpty(); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .getVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion()) .isEqualTo(0); assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) + DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()) .maybePrivacyVersion() .get()) .isEqualTo(0); @@ -107,21 +97,14 @@ public class RocksDBKeyValuePrivacyStorageFactoryTest { // Side effect is creation of the Metadata version file storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .maybePrivacyVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion()) .isNotEmpty(); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .getVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion()) .isEqualTo(DEFAULT_VERSION); assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) + DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()) .maybePrivacyVersion() .get()) .isEqualTo(DEFAULT_PRIVACY_VERSION); @@ -140,16 +123,10 @@ public class RocksDBKeyValuePrivacyStorageFactoryTest { storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .maybePrivacyVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion()) .isEmpty(); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .getVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion()) .isEqualTo(DEFAULT_VERSION); final RocksDBKeyValuePrivacyStorageFactory privacyStorageFactory = @@ -157,15 +134,11 @@ public class RocksDBKeyValuePrivacyStorageFactoryTest { privacyStorageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .maybePrivacyVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion()) .isNotEmpty(); assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) + DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()) .maybePrivacyVersion() .get()) .isEqualTo(DEFAULT_PRIVACY_VERSION); diff --git a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java index 6ea9cc1a0a..2343c9aaf0 100644 --- a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java +++ b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactoryTest.java @@ -66,10 +66,7 @@ public class RocksDBKeyValueStorageFactoryTest { // Side effect is creation of the Metadata version file storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat( - DatabaseMetadata.lookUpFrom( - commonConfiguration.getStoragePath(), commonConfiguration.getDataPath()) - .getVersion()) + assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion()) .isEqualTo(DEFAULT_VERSION); } @@ -79,7 +76,6 @@ public class RocksDBKeyValueStorageFactoryTest { final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); Files.createDirectories(tempDataDir); - tempDatabaseDir.resolve("IDENTITY").toFile().createNewFile(); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); @@ -89,14 +85,13 @@ public class RocksDBKeyValueStorageFactoryTest { storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat(DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir).getVersion()).isZero(); + assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersion()).isZero(); } @Test public void shouldDetectCorrectVersionIfMetadataFileExists() throws Exception { final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); - Files.createDirectories(tempDatabaseDir); Files.createDirectories(tempDataDir); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); @@ -107,8 +102,7 @@ public class RocksDBKeyValueStorageFactoryTest { storageFactory.create(segment, commonConfiguration, metricsSystem); - assertThat(DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir).getVersion()) - .isEqualTo(DEFAULT_VERSION); + assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersion()).isEqualTo(DEFAULT_VERSION); assertThat(storageFactory.isSegmentIsolationSupported()).isTrue(); } @@ -140,10 +134,9 @@ public class RocksDBKeyValueStorageFactoryTest { final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); Files.createDirectories(tempDataDir); - tempDatabaseDir.resolve("IDENTITY").toFile().createNewFile(); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); - new DatabaseMetadata(-1).writeToDirectory(tempDatabaseDir); + new DatabaseMetadata(-1).writeToDirectory(tempDataDir); assertThatThrownBy( () -> new RocksDBKeyValueStorageFactory( @@ -176,13 +169,12 @@ public class RocksDBKeyValueStorageFactoryTest { final Path tempDatabaseDir = temporaryFolder.newFolder().toPath().resolve("db"); Files.createDirectories(tempDatabaseDir); Files.createDirectories(tempDataDir); - tempDatabaseDir.resolve("IDENTITY").toFile().createNewFile(); when(commonConfiguration.getStoragePath()).thenReturn(tempDatabaseDir); when(commonConfiguration.getDataPath()).thenReturn(tempDataDir); final String badVersion = "{\"🦄\":1}"; Files.write( - tempDatabaseDir.resolve(METADATA_FILENAME), badVersion.getBytes(Charset.defaultCharset())); + tempDataDir.resolve(METADATA_FILENAME), badVersion.getBytes(Charset.defaultCharset())); assertThatThrownBy( () -> diff --git a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java index 902e4508db..b060b022d1 100644 --- a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java +++ b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/configuration/DatabaseMetadataTest.java @@ -16,9 +16,6 @@ package org.hyperledger.besu.plugin.services.storage.rocksdb.configuration; import static org.assertj.core.api.Assertions.assertThat; -import static org.hyperledger.besu.plugin.services.helper.Conditions.FILE_DOES_NOT_EXIST; -import static org.hyperledger.besu.plugin.services.helper.Conditions.FILE_EXISTS; -import static org.hyperledger.besu.plugin.services.helper.Conditions.shouldContain; import java.io.IOException; import java.nio.file.Files; @@ -43,10 +40,8 @@ public class DatabaseMetadataTest { final Path tempDataDir = createAndWrite( "data", "DATABASE_METADATA.json", "{\"version\":42 , \"privacyVersion\":55}"); - final Path tempDatabaseDir = createAndWrite("db", "DATABASE_METADATA.json", "{\"version\":99}"); - final DatabaseMetadata databaseMetadata = - DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir); + final DatabaseMetadata databaseMetadata = DatabaseMetadata.lookUpFrom(tempDataDir); assertThat(databaseMetadata).isNotNull(); assertThat(databaseMetadata.getVersion()).isEqualTo(42); assertThat(databaseMetadata.maybePrivacyVersion()).isNotEmpty(); @@ -56,28 +51,11 @@ public class DatabaseMetadataTest { @Test public void metaFileShouldBeSoughtIntoDataDirFirst() throws Exception { final Path tempDataDir = createAndWrite("data", "DATABASE_METADATA.json", "{\"version\":42}"); - final Path tempDatabaseDir = createAndWrite("db", "DATABASE_METADATA.json", "{\"version\":99}"); - final DatabaseMetadata databaseMetadata = - DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir); + final DatabaseMetadata databaseMetadata = DatabaseMetadata.lookUpFrom(tempDataDir); assertThat(databaseMetadata).isNotNull(); assertThat(databaseMetadata.getVersion()).isEqualTo(42); } - @Test - public void metaFileNotFoundInDataDirShouldLookupIntoDbDir() throws Exception { - final Path tempDataDir = temporaryFolder.newFolder().toPath().resolve("data"); - Files.createDirectories(tempDataDir); - final Path tempDatabaseDir = createAndWrite("db", "DATABASE_METADATA.json", "{\"version\":42}"); - final Path metadataPathInDataDir = tempDataDir.resolve("DATABASE_METADATA.json"); - assertThat(metadataPathInDataDir).is(FILE_DOES_NOT_EXIST); - final DatabaseMetadata databaseMetadata = - DatabaseMetadata.lookUpFrom(tempDatabaseDir, tempDataDir); - assertThat(databaseMetadata).isNotNull(); - assertThat(databaseMetadata.getVersion()).isEqualTo(42); - assertThat(metadataPathInDataDir).is(FILE_EXISTS); - assertThat(metadataPathInDataDir).is(shouldContain("{\"version\":42}")); - } - private Path createAndWrite(final String dir, final String file, final String content) throws IOException { return createAndWrite(temporaryFolder, dir, file, content);