HDDS-1604. ContainerReader#initializeUsedBytes leaks DB reference. Co… (#866)

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java

@@ -48,7 +48,7 @@ import org.apache.hadoop.ozone.protocol.commands.DeleteBlocksCommand;
 import org.apache.hadoop.ozone.protocol.commands.SCMCommand;
 import org.apache.hadoop.util.Time;
 import org.apache.hadoop.utils.BatchOperation;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java

@@ -28,11 +28,8 @@ import org.apache.hadoop.utils.MetadataStoreBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
@@ -69,22 +66,6 @@ public final class ContainerCache extends LRUMap {
     return cache;
   }
 
-  /**
-   * Closes a db instance.
-   *
-   * @param containerPath - path of the container db to be closed.
-   * @param db - db instance to close.
-   */
-  private void closeDB(String containerPath, MetadataStore db) {
-    if (db != null) {
-      try {
-        db.close();
-      } catch (Exception e) {
-        LOG.error("Error closing DB. Container: " + containerPath, e);
-      }
-    }
-  }
-
   /**
    * Closes all the db instances and resets the cache.
    */
@@ -183,55 +164,4 @@ public final class ContainerCache extends LRUMap {
       lock.unlock();
     }
   }
-
-
-  /**
-   * Class to implement reference counting over instances handed by Container
-   * Cache.
-   */
-  public class ReferenceCountedDB implements Closeable {
-    private final AtomicInteger referenceCount;
-    private final AtomicBoolean isEvicted;
-    private final MetadataStore store;
-    private final String containerDBPath;
-
-    public ReferenceCountedDB(MetadataStore store, String containerDBPath) {
-      this.referenceCount = new AtomicInteger(0);
-      this.isEvicted = new AtomicBoolean(false);
-      this.store = store;
-      this.containerDBPath = containerDBPath;
-    }
-
-    private void incrementReference() {
-      this.referenceCount.incrementAndGet();
-    }
-
-    private void decrementReference() {
-      this.referenceCount.decrementAndGet();
-      cleanup();
-    }
-
-    private void setEvicted(boolean checkNoReferences) {
-      Preconditions.checkState(!checkNoReferences ||
-              (referenceCount.get() == 0),
-          "checkNoReferences:%b, referencount:%d",
-          checkNoReferences, referenceCount.get());
-      isEvicted.set(true);
-      cleanup();
-    }
-
-    private void cleanup() {
-      if (referenceCount.get() == 0 && isEvicted.get() && store != null) {
-        closeDB(containerDBPath, store);
-      }
-    }
-
-    public MetadataStore getStore() {
-      return store;
-    }
-
-    public void close() {
-      decrementReference();
-    }
-  }
 }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ReferenceCountedDB.java

@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.container.common.utils;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.utils.MetadataStore;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.Closeable;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * Class to implement reference counting over instances handed by Container
+ * Cache.
+ * Enable DEBUG log below will enable us quickly locate the leaked reference
+ * from caller stack. When JDK9 StackWalker is available, we can switch to
+ * StackWalker instead of new Exception().printStackTrace().
+ */
+public class ReferenceCountedDB implements Closeable {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ReferenceCountedDB.class);
+  private final AtomicInteger referenceCount;
+  private final AtomicBoolean isEvicted;
+  private final MetadataStore store;
+  private final String containerDBPath;
+
+  public ReferenceCountedDB(MetadataStore store, String containerDBPath) {
+    this.referenceCount = new AtomicInteger(0);
+    this.isEvicted = new AtomicBoolean(false);
+    this.store = store;
+    this.containerDBPath = containerDBPath;
+  }
+
+  public void incrementReference() {
+    this.referenceCount.incrementAndGet();
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("IncRef {} to refCnt {} \n", containerDBPath,
+          referenceCount.get());
+      new Exception().printStackTrace();
+    }
+  }
+
+  public void decrementReference() {
+    this.referenceCount.decrementAndGet();
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("DecRef {} to refCnt {} \n", containerDBPath,
+          referenceCount.get());
+      new Exception().printStackTrace();
+    }
+    cleanup();
+  }
+
+  public void setEvicted(boolean checkNoReferences) {
+    Preconditions.checkState(!checkNoReferences ||
+            (referenceCount.get() == 0),
+        "checkNoReferences:%b, referencount:%d, dbPath:%s",
+        checkNoReferences, referenceCount.get(), containerDBPath);
+    isEvicted.set(true);
+    cleanup();
+  }
+
+  private void cleanup() {
+    if (referenceCount.get() == 0 && isEvicted.get() && store != null) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Close {} refCnt {}", containerDBPath,
+            referenceCount.get());
+      }
+      try {
+        store.close();
+      } catch (Exception e) {
+        LOG.error("Error closing DB. Container: " + containerDBPath, e);
+      }
+    }
+  }
+
+  public MetadataStore getStore() {
+    return store;
+  }
+
+  public void close() {
+    decrementReference();
+  }
+}

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueBlockIterator.java

