jojochuang commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3539214249


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBProfile.java:
##########
@@ -87,6 +87,7 @@ public ManagedBlockBasedTableConfig 
getBlockBasedTableConfig() {
       ManagedBlockBasedTableConfig config = new ManagedBlockBasedTableConfig();
       config.setBlockCache(new ManagedLRUCache(blockCacheSize))
           .setBlockSize(blockSize)
+          .setFormatVersion(ManagedBlockBasedTableConfig.FORMAT_VERSION)

Review Comment:
   up until now, the sst file in the rocksdb used by Ozone does not define 
format version.
   
   Future versions of Ozone, if they bump the format version, can roll back to 
as early as this version of Ozone (2.3.0?), but they can't roll back to before 
2.3.0. Otherwise the sst generated by future Ozone version may not be 
compatible.



##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java:
##########
@@ -28,6 +28,20 @@
 public class ManagedBloomFilter extends BloomFilter {
   private final UncheckedAutoCloseable leakTracker = track(this);
 
+  // Delegate to satisfy SpotBugs EQ_DOESNT_OVERRIDE_EQUALS: BloomFilter 
defines

Review Comment:
   another approach is annotate the class to supress the warning.
   
   ```
   @SuppressFBWarnings(
       value = "EQ_DOESNT_OVERRIDE_EQUALS",
       justification = "leakTracker is resource-tracking state and not part of 
logical identity"
   )
   ```



##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/hadoop/hdds/utils/db/RDBSstFileWriter.java:
##########
@@ -36,8 +37,13 @@ public class RDBSstFileWriter implements Closeable {
   private AtomicLong keyCounter;
   private ManagedOptions emptyOption = new ManagedOptions();
   private final ManagedEnvOptions emptyEnvOptions = new ManagedEnvOptions();
+  private final ManagedBlockBasedTableConfig tableConfig = new 
ManagedBlockBasedTableConfig();
 
   public RDBSstFileWriter(File externalFile) throws RocksDatabaseException {
+    // Pin the SST format version so files written here (e.g. snapshot defrag
+    // ingest) stay readable if Ozone is downgraded before finalization.
+    tableConfig.setFormatVersion(ManagedBlockBasedTableConfig.FORMAT_VERSION);

Review Comment:
   Need an upgrade test later. We'll need it for 2.3.0 release anyway.
   
   Actually, we're missing upgrade tests for 2.1 and 2.2 too 
https://github.com/apache/ozone/tree/master/hadoop-ozone/dist/src/main/compose/upgrade/upgrades/non-rolling-upgrade/callbacks



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to