HDDS-250. Cleanup ContainerData.

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java

@@ -70,12 +70,9 @@ public final class OzoneConsts {
   public static final String CONTAINER_EXTENSION = ".container";
   public static final String CONTAINER_META = ".meta";
 
-  //  container storage is in the following format.
+  // Refer to {@link ContainerReader} for container storage layout on disk.
-  //  Data Volume basePath/containers/<containerName>/metadata and
-  //  Data Volume basePath/containers/<containerName>/data/...
   public static final String CONTAINER_PREFIX  = "containers";
   public static final String CONTAINER_META_PATH = "metadata";
-  public static final String CONTAINER_DATA_PATH = "data";
   public static final String CONTAINER_TEMPORARY_CHUNK_PREFIX = "tmp";
   public static final String CONTAINER_CHUNK_NAME_DELIMITER = ".";
   public static final String CONTAINER_ROOT_PREFIX = "repository";

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/Storage.java

@@ -47,7 +47,7 @@ public abstract class Storage {
 
   public static final String STORAGE_DIR_CURRENT = "current";
   protected static final String STORAGE_FILE_VERSION = "VERSION";
-  public static final String CONTAINER_DIR = "containerdir";
+  public static final String CONTAINER_DIR = "containerDir";
 
   private final NodeType nodeType;
   private final File root;

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java

@@ -103,27 +103,6 @@ public final class ContainerUtils {
     return builder.build();
   }
 
-  /**
-   * Returns a ReadContainer Response.
-   * @param msg requestProto message.
-   * @param containerData container data to be returned.
-   * @return ReadContainer Response
-   */
-  public static ContainerProtos.ContainerCommandResponseProto
-    getReadContainerResponse(ContainerProtos.ContainerCommandRequestProto msg,
-      ContainerData containerData) {
-    Preconditions.checkNotNull(containerData);
-
-    ContainerProtos.ReadContainerResponseProto.Builder response =
-        ContainerProtos.ReadContainerResponseProto.newBuilder();
-    response.setContainerData(containerData.getProtoBufMessage());
-
-    ContainerProtos.ContainerCommandResponseProto.Builder builder =
-        getSuccessResponseBuilder(msg);
-    builder.setReadContainer(response);
-    return builder.build();
-  }
-
   /**
    * We found a command type but no associated payload for the command. Hence
    * return malformed Command as response.

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java

@@ -18,6 +18,7 @@
 package org.apache.hadoop.ozone.container.common.impl;
 
 import com.google.common.base.Preconditions;
+import java.util.List;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.
     ContainerType;
@@ -38,7 +39,7 @@ import static java.lang.Math.max;
  * ContainerData is the in-memory representation of container metadata and is
  * represented on disk by the .container file.
  */
-public class ContainerData {
+public abstract class ContainerData {
 
   //Type of the container.
   // For now, we support only KeyValueContainer.
@@ -47,9 +48,6 @@ public class ContainerData {
   // Unique identifier for the container
   private final long containerID;
 
-  // Path to container root dir.
-  private String containerPath;
-
   // Layout version of the container data
   private final int layOutVersion;
 
@@ -85,7 +83,7 @@ public class ContainerData {
    * @param containerId - ContainerId
    * @param size - container maximum size
    */
-  public ContainerData(ContainerType type, long containerId, int size) {
+  protected ContainerData(ContainerType type, long containerId, int size) {
     this(type, containerId,
         ChunkLayOutVersion.getLatestVersion().getVersion(), size);
   }
@@ -97,7 +95,7 @@ public class ContainerData {
    * @param layOutVersion - Container layOutVersion
    * @param size - Container maximum size
    */
-  public ContainerData(ContainerType type, long containerId,
+  protected ContainerData(ContainerType type, long containerId,
     int layOutVersion, int size) {
     Preconditions.checkNotNull(type);
 
@@ -128,17 +126,7 @@ public class ContainerData {
    * Returns the path to base dir of the container.
    * @return Path to base dir.
    */
-  public String getContainerPath() {
+  public abstract String getContainerPath();
-    return containerPath;
-  }
-
-  /**
-   * Set the base dir path of the container.
-   * @param baseDir path to base dir
-   */
-  public void setContainerPath(String baseDir) {
-    this.containerPath = baseDir;
-  }
 
   /**
    * Returns the type of the container.
@@ -387,20 +375,6 @@ public class ContainerData {
     this.keyCount.set(count);
   }
 
-  /**
-   * Returns container metadata path.
-   */
-  public String getMetadataPath() {
-    return null;
-  }
-
-  /**
-   * Returns container data path.
-   */
-  public String getDataPath() {
-    return null;
-  }
-
   /**
    * Increase the count of pending deletion blocks.
    *
@@ -431,33 +405,7 @@ public class ContainerData {
    *
    * @return Protocol Buffer Message
    */
-  public ContainerProtos.ContainerData getProtoBufMessage() {
+  public abstract ContainerProtos.ContainerData getProtoBufMessage();
-    ContainerProtos.ContainerData.Builder builder =
-        ContainerProtos.ContainerData.newBuilder();
-
-    builder.setContainerID(this.getContainerID());
-
-    if (this.containerPath != null) {
-      builder.setContainerPath(this.containerPath);
-    }
-
-    builder.setState(this.getState());
-
-    for (Map.Entry<String, String> entry : metadata.entrySet()) {
-      ContainerProtos.KeyValue.Builder keyValBuilder =
-          ContainerProtos.KeyValue.newBuilder();
-      builder.addMetadata(keyValBuilder.setKey(entry.getKey())
-          .setValue(entry.getValue()).build());
-    }
-
-    if (this.getBytesUsed() >= 0) {
-      builder.setBytesUsed(this.getBytesUsed());
-    }
-
-    builder.setContainerType(containerType);
-
-    return builder.build();
-  }
 
   /**
    * Sets deleteTransactionId to latest delete transactionId for the container.

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

@@ -111,15 +111,15 @@ public class KeyValueContainer implements Container {
     try {
       HddsVolume containerVolume = volumeChoosingPolicy.chooseVolume(volumeSet
           .getVolumesList(), maxSize);
-      String containerBasePath = containerVolume.getHddsRootDir().toString();
+      String hddsVolumeDir = containerVolume.getHddsRootDir().toString();
 
       long containerId = containerData.getContainerID();
       String containerName = Long.toString(containerId);
 
       containerMetaDataPath = KeyValueContainerLocationUtil
-          .getContainerMetaDataPath(containerBasePath, scmId, containerId);
+          .getContainerMetaDataPath(hddsVolumeDir, scmId, containerId);
       File chunksPath = KeyValueContainerLocationUtil.getChunksLocationPath(
-          containerBasePath, scmId, containerId);
+          hddsVolumeDir, scmId, containerId);
       File containerFile = KeyValueContainerLocationUtil.getContainerFile(
           containerMetaDataPath, containerName);
       File containerCheckSumFile = KeyValueContainerLocationUtil

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

@@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
 import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.ozone.OzoneConsts;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
 import org.yaml.snakeyaml.nodes.Tag;
 
@@ -73,9 +74,6 @@ public class KeyValueContainerData extends ContainerData {
   //Type of DB used to store key to chunks mapping
   private String containerDBType;
 
-  //Number of pending deletion blocks in container.
-  private int numPendingDeletionBlocks;
-
   private File dbFile = null;
 
   /**
@@ -85,7 +83,6 @@ public class KeyValueContainerData extends ContainerData {
    */
   public KeyValueContainerData(long id, int size) {
     super(ContainerProtos.ContainerType.KeyValueContainer, id, size);
-    this.numPendingDeletionBlocks = 0;
   }
 
   /**
@@ -97,7 +94,6 @@ public class KeyValueContainerData extends ContainerData {
   public KeyValueContainerData(long id, int layOutVersion, int size) {
     super(ContainerProtos.ContainerType.KeyValueContainer, id, layOutVersion,
         size);
-    this.numPendingDeletionBlocks = 0;
   }
 
 
@@ -120,8 +116,8 @@ public class KeyValueContainerData extends ContainerData {
 
   /**
    * Returns container metadata path.
+   * @return - Physical path where container file and checksum is stored.
    */
-  @Override
   public String getMetadataPath() {
     return metadataPath;
   }
@@ -136,18 +132,21 @@ public class KeyValueContainerData extends ContainerData {
   }
 
   /**
-   * Get chunks path.
+   * Returns the path to base dir of the container.
-   * @return - Physical path where container file and checksum is stored.
+   * @return Path to base dir
    */
-  public String getChunksPath() {
+  public String getContainerPath() {
-    return chunksPath;
+    if (metadataPath == null) {
+      return null;
+    }
+    return new File(metadataPath).getParent();
   }
 
   /**
-   * Returns container chunks path.
+   * Get chunks path.
+   * @return - Path where chunks are stored
    */
-  @Override
+  public String getChunksPath() {
-  public String getDataPath() {
     return chunksPath;
   }
 
@@ -175,33 +174,6 @@ public class KeyValueContainerData extends ContainerData {
     this.containerDBType = containerDBType;
   }
 
-  /**
-   * Returns the number of pending deletion blocks in container.
-   * @return numPendingDeletionBlocks
-   */
-  public int getNumPendingDeletionBlocks() {
-    return numPendingDeletionBlocks;
-  }
-
-
-  /**
-   * Increase the count of pending deletion blocks.
-   *
-   * @param numBlocks increment number
-   */
-  public void incrPendingDeletionBlocks(int numBlocks) {
-    this.numPendingDeletionBlocks += numBlocks;
-  }
-
-  /**
-   * Decrease the count of pending deletion blocks.
-   *
-   * @param numBlocks decrement number
-   */
-  public void decrPendingDeletionBlocks(int numBlocks) {
-    this.numPendingDeletionBlocks -= numBlocks;
-  }
-
   /**
    * Returns a ProtoBuf Message from ContainerData.
    *
@@ -260,7 +232,9 @@ public class KeyValueContainerData extends ContainerData {
     }
 
     if (protoData.hasContainerPath()) {
-      data.setContainerPath(protoData.getContainerPath());
+      String metadataPath = protoData.getContainerPath()+ File.separator +
+          OzoneConsts.CONTAINER_META_PATH;
+      data.setMetadataPath(metadataPath);
     }
 
     if (protoData.hasState()) {

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

@@ -34,14 +34,16 @@ public final class KeyValueContainerLocationUtil {
   }
   /**
    * Returns Container Metadata Location.
-   * @param baseDir
+   * @param hddsVolumeDir base dir of the hdds volume where scm directories
+   *                      are stored
    * @param scmId
    * @param containerId
-   * @return containerMetadata Path
+   * @return containerMetadata Path to container metadata location where
+   * .container file will be stored.
    */
-  public static File getContainerMetaDataPath(String baseDir, String scmId,
+  public static File getContainerMetaDataPath(String hddsVolumeDir, String scmId,
                                               long containerId) {
-    String containerMetaDataPath = getBaseContainerLocation(baseDir, scmId,
+    String containerMetaDataPath = getBaseContainerLocation(hddsVolumeDir, scmId,
         containerId);
     containerMetaDataPath = containerMetaDataPath + File.separator +
         OzoneConsts.CONTAINER_META_PATH;
@@ -65,21 +67,21 @@ public final class KeyValueContainerLocationUtil {
 
   /**
    * Returns base directory for specified container.
-   * @param baseDir
+   * @param hddsVolumeDir
    * @param scmId
    * @param containerId
    * @return base directory for container.
    */
-  private static String getBaseContainerLocation(String baseDir, String scmId,
+  private static String getBaseContainerLocation(String hddsVolumeDir, String scmId,
                                         long containerId) {
-    Preconditions.checkNotNull(baseDir, "Base Directory cannot be null");
+    Preconditions.checkNotNull(hddsVolumeDir, "Base Directory cannot be null");
     Preconditions.checkNotNull(scmId, "scmUuid cannot be null");
     Preconditions.checkState(containerId >= 0,
         "Container Id cannot be negative.");
 
     String containerSubDirectory = getContainerSubDirectory(containerId);
 
-    String containerMetaDataPath = baseDir  + File.separator + scmId +
+    String containerMetaDataPath = hddsVolumeDir  + File.separator + scmId +
         File.separator + Storage.STORAGE_DIR_CURRENT + File.separator +
         containerSubDirectory + File.separator + containerId;
 

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

@@ -172,11 +172,11 @@ public class BlockDeletingService extends BackgroundService{
       implements BackgroundTask<BackgroundTaskResult> {
 
     private final int priority;
-    private final ContainerData containerData;
+    private final KeyValueContainerData containerData;
 
     BlockDeletingTask(ContainerData containerName, int priority) {
       this.priority = priority;
-      this.containerData = containerName;
+      this.containerData = (KeyValueContainerData) containerName;
     }
 
     @Override
@@ -199,10 +199,10 @@ public class BlockDeletingService extends BackgroundService{
       List<String> succeedBlocks = new LinkedList<>();
       LOG.debug("Container : {}, To-Delete blocks : {}",
           containerData.getContainerID(), toDeleteBlocks.size());
-      File dataDir = new File(containerData.getDataPath());
+      File dataDir = new File(containerData.getChunksPath());
       if (!dataDir.exists() || !dataDir.isDirectory()) {
         LOG.error("Invalid container data dir {} : "
-            + "not exist or not a directory", dataDir.getAbsolutePath());
+            + "does not exist or not a directory", dataDir.getAbsolutePath());
         return crr;
       }
 

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

@@ -40,6 +40,21 @@ import java.io.IOException;
 
 /**
  * Class used to read .container files from Volume and build container map.
+ *
+ * Layout of the container directory on disk is as follows:
+ *
+ * ../hdds/VERSION
+ * ../hdds/<<scmUuid>>/current/<<containerDir>>/<<containerID>/metadata/<<containerID>>.container
+ * ../hdds/<<scmUuid>>/current/<<containerDir>>/<<containerID>/metadata/<<containerID>>.checksum
+ * ../hdds/<<scmUuid>>/current/<<containerDir>>/<<containerID>/metadata/<<containerID>>.db
+ * ../hdds/<<scmUuid>>/current/<<containerDir>>/<<containerID>/<<dataPath>>
+ *
+ * Note that the <<dataPath>> is dependent on the ContainerType.
+ * For KeyValueContainers, the data is stored in a "chunks" folder. As such,
+ * the <<dataPath>> layout for KeyValueContainers is
+ *
+ * ../hdds/<<scmUuid>>/current/<<containerDir>>/<<containerID>/chunks/<<chunksFile>>
+ *
  */
 public class ContainerReader implements Runnable {
 
@@ -73,21 +88,6 @@ public class ContainerReader implements Runnable {
     Preconditions.checkNotNull(hddsVolumeRootDir, "hddsVolumeRootDir" +
         "cannot be null");
 
-
-    /**
-     *
-     * layout of the container directory on the disk.
-     * /hdds/<<scmUuid>>/current/<<containerdir>>/</containerID>/metadata
-     * /<<containerID>>.container
-     * /hdds/<<scmUuid>>/current/<<containerdir>>/<<containerID>>/metadata
-     * /<<containerID>>.checksum
-     * /hdds/<<scmUuid>>/current/<<containerdir>>/<<containerID>>/metadata
-     * /<<containerID>>.db
-     * /hdds/<<scmUuid>>/current/<<containerdir>>/<<containerID>>/chunks
-     * /<<chunkFile>>
-     *
-     **/
-
     //filtering scm directory
     File[] scmDir = hddsVolumeRootDir.listFiles(new FileFilter() {
       @Override

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java

@@ -151,9 +151,9 @@ public class TestContainerDataYaml {
       assertEquals(ContainerProtos.ContainerType.KeyValueContainer, kvData
           .getContainerType());
       assertEquals(9223372036854775807L, kvData.getContainerID());
-      assertEquals("/hdds/current/aed-fg4-hji-jkl/containerdir0/1", kvData
+      assertEquals("/hdds/current/aed-fg4-hji-jkl/containerDir0/1", kvData
           .getChunksPath());
-      assertEquals("/hdds/current/aed-fg4-hji-jkl/containerdir0/1", kvData
+      assertEquals("/hdds/current/aed-fg4-hji-jkl/containerDir0/1", kvData
           .getMetadataPath());
       assertEquals(1, kvData.getLayOutVersion());
       assertEquals(2, kvData.getMetadata().size());

hadoop-hdds/container-service/src/test/resources/additionalfields.container

@@ -1,9 +1,9 @@
 !<KeyValueContainerData>
 containerDBType: RocksDB
-chunksPath: /hdds/current/aed-fg4-hji-jkl/containerdir0/1
+chunksPath: /hdds/current/aed-fg4-hji-jkl/containerDir0/1
 containerID: 9223372036854775807
 containerType: KeyValueContainer
-metadataPath: /hdds/current/aed-fg4-hji-jkl/containerdir0/1
+metadataPath: /hdds/current/aed-fg4-hji-jkl/containerDir0/1
 layOutVersion: 1
 maxSizeGB: 5
 metadata: {OWNER: ozone, VOLUME: hdfs}

hadoop-hdds/container-service/src/test/resources/incorrect.container

@@ -1,9 +1,9 @@
 !<KeyValueContainerData>
 containerDBType: RocksDB
-chunksPath: /hdds/current/aed-fg4-hji-jkl/containerdir0/1
+chunksPath: /hdds/current/aed-fg4-hji-jkl/containerDir0/1
 containerID: 9223372036854775807
 containerType: KeyValueContainer
-metadataPath: /hdds/current/aed-fg4-hji-jkl/containerdir0/1
+metadataPath: /hdds/current/aed-fg4-hji-jkl/containerDir0/1
 layOutVersion: 1
 maxSizeGB: 5
 metadata: {OWNER: ozone, VOLUME: hdfs}

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

@@ -92,6 +92,9 @@ import static org.junit.Assert.fail;
 
 /**
  * Simple tests to verify that container persistence works as expected.
+ * Some of these tests are specific to {@link KeyValueContainer}. If a new
+ * {@link ContainerProtos.ContainerType} is added, the tests need to be
+ * modified.
  */
 public class TestContainerPersistence {
   @Rule
@@ -409,9 +412,10 @@ public class TestContainerPersistence {
       fileHashMap.put(fileName, info);
     }
 
-    ContainerData cNewData = container.getContainerData();
+    KeyValueContainerData cNewData =
+        (KeyValueContainerData) container.getContainerData();
     Assert.assertNotNull(cNewData);
-    Path dataDir = Paths.get(cNewData.getDataPath());
+    Path dataDir = Paths.get(cNewData.getChunksPath());
 
     String globFormat = String.format("%s.data.*", blockID.getLocalID());
     MessageDigest sha = MessageDigest.getInstance(OzoneConsts.FILE_HASH);
@@ -707,7 +711,8 @@ public class TestContainerPersistence {
   @Test
   public void testUpdateContainer() throws IOException {
     long testContainerID = ContainerTestHelper.getTestContainerID();
-    Container container = addContainer(containerSet, testContainerID);
+    KeyValueContainer container =
+        (KeyValueContainer) addContainer(containerSet, testContainerID);
 
     File orgContainerFile = KeyValueContainerLocationUtil.getContainerFile(
         new File(container.getContainerData().getMetadataPath()),
@@ -725,7 +730,7 @@ public class TestContainerPersistence {
         .containsKey(testContainerID));
 
     // Verify in-memory map
-    ContainerData actualNewData =
+    KeyValueContainerData actualNewData = (KeyValueContainerData)
         containerSet.getContainer(testContainerID).getContainerData();
     Assert.assertEquals("shire_new",
         actualNewData.getMetadata().get("VOLUME"));
@@ -766,7 +771,7 @@ public class TestContainerPersistence {
     container.update(newMetadata, true);
 
     // Verify in-memory map
-    actualNewData =
+    actualNewData = (KeyValueContainerData)
         containerSet.getContainer(testContainerID).getContainerData();
     Assert.assertEquals("shire_new_1",
         actualNewData.getMetadata().get("VOLUME"));