@@ -31,7 +31,7 @@ import org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerLocat
 import org.apache.hadoop.utils.MetaStoreIterator;
 import org.apache.hadoop.utils.MetadataKeyFilters;
 import org.apache.hadoop.utils.MetadataKeyFilters.KeyPrefixFilter;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.apache.hadoop.utils.MetadataStore.KeyValue;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java

@@ -73,7 +73,7 @@ import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos
 import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos
     .Result.UNSUPPORTED_REQUEST;
 
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java

@@ -35,7 +35,7 @@ import java.io.File;
 import java.io.IOException;
 import java.util.List;
 
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java

@@ -37,7 +37,7 @@ import org.apache.hadoop.ozone.container.common.helpers.BlockData;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerUtils;
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
 import org.apache.hadoop.ozone.container.common.utils.ContainerCache;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 
 import java.io.IOException;
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java

@@ -39,7 +39,7 @@ import org.apache.hadoop.utils.MetadataStoreBuilder;
 
 import com.google.common.base.Preconditions;
 import org.apache.commons.io.FileUtils;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java

@@ -35,7 +35,7 @@ import org.apache.hadoop.ozone.container.keyvalue.interfaces.BlockManager;
 import org.apache.hadoop.ozone.container.common.utils.ContainerCache;
 import org.apache.hadoop.utils.BatchOperation;
 import org.apache.hadoop.utils.MetadataKeyFilters;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java

@@ -43,7 +43,7 @@ import org.apache.hadoop.utils.BackgroundTaskQueue;
 import org.apache.hadoop.utils.BackgroundTaskResult;
 import org.apache.hadoop.utils.BatchOperation;
 import org.apache.hadoop.utils.MetadataKeyFilters.KeyPrefixFilter;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java

