HDFS-11594. Ozone: close container should call compactDB. Contributed by Anu Engineer.

hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/DatanodeContainerProtocol.proto

@@ -125,6 +125,7 @@ enum Result {
   GET_SMALL_FILE_ERROR = 21;
   CLOSED_CONTAINER_IO = 22;
   ERROR_CONTAINER_NOT_EMPTY = 23;
+  ERROR_IN_COMPACT_DB = 24;
 }
 
 message ContainerCommandRequestProto {

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ChunkUtils.java

@@ -24,9 +24,9 @@ import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.hadoop.hdfs.ozone.protocol.proto.ContainerProtos;
 import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.ozone.OzoneConsts;
-import org.apache.hadoop.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.impl.ChunkManagerImpl;
 import org.apache.hadoop.scm.container.common.helpers.Pipeline;
+import org.apache.hadoop.scm.container.common.helpers.StorageContainerException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -217,7 +217,12 @@ public final class ChunkUtils {
         }
       }
       if (file != null) {
-        IOUtils.closeStream(file);
+        try {
+          file.close();
+        } catch (IOException e) {
+          throw new StorageContainerException("Error closing chunk file",
+              e, CONTAINER_INTERNAL_ERROR);
+        }
       }
     }
   }
@@ -250,9 +255,7 @@ public final class ChunkUtils {
    *
    * @param chunkFile - file where data lives.
    * @param data - chunk definition.
-   *
    * @return ByteBuffer
-   *
    * @throws StorageContainerException
    * @throws ExecutionException
    * @throws InterruptedException

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerManagerImpl.java

@@ -24,6 +24,7 @@ import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hdfs.ozone.protocol.proto.ContainerProtos;
+import org.apache.hadoop.ozone.container.common.helpers.KeyUtils;
 import org.apache.hadoop.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.protocol.proto
     .StorageContainerDatanodeProtocolProtos.SCMNodeReport;
@@ -40,6 +41,7 @@ import org.apache.hadoop.ozone.container.common.interfaces
 import org.apache.hadoop.ozone.container.common.interfaces.ContainerManager;
 import org.apache.hadoop.ozone.container.common.interfaces.KeyManager;
 import org.apache.hadoop.scm.container.common.helpers.Pipeline;
+import org.apache.hadoop.utils.LevelDBStore;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -78,6 +80,8 @@ import static org.apache.hadoop.hdfs.ozone.protocol.proto.ContainerProtos
     .Result.UNABLE_TO_READ_METADATA_DB;
 import static org.apache.hadoop.hdfs.ozone.protocol.proto.ContainerProtos
     .Result.UNSUPPORTED_REQUEST;
+import static org.apache.hadoop.hdfs.ozone.protocol.proto.ContainerProtos
+   .Result.ERROR_IN_COMPACT_DB;
 import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_EXTENSION;
 import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_META;
 
@@ -301,19 +305,23 @@ public class ContainerManagerImpl implements ContainerManager {
     FileOutputStream metaStream = null;
 
     try {
-      Path location = locationManager.getContainerPath();
+      Path metadataPath = null;
+      Path location = (!overwrite) ? locationManager.getContainerPath():
+          Paths.get(containerData.getContainerPath()).getParent();
       File containerFile = ContainerUtils.getContainerFile(containerData,
           location);
       File metadataFile = ContainerUtils.getMetadataFile(containerData,
           location);
+
       if(!overwrite) {
         ContainerUtils.verifyIsNewContainer(containerFile, metadataFile);
+        metadataPath = this.locationManager.getDataPath(
+            containerData.getContainerName());
+        metadataPath = ContainerUtils.createMetadata(metadataPath);
+      }  else {
+        metadataPath = ContainerUtils.getMetadataDirectory(containerData);
       }
 
-      Path metadataPath = this.locationManager.getDataPath(
-          containerData.getContainerName());
-      metadataPath = ContainerUtils.createMetadata(metadataPath);
-
       containerStream = new FileOutputStream(containerFile);
       metaStream = new FileOutputStream(metadataFile);
       MessageDigest sha = MessageDigest.getInstance(OzoneConsts.FILE_HASH);
@@ -470,6 +478,16 @@ public class ContainerManagerImpl implements ContainerManager {
     ContainerData containerData = readContainer(containerName);
     containerData.closeContainer();
     writeContainerInfo(containerData, true);
+    LevelDBStore db = KeyUtils.getDB(containerData, conf);
+
+    // It is ok if this operation takes a bit of time.
+    // Close container is not expected to be instantaneous.
+    try {
+      db.compactDB();
+    } catch (IOException e) {
+      LOG.error("Error in DB compaction while closing container", e);
+      throw new StorageContainerException(e, ERROR_IN_COMPACT_DB);
+    }
 
     // Active is different from closed. Closed means it is immutable, active
     // false means we have some internal error that is happening to this

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/utils/LevelDBStore.java

@@ -156,4 +156,16 @@ public class LevelDBStore implements Closeable {
   public void destroy() throws IOException {
     JniDBFactory.factory.destroy(dbFile, dbOptions);
   }
+
+
+  /**
+   * Compacts the DB by removing deleted keys etc.
+   * @throws IOException if there is an error.
+   */
+  public void compactDB() throws IOException {
+    if(db != null) {
+      // From LevelDB docs : begin == null and end == null means the whole DB.
+      db.compactRange(null, null);
+    }
+  }
 }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java

@@ -276,7 +276,8 @@ public class TestOzoneContainer {
     }
   }
 
-  private void testCloseContainer() throws Exception {
+  @Test
+  public void testCloseContainer() throws Exception {
     MiniOzoneCluster cluster = null;
     XceiverClient client = null;
     try {
@@ -307,17 +308,19 @@ public class TestOzoneContainer {
       client.connect();
 
 
+
+      // Create container
+      ContainerProtos.ContainerCommandRequestProto request =
+          ContainerTestHelper.getCreateContainerRequest(containerName);
+      ContainerProtos.ContainerCommandResponseProto response =
+          client.sendCommand(request);
+      Assert.assertNotNull(response);
+      Assert.assertTrue(request.getTraceID().equals(response.getTraceID()));
+
       ContainerProtos.ContainerCommandRequestProto writeChunkRequest =
           ContainerTestHelper.getWriteChunkRequest(pipeline, containerName,
               keyName, 1024);
 
-      ContainerProtos.ContainerCommandRequestProto request;
-      ContainerProtos.ContainerCommandResponseProto response;
-
-      ContainerProtos.ContainerCommandRequestProto putKeyRequest =
-          ContainerTestHelper.getPutKeyRequest(writeChunkRequest
-              .getWriteChunk());
-
       // Write Chunk before closing
       response = client.sendCommand(writeChunkRequest);
       Assert.assertNotNull(response);
@@ -327,6 +330,9 @@ public class TestOzoneContainer {
           .getTraceID()));
 
 
+      ContainerProtos.ContainerCommandRequestProto putKeyRequest =
+          ContainerTestHelper.getPutKeyRequest(writeChunkRequest
+              .getWriteChunk());
       // Put key before closing.
       response = client.sendCommand(putKeyRequest);
       Assert.assertNotNull(response);

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/scm/node/TestNodeManager.java

@@ -866,7 +866,7 @@ public class TestNodeManager {
   }
 
   @Test
-  public void testScmEnterAndExistChillMode() throws IOException,
+  public void testScmEnterAndExitChillMode() throws IOException,
       InterruptedException {
     OzoneConfiguration conf = getConf();
     conf.setInt(OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL_MS, 100);
@@ -879,7 +879,7 @@ public class TestNodeManager {
       Assert.assertThat(status, CoreMatchers.containsString("Still in chill " +
           "mode. Waiting on nodes to report in."));
 
-      // Should not exist chill mode since 10 nodes have not heartbeat yet.
+      // Should not exit chill mode since 10 nodes have not heartbeat yet.
       assertFalse(nodeManager.isOutOfNodeChillMode());
       assertFalse((nodeManager.isInManualChillMode()));