[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #982: HDDS-3668. OzoneManager start fails with RocksDB error on downgrade to older version.

[GitBox <gi...@apache.org>]

adoroszlai commented on a change in pull request #982:
URL: https://github.com/apache/hadoop-ozone/pull/982#discussion_r435773972



##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
##########
@@ -149,6 +162,26 @@ public RDBStore(File dbFile, DBOptions options,
     }
   }
 
+  /**
+   * Read DB and return existing column families.
+   * @return List of column families
+   * @throws RocksDBException on Error.
+   */
+  private List<TableConfig> getColumnFamiliesInExistingDb()
+      throws RocksDBException {
+    List<byte[]> bytes = RocksDB.listColumnFamilies(new Options(),
+        dbLocation.getAbsolutePath());
+    List<TableConfig> columnFamiliesInDb = bytes.stream()
+        .map(cfbytes -> new TableConfig(StringUtils.bytes2String(cfbytes),
+            DBProfile.DISK.getColumnFamilyOptions()))
+        .collect(Collectors.toList());
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Found column Families in DB : {}",
+          Arrays.toString(columnFamiliesInDb.toArray()));

Review comment:
       Nit: same here regarding arrays:
   
   ```suggestion
             columnFamiliesInDb);
   ```

##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
##########
@@ -94,6 +98,15 @@ public RDBStore(File dbFile, DBOptions options,
     this.writeOptions = writeOptions;
 
     try {
+      List<TableConfig> columnFamiliesInDb = getColumnFamiliesInExistingDb();
+      List<TableConfig> extraCf = columnFamiliesInDb.stream().filter(
+          cf -> !families.contains(cf)).collect(Collectors.toList());
+      if (CollectionUtils.isNotEmpty(extraCf)) {
+        LOG.info("Found the following extra column families in existing DB : " +
+                "{}", Arrays.toString(extraCf.toArray()));

Review comment:
       Nit: I think `List.toString` is just as good:
   
   ```suggestion
                   "{}", extraCf);
   ```

##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
##########
@@ -94,6 +98,15 @@ public RDBStore(File dbFile, DBOptions options,
     this.writeOptions = writeOptions;
 
     try {
+      List<TableConfig> columnFamiliesInDb = getColumnFamiliesInExistingDb();
+      List<TableConfig> extraCf = columnFamiliesInDb.stream().filter(
+          cf -> !families.contains(cf)).collect(Collectors.toList());
+      if (CollectionUtils.isNotEmpty(extraCf)) {

Review comment:
       Nit: `extraCf` cannot be null, so I would prefer not adding a dependency:
   
   ```suggestion
         if (!extraCf.isEmpty()) {
   ```

##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
##########
@@ -24,11 +24,14 @@
 import java.io.IOException;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
+import org.apache.commons.collections.CollectionUtils;

Review comment:
       Corresponding to suggestion about empty list:
   
   ```suggestion
   ```

##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
##########
@@ -24,11 +24,14 @@
 import java.io.IOException;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;

Review comment:
       Corresponding to suggestion about list to string:
   
   ```suggestion
   ```

##########
File path: hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
##########
@@ -149,6 +162,26 @@ public RDBStore(File dbFile, DBOptions options,
     }
   }
 
+  /**
+   * Read DB and return existing column families.
+   * @return List of column families
+   * @throws RocksDBException on Error.
+   */
+  private List<TableConfig> getColumnFamiliesInExistingDb()
+      throws RocksDBException {
+    List<byte[]> bytes = RocksDB.listColumnFamilies(new Options(),
+        dbLocation.getAbsolutePath());
+    List<TableConfig> columnFamiliesInDb = bytes.stream()
+        .map(cfbytes -> new TableConfig(StringUtils.bytes2String(cfbytes),
+            DBProfile.DISK.getColumnFamilyOptions()))

Review comment:
       ```suggestion
               DBStoreBuilder.HDDS_DEFAULT_DB_PROFILE.getColumnFamilyOptions()))
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org