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 <ratan.r.sur@gmail.com>

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),

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 =
+      privacyDatabaseVersion = DatabaseMetadata.lookUpFrom(dataDir).maybePrivacyVersion().orElse(0);
-          DatabaseMetadata.lookUpFrom(databaseDir, 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);

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);
     }

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)
+  public static DatabaseMetadata lookUpFrom(final Path dataDir) throws IOException {
-      throws IOException {
     LOG.info("Lookup database metadata file in data directory: {}", dataDir.toString());
-    File metadataFile = getDefaultMetadataFile(dataDir);
+    return resolveDatabaseMetadata(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);
   }
 
-  private static void writeToDirectory(
+  public void writeToDirectory(final Path dataDir) throws IOException {
-      final DatabaseMetadata databaseMetadata, final Path databaseDir) throws IOException {
     try {
-      final DatabaseMetadata curremtMetadata =
+      final DatabaseMetadata currentMetadata =
-          MAPPER.readValue(getDefaultMetadataFile(databaseDir), DatabaseMetadata.class);
+          MAPPER.readValue(getDefaultMetadataFile(dataDir), DatabaseMetadata.class);
-      if (curremtMetadata.maybePrivacyVersion().isPresent()) {
+      if (currentMetadata.maybePrivacyVersion().isPresent()) {
-        databaseMetadata.setPrivacyVersion(curremtMetadata.getPrivacyVersion());
+        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) {
+  private static File getDefaultMetadataFile(final Path dataDir) {
-    return databaseDir.resolve(METADATA_FILENAME).toFile();
+    return dataDir.resolve(METADATA_FILENAME).toFile();
   }
 
   private static DatabaseMetadata resolveDatabaseMetadata(final File metadataFile)

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(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
-                .maybePrivacyVersion())
         .isNotEmpty();
 
-    assertThat(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
-                .getVersion())
         .isEqualTo(0);
 
     assertThat(
-            DatabaseMetadata.lookUpFrom(
+            DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath())
-                    commonConfiguration.getStoragePath(), 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(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
-                .maybePrivacyVersion())
         .isNotEmpty();
 
-    assertThat(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
-                .getVersion())
         .isEqualTo(DEFAULT_VERSION);
 
     assertThat(
-            DatabaseMetadata.lookUpFrom(
+            DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath())
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
                 .maybePrivacyVersion()
                 .get())
         .isEqualTo(DEFAULT_PRIVACY_VERSION);
@@ -140,16 +123,10 @@ public class RocksDBKeyValuePrivacyStorageFactoryTest {
 
     storageFactory.create(segment, commonConfiguration, metricsSystem);
 
-    assertThat(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
-                .maybePrivacyVersion())
         .isEmpty();
 
-    assertThat(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
-                .getVersion())
         .isEqualTo(DEFAULT_VERSION);
 
     final RocksDBKeyValuePrivacyStorageFactory privacyStorageFactory =
@@ -157,15 +134,11 @@ public class RocksDBKeyValuePrivacyStorageFactoryTest {
 
     privacyStorageFactory.create(segment, commonConfiguration, metricsSystem);
 
-    assertThat(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).maybePrivacyVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
-                .maybePrivacyVersion())
         .isNotEmpty();
 
     assertThat(
-            DatabaseMetadata.lookUpFrom(
+            DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath())
-                    commonConfiguration.getStoragePath(), commonConfiguration.getDataPath())
                 .maybePrivacyVersion()
                 .get())
         .isEqualTo(DEFAULT_PRIVACY_VERSION);

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(
+    assertThat(DatabaseMetadata.lookUpFrom(commonConfiguration.getDataPath()).getVersion())
-            DatabaseMetadata.lookUpFrom(
-                    commonConfiguration.getStoragePath(), 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())
+    assertThat(DatabaseMetadata.lookUpFrom(tempDataDir).getVersion()).isEqualTo(DEFAULT_VERSION);
-        .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(
             () ->

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 =
+    final DatabaseMetadata databaseMetadata = DatabaseMetadata.lookUpFrom(tempDataDir);
-        DatabaseMetadata.lookUpFrom(tempDatabaseDir, 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(tempDataDir);
-    final DatabaseMetadata databaseMetadata =
-        DatabaseMetadata.lookUpFrom(tempDatabaseDir, 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);