@@ -41,7 +41,7 @@ import org.apache.hadoop.ozone.container.common.impl.ContainerDataYaml;
 import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
 import org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerUtil;
 import org.apache.hadoop.utils.MetadataKeyFilters;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -237,24 +237,25 @@ public class ContainerReader implements Runnable {
 
   private void initializeUsedBytes(KeyValueContainer container)
       throws IOException {
-    KeyValueBlockIterator blockIter = new KeyValueBlockIterator(
+    try (KeyValueBlockIterator blockIter = new KeyValueBlockIterator(
         container.getContainerData().getContainerID(),
-        new File(container.getContainerData().getContainerPath()));
-    long usedBytes = 0;
+        new File(container.getContainerData().getContainerPath()))) {
+      long usedBytes = 0;
 
-    while (blockIter.hasNext()) {
-      BlockData block = blockIter.nextBlock();
-      long blockLen = 0;
+      while (blockIter.hasNext()) {
+        BlockData block = blockIter.nextBlock();
+        long blockLen = 0;
 
-      List<ContainerProtos.ChunkInfo> chunkInfoList = block.getChunks();
-      for (ContainerProtos.ChunkInfo chunk : chunkInfoList) {
-        ChunkInfo info = ChunkInfo.getFromProtoBuf(chunk);
-        blockLen += info.getLen();
+        List<ContainerProtos.ChunkInfo> chunkInfoList = block.getChunks();
+        for (ContainerProtos.ChunkInfo chunk : chunkInfoList) {
+          ChunkInfo info = ChunkInfo.getFromProtoBuf(chunk);
+          blockLen += info.getLen();
+        }
+
+        usedBytes += blockLen;
       }
 
-      usedBytes += blockLen;
+      container.getContainerData().setBytesUsed(usedBytes);
     }
-
-    container.getContainerData().setBytesUsed(usedBytes);
   }
 }

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueBlockIterator.java

@@ -34,7 +34,7 @@ import org.apache.hadoop.ozone.container.common.volume.VolumeSet;
 import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.utils.MetadataKeyFilters;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java

@@ -36,7 +36,7 @@ import org.apache.hadoop.ozone.container.common.volume.VolumeSet;
 import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.util.DiskChecker;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 
 import org.junit.Assert;
 import org.junit.Before;

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainerCheck.java

@@ -35,7 +35,7 @@ import org.apache.hadoop.ozone.container.common.volume.RoundRobinVolumeChoosingP
 import org.apache.hadoop.ozone.container.common.volume.VolumeSet;
 import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
 import org.apache.hadoop.test.GenericTestUtils;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java

@@ -33,7 +33,7 @@ import org.apache.hadoop.ozone.container.common.helpers.ChunkInfo;
 import org.apache.hadoop.ozone.container.common.impl.ContainerSet;
 import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine;
 import org.apache.hadoop.ozone.container.common.statemachine.StateContext;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.apache.hadoop.ozone.container.common.volume.HddsVolume;
 import org.apache.hadoop.ozone.container.common.volume.RoundRobinVolumeChoosingPolicy;
 import org.apache.hadoop.ozone.container.common.volume.VolumeSet;
@@ -155,7 +155,7 @@ public class TestOzoneContainer {
 
     long freeBytes = container.getContainerData().getMaxSize();
     long containerId = container.getContainerData().getContainerID();
-    ContainerCache.ReferenceCountedDB db = BlockUtils.getDB(container
+    ReferenceCountedDB db = BlockUtils.getDB(container
         .getContainerData(), conf);
 
     for (int bi = 0; bi < blocks; bi++) {

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManagerHelper.java

@@ -40,7 +40,7 @@ import org.apache.hadoop.ozone.web.interfaces.StorageHandler;
 import org.apache.hadoop.ozone.web.utils.OzoneUtils;
 import org.apache.hadoop.utils.MetadataKeyFilters;
 import org.apache.hadoop.utils.MetadataKeyFilters.KeyPrefixFilter;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 
 import java.io.IOException;
 import java.io.OutputStream;

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java

@@ -45,7 +45,7 @@ import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.test.GenericTestUtils.LogCapturer;
 import org.apache.hadoop.utils.BackgroundService;
 import org.apache.hadoop.utils.MetadataKeyFilters;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.Test;

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java

@@ -47,7 +47,7 @@ import org.apache.hadoop.ozone.container.keyvalue.impl.BlockManagerImpl;
 import org.apache.hadoop.ozone.container.keyvalue.interfaces.ChunkManager;
 import org.apache.hadoop.ozone.container.keyvalue.interfaces.BlockManager;
 import org.apache.hadoop.test.GenericTestUtils;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Assert;

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java

@@ -50,7 +50,7 @@ import org.apache.hadoop.ozone.ozShell.TestOzoneShell;
 import org.apache.hadoop.ozone.protocol.commands.RetriableDatanodeEventWatcher;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.test.GenericTestUtils.LogCapturer;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestCloseContainerByPipeline.java

@@ -41,7 +41,7 @@ import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
 import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
 import org.apache.hadoop.ozone.protocol.commands.CloseContainerCommand;
 import org.apache.hadoop.test.GenericTestUtils;
-import org.apache.hadoop.ozone.container.common.utils.ContainerCache.ReferenceCountedDB;
+import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;