HDDS-10184. Fix ManagedStatistics not closed properly

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/AbstractDatanodeStore.java

@@ -22,6 +22,7 @@
 import org.apache.hadoop.hdds.StringUtils;
 import org.apache.hadoop.hdds.annotation.InterfaceAudience;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.utils.IOUtils;
 import org.apache.hadoop.hdds.utils.MetadataKeyFilters;
 import org.apache.hadoop.hdds.utils.MetadataKeyFilters.KeyPrefixFilter;
 import org.apache.hadoop.hdds.utils.db.BatchOperationHandler;
@@ -117,62 +118,67 @@ public void start(ConfigurationSource config)
           OZONE_METADATA_STORE_ROCKSDB_STATISTICS,
           OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT);
 
-      if (!rocksDbStat.equals(OZONE_METADATA_STORE_ROCKSDB_STATISTICS_OFF)) {
-        ManagedStatistics statistics = new ManagedStatistics();
-        statistics.setStatsLevel(StatsLevel.valueOf(rocksDbStat));
-        options.setStatistics(statistics);
-      }
+      ManagedStatistics statistics = null;
+      try {
+        if (!rocksDbStat.equals(OZONE_METADATA_STORE_ROCKSDB_STATISTICS_OFF)) {
+          statistics = new ManagedStatistics();
+          statistics.setStatsLevel(StatsLevel.valueOf(rocksDbStat));
+          options.setStatistics(statistics);
+        }
 
-      DatanodeConfiguration dc =
-          config.getObject(DatanodeConfiguration.class);
-      // Config user log files
-      InfoLogLevel level = InfoLogLevel.valueOf(
-          dc.getRocksdbLogLevel() + "_LEVEL");
-      options.setInfoLogLevel(level);
-      options.setMaxLogFileSize(dc.getRocksdbLogMaxFileSize());
-      options.setKeepLogFileNum(dc.getRocksdbLogMaxFileNum());
-      
-      if (this.dbDef instanceof DatanodeSchemaThreeDBDefinition) {
-        options.setDeleteObsoleteFilesPeriodMicros(
-            dc.getRocksdbDeleteObsoleteFilesPeriod());
-
-        // For V3, all Rocksdb dir has the same "container.db" name. So use
-        // parentDirName(storage UUID)-dbDirName as db metrics name
-        this.store = DBStoreBuilder.newBuilder(config, dbDef)
-            .setDBOptions(options)
-            .setDefaultCFOptions(cfOptions)
-            .setOpenReadOnly(openReadOnly)
-            .setDBJmxBeanNameName(dbDef.getDBLocation(config).getName() + "-" +
-                dbDef.getName())
-            .build();
-      } else {
-        this.store = DBStoreBuilder.newBuilder(config, dbDef)
-            .setDBOptions(options)
-            .setDefaultCFOptions(cfOptions)
-            .setOpenReadOnly(openReadOnly)
-            .build();
-      }
+        DatanodeConfiguration dc =
+            config.getObject(DatanodeConfiguration.class);
+        // Config user log files
+        InfoLogLevel level = InfoLogLevel.valueOf(
+            dc.getRocksdbLogLevel() + "_LEVEL");
+        options.setInfoLogLevel(level);
+        options.setMaxLogFileSize(dc.getRocksdbLogMaxFileSize());
+        options.setKeepLogFileNum(dc.getRocksdbLogMaxFileNum());
+
+        if (this.dbDef instanceof DatanodeSchemaThreeDBDefinition) {
+          options.setDeleteObsoleteFilesPeriodMicros(
+              dc.getRocksdbDeleteObsoleteFilesPeriod());
+
+          // For V3, all Rocksdb dir has the same "container.db" name. So use
+          // parentDirName(storage UUID)-dbDirName as db metrics name
+          this.store = DBStoreBuilder.newBuilder(config, dbDef)
+              .setDBOptions(options)
+              .setDefaultCFOptions(cfOptions)
+              .setOpenReadOnly(openReadOnly)
+              .setDBJmxBeanNameName(dbDef.getDBLocation(config).getName() + "-" +
+                  dbDef.getName())
+              .build();
+        } else {
+          this.store = DBStoreBuilder.newBuilder(config, dbDef)
+              .setDBOptions(options)
+              .setDefaultCFOptions(cfOptions)
+              .setOpenReadOnly(openReadOnly)
+              .build();
+        }
 
-      // Use the DatanodeTable wrapper to disable the table iterator on
-      // existing Table implementations retrieved from the DBDefinition.
-      // See the DatanodeTable's Javadoc for an explanation of why this is
-      // necessary.
-      metadataTable = new DatanodeTable<>(
-              dbDef.getMetadataColumnFamily().getTable(this.store));
-      checkTableStatus(metadataTable, metadataTable.getName());
-
-      // The block iterator this class returns will need to use the table
-      // iterator internally, so construct a block data table instance
-      // that does not have the iterator disabled by DatanodeTable.
-      blockDataTableWithIterator =
-              dbDef.getBlockDataColumnFamily().getTable(this.store);
-
-      blockDataTable = new DatanodeTable<>(blockDataTableWithIterator);
-      checkTableStatus(blockDataTable, blockDataTable.getName());
-
-      deletedBlocksTable = new DatanodeTable<>(
-              dbDef.getDeletedBlocksColumnFamily().getTable(this.store));
-      checkTableStatus(deletedBlocksTable, deletedBlocksTable.getName());
+        // Use the DatanodeTable wrapper to disable the table iterator on
+        // existing Table implementations retrieved from the DBDefinition.
+        // See the DatanodeTable's Javadoc for an explanation of why this is
+        // necessary.
+        metadataTable = new DatanodeTable<>(
+                dbDef.getMetadataColumnFamily().getTable(this.store));
+        checkTableStatus(metadataTable, metadataTable.getName());
+
+        // The block iterator this class returns will need to use the table
+        // iterator internally, so construct a block data table instance
+        // that does not have the iterator disabled by DatanodeTable.
+        blockDataTableWithIterator =
+                dbDef.getBlockDataColumnFamily().getTable(this.store);
+
+        blockDataTable = new DatanodeTable<>(blockDataTableWithIterator);
+        checkTableStatus(blockDataTable, blockDataTable.getName());
+
+        deletedBlocksTable = new DatanodeTable<>(
+                dbDef.getDeletedBlocksColumnFamily().getTable(this.store));
+        checkTableStatus(deletedBlocksTable, deletedBlocksTable.getName());
+      } finally {
+        IOUtils.closeQuietly(statistics);
+      }
     }
   }
 

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java

@@ -50,6 +50,7 @@
 import static org.rocksdb.RocksDB.DEFAULT_COLUMN_FAMILY;
 
 import org.apache.hadoop.hdds.conf.StorageUnit;
+import org.apache.hadoop.hdds.utils.IOUtils;
 import org.apache.hadoop.hdds.utils.db.managed.ManagedColumnFamilyOptions;
 import org.apache.hadoop.hdds.utils.db.managed.ManagedDBOptions;
 import org.apache.hadoop.hdds.utils.db.managed.ManagedRocksDB;
@@ -83,6 +84,9 @@ public final class DBStoreBuilder {
   // The column family options that will be used for any column families
   // added by name only (without specifying options).
   private ManagedColumnFamilyOptions defaultCfOptions;
+
+  private ManagedStatistics statistics;
+
   private String dbname;
   private Path dbPath;
   private String dbJmxBeanNameName;
@@ -222,6 +226,7 @@ public DBStore build() throws IOException {
           enableCompactionDag, maxDbUpdatesSizeThreshold, createCheckpointDirs,
           configuration, threadNamePrefix);
     } finally {
+      IOUtils.closeQuietly(statistics);
       tableConfigs.forEach(TableConfig::close);
     }
   }
@@ -415,7 +420,7 @@ protected void log(InfoLogLevel infoLogLevel, String s) {
 
     // Create statistics.
     if (!rocksDbStat.equals(OZONE_METADATA_STORE_ROCKSDB_STATISTICS_OFF)) {
-      ManagedStatistics statistics = new ManagedStatistics();
+      statistics = new ManagedStatistics();
       statistics.setStatsLevel(StatsLevel.valueOf(rocksDbStat));
       dbOptions.setStatistics(statistics);
     }

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java

@@ -31,6 +31,7 @@
 import java.util.concurrent.TimeoutException;
 import java.util.function.Supplier;
 
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_METADATA_STORE_ROCKSDB_STATISTICS;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -49,6 +50,7 @@ public class TestRocksObjectLeakDetector {
   static void setUp() throws IOException, InterruptedException,
       TimeoutException {
     OzoneConfiguration conf = new OzoneConfiguration();
+    conf.set(OZONE_METADATA_STORE_ROCKSDB_STATISTICS, "ALL");
     String clusterId = UUID.randomUUID().toString();
     String scmId = UUID.randomUUID().toString();
     String omServiceId = "omServiceId1";