Address review comments

Author: mladjan-gadzic

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/debug/TestFindContainerKeys.java

@@ -19,21 +19,22 @@
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import org.apache.hadoop.hdds.client.BlockID;
+import org.apache.hadoop.hdds.client.RatisReplicationConfig;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
-import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.utils.db.DBStore;
 import org.apache.hadoop.hdds.utils.db.DBStoreBuilder;
 import org.apache.hadoop.hdds.utils.db.Table;
 import org.apache.hadoop.ozone.ClientVersion;
+import org.apache.hadoop.ozone.debug.container.ContainerCommands;
 import org.apache.hadoop.ozone.debug.container.ContainerKeyInfo;
 import org.apache.hadoop.ozone.debug.container.ContainerKeyInfoResponse;
 import org.apache.hadoop.ozone.debug.container.FindContainerKeys;
 import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup;
 import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -50,14 +51,15 @@
 import java.util.Map;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
 import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DIRECTORY_TABLE;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE;
+import static org.assertj.core.api.Assertions.assertThat;
 
 /**
- * This class tests `ozone debug ckscanner` CLI that reads from RocksDB
- * and gets keys for container ids.
+ * Test class for {@link FindContainerKeys}.
  */
 public class TestFindContainerKeys {
   private DBStore dbStore;
@@ -66,37 +68,8 @@ public class TestFindContainerKeys {
   private StringWriter stdout, stderr;
   private PrintWriter pstdout, pstderr;
   private CommandLine cmd;
-  private static final Gson GSON =
-      new GsonBuilder().setPrettyPrinting().create();
-  private static final ContainerKeyInfo KEY_ONE =
-      new ContainerKeyInfo(1L, "vol1", -123L, "bucket1", -456L, "dir1/key1",
-          -789L);
-  private static final ContainerKeyInfo KEY_TWO =
-      new ContainerKeyInfo(2L, "vol1", 0L, "bucket1", 0L, "key2", 0L);
-  private static final ContainerKeyInfo KEY_THREE =
-      new ContainerKeyInfo(3L, "vol1", 0L, "bucket1", 0L, "key3", 0L);
-
-  private static final Map<Long, List<ContainerKeyInfo>> CONTAINER_KEYS =
-      new HashMap<>();
-
-  static {
-    List<ContainerKeyInfo> list1 = new ArrayList<>();
-    list1.add(KEY_ONE);
-    List<ContainerKeyInfo> list2 = new ArrayList<>();
-    list2.add(KEY_TWO);
-    List<ContainerKeyInfo> list3 = new ArrayList<>();
-    list3.add(KEY_THREE);
-    CONTAINER_KEYS.put(1L, list1);
-    CONTAINER_KEYS.put(2L, list2);
-    CONTAINER_KEYS.put(3L, list3);
-  }
-
-  private static final ContainerKeyInfoResponse KEYS_FOUND_OUTPUT =
-      new ContainerKeyInfoResponse(3, CONTAINER_KEYS);
-
-  private static final String KEYS_NOT_FOUND_OUTPUT =
-      "No keys were found for container IDs: [1, 2, 3]\n" +
-          "Keys processed: 3\n";
+  private static final Gson GSON = new GsonBuilder().setPrettyPrinting().create();
+  private String[] cmdArgs;
 
   @BeforeEach
   public void setup() throws IOException {
@@ -107,6 +80,7 @@ public void setup() throws IOException {
     pstderr = new PrintWriter(stderr);
 
     cmd = new CommandLine(new OzoneDebug())
+        .addSubcommand(new ContainerCommands())
         .addSubcommand(new FindContainerKeys())
         .setOut(pstdout)
         .setErr(pstderr);
@@ -115,6 +89,9 @@ public void setup() throws IOException {
         .setPath(tempDir.toPath()).addTable(KEY_TABLE).addTable(FILE_TABLE)
         .addTable(DIRECTORY_TABLE)
         .build();
+
+    cmdArgs =
+        new String[]{"find-keys", "--om-db", dbStore.getDbLocation().getAbsolutePath(), "--container-ids", "1 2 3"};
   }
 
   @AfterEach
@@ -127,44 +104,114 @@ public void shutdown() throws IOException {
   }
 
   @Test
-  void testWhenThereAreKeysForContainerIds() throws IOException {
-
-    // create keys for tables
+  void testFSO() throws Exception {
+    /*
+      Structure:
+      keyName (container id)
+
+      /vol1/bucet1
+        - key1 (1)
+        - dir1
+          - key2 (2)
+          - dir2
+            - key3 (3)
+            - key4 (3)
+        - key5 (4)
+     */
     long volumeId = -123L;
     long bucketId = -456L;
-    long dirObjectId = -789L;
-    createDirectory(volumeId, bucketId, bucketId, dirObjectId, "dir1");
-    createFile(volumeId, bucketId, "key1", -987L, dirObjectId, 1L);
-    createKey("key2", 2L);
-    createKey("key3", 3L);
-
-    String[] cmdArgs =
-        {"ckscanner", "--om-db", dbStore.getDbLocation().getAbsolutePath(),
-            "--container-ids", "1 2 3"};
+    long dirObjectId1 = -789L;
+    long dirObjectId2 = -788L;
+    createDirectory(volumeId, bucketId, bucketId, dirObjectId1, "dir1");
+    createDirectory(volumeId, bucketId, dirObjectId1, dirObjectId2, "dir2");
+    createFile(volumeId, bucketId, "key1", -987L, bucketId, 1L);
+    createFile(volumeId, bucketId, "key2", -986L, dirObjectId1, 2L);
+    createFile(volumeId, bucketId, "key3", -985L, dirObjectId2, 3L);
+    createFile(volumeId, bucketId, "key4", -984L, dirObjectId2, 3L);
+    createFile(volumeId, bucketId, "key5", -983L, dirObjectId2, 4L);
 
     closeDbStore();
 
     int exitCode = cmd.execute(cmdArgs);
-    Assertions.assertEquals(0, exitCode);
+    assertThat(exitCode).isEqualTo(0);
+
+    // Create expected response
+    List<ContainerKeyInfo> expectedKeysForContainer1 = new ArrayList<>();
+    expectedKeysForContainer1.add(new ContainerKeyInfo(1L, "vol1", volumeId, "bucket1", bucketId, "key1", bucketId));
+    List<ContainerKeyInfo> expectedKeysForContainer2 = new ArrayList<>();
+    expectedKeysForContainer2.add(
+        new ContainerKeyInfo(2L, "vol1", volumeId, "bucket1", bucketId, "dir1/key2", dirObjectId1));
+    List<ContainerKeyInfo> expectedKeysForContainer3 = new ArrayList<>();
+    expectedKeysForContainer3.add(
+        new ContainerKeyInfo(3L, "vol1", volumeId, "bucket1", bucketId, "dir1/dir2/key3", dirObjectId2));
+    expectedKeysForContainer3.add(
+        new ContainerKeyInfo(3L, "vol1", volumeId, "bucket1", bucketId, "dir1/dir2/key4", dirObjectId2));
+    Map<Long, List<ContainerKeyInfo>> expectedContainerIdToKeyInfos = new HashMap<>();
+    expectedContainerIdToKeyInfos.put(1L, expectedKeysForContainer1);
+    expectedContainerIdToKeyInfos.put(2L, expectedKeysForContainer2);
+    expectedContainerIdToKeyInfos.put(3L, expectedKeysForContainer3);
+    ContainerKeyInfoResponse expectedResponse = new ContainerKeyInfoResponse(5, expectedContainerIdToKeyInfos);
+    assertThat(GSON.fromJson(stdout.toString(), ContainerKeyInfoResponse.class)).isEqualTo(expectedResponse);
+
+    assertThat(stderr.toString()).isEmpty();
+  }
 
-    Assertions.assertEquals(
-        GSON.fromJson(stdout.toString(), ContainerKeyInfoResponse.class),
-        KEYS_FOUND_OUTPUT);
+  @Test
+  void testNonFSO() throws Exception {
+    /*
+      Structure:
+      keyName (container id)
+
+      /vol1/bucket1
+        - key1 (1)
+          - dir1/key2 (2)
+          - dir1/dir2/key3 (3)
+          - dir1/dir2/key4 (3)
+        - key5 (4)
+     */
+    createKey("key1", 1L);
+    createKey("dir1/key2", 2L);
+    createKey("dir1/dir2/key3", 3L);
+    createKey("dir1/dir2/key4", 3L);
+    createKey("key5", 4L);
 
-    Assertions.assertTrue(stderr.toString().isEmpty());
+    closeDbStore();
+
+    int exitCode = cmd.execute(cmdArgs);
+    assertThat(exitCode).isEqualTo(0);
+
+    // Create expected response
+    List<ContainerKeyInfo> expectedKeysForContainer1 = new ArrayList<>();
+    expectedKeysForContainer1.add(new ContainerKeyInfo(1L, "vol1", 0, "bucket1", 0, "key1", 0));
+    List<ContainerKeyInfo> expectedKeysForContainer2 = new ArrayList<>();
+    expectedKeysForContainer2.add(
+        new ContainerKeyInfo(2L, "vol1", 0, "bucket1", 0, "dir1/key2", 0));
+    List<ContainerKeyInfo> expectedKeysForContainer3 = new ArrayList<>();
+    expectedKeysForContainer3.add(
+        new ContainerKeyInfo(3L, "vol1", 0, "bucket1", 0, "dir1/dir2/key3", 0));
+    expectedKeysForContainer3.add(
+        new ContainerKeyInfo(3L, "vol1", 0, "bucket1", 0, "dir1/dir2/key4", 0));
+    Map<Long, List<ContainerKeyInfo>> expectedContainerIdToKeyInfos = new HashMap<>();
+    expectedContainerIdToKeyInfos.put(1L, expectedKeysForContainer1);
+    expectedContainerIdToKeyInfos.put(2L, expectedKeysForContainer2);
+    expectedContainerIdToKeyInfos.put(3L, expectedKeysForContainer3);
+    ContainerKeyInfoResponse expectedResponse = new ContainerKeyInfoResponse(5, expectedContainerIdToKeyInfos);
+    assertThat(GSON.fromJson(stdout.toString(), ContainerKeyInfoResponse.class)).isEqualTo(expectedResponse);
+
+    assertThat(stderr.toString()).isEmpty();
   }
 
   /**
    * Close db store because of the lock.
    */
   private void closeDbStore() throws IOException {
-    if (dbStore != null) {
+    if (dbStore != null && !dbStore.isClosed()) {
       dbStore.close();
     }
   }
 
   @Test
-  void testWhenThereAreNotKeysForContainerIds() throws IOException {
+  void testWhenThereAreNoKeysForContainerIds() throws Exception {
 
     // create keys for tables
     long volumeId = -123L;
@@ -173,83 +220,67 @@ void testWhenThereAreNotKeysForContainerIds() throws IOException {
     createKey("key2", 5L);
     createKey("key3", 6L);
 
-    String[] cmdArgs =
-        {"ckscanner", "--om-db", dbStore.getDbLocation().getAbsolutePath(),
-            "--container-ids", "1 2 3"};
-
     closeDbStore();
 
     int exitCode = cmd.execute(cmdArgs);
-    Assertions.assertEquals(0, exitCode);
+    assertThat(exitCode).isEqualTo(0);
 
-    Assertions.assertTrue(stderr.toString().contains(KEYS_NOT_FOUND_OUTPUT));
+    assertThat(stderr.toString()).contains("No keys were found for container IDs: [1, 2, 3]\n" + "Keys processed: 3\n");
 
-    Assertions.assertTrue(stdout.toString().isEmpty());
+    assertThat(stdout.toString()).isEmpty();
   }
 
-  private void createFile(long volumeId, long bucketId, String keyName,
-                          long objectId, long parentId, long containerId)
-      throws IOException {
-    Table<byte[], byte[]> table = dbStore.getTable(FILE_TABLE);
+  private void createFile(long volumeId, long bucketId, String keyName, long objectId, long parentId, long containerId)
+      throws Exception {
+    try (Table<byte[], byte[]> table = dbStore.getTable(FILE_TABLE)) {
+      // format: /volumeId/bucketId/parentId(bucketId)/keyName
+      String key =
+          OM_KEY_PREFIX + volumeId + OM_KEY_PREFIX + bucketId + OM_KEY_PREFIX + parentId + OM_KEY_PREFIX + keyName;
 
-    // format: /volumeId/bucketId/parentId(bucketId)/keyName
-    String key =
-        OM_KEY_PREFIX + volumeId + OM_KEY_PREFIX +
-            bucketId + OM_KEY_PREFIX + parentId +
-            OM_KEY_PREFIX + keyName;
+      OmKeyInfo value = getOmKeyInfo("vol1", "bucket1", keyName, containerId, objectId, parentId);
 
-    OmKeyInfo value =
-        getOmKeyInfo("vol1", "bucket1", keyName, containerId, objectId,
-            parentId);
-
-    table.put(key.getBytes(UTF_8),
-        value.getProtobuf(ClientVersion.CURRENT_VERSION).toByteArray());
+      table.put(key.getBytes(UTF_8), value.getProtobuf(ClientVersion.CURRENT_VERSION).toByteArray());
+    }
   }
 
-  private void createKey(String keyName, long containerId) throws IOException {
-    Table<byte[], byte[]> table = dbStore.getTable(KEY_TABLE);
+  private void createKey(String keyName, long containerId) throws Exception {
+    try (Table<byte[], byte[]> table = dbStore.getTable(KEY_TABLE)) {
+      String volumeName = "vol1";
+      String bucketName = "bucket1";
+      // format: /volumeName/bucketName/keyName
+      String key = OM_KEY_PREFIX + volumeName + OM_KEY_PREFIX + bucketName + OM_KEY_PREFIX + keyName;
 
-    String volumeName = "vol1";
-    String bucketName = "bucket1";
-    // format: /volumeName/bucketName/keyName
-    String key = OM_KEY_PREFIX + volumeName + OM_KEY_PREFIX + bucketName +
-        OM_KEY_PREFIX + keyName;
+      // generate table value
+      OmKeyInfo value = getOmKeyInfo(volumeName, bucketName, keyName, containerId, 0, 0);
 
-    // generate table value
-    OmKeyInfo value =
-        getOmKeyInfo(volumeName, bucketName, keyName, containerId, 0, 0);
-
-    table.put(key.getBytes(UTF_8),
-        value.getProtobuf(ClientVersion.CURRENT_VERSION).toByteArray());
+      table.put(key.getBytes(UTF_8), value.getProtobuf(ClientVersion.CURRENT_VERSION).toByteArray());
+    }
   }
 
-  private void createDirectory(long volumeId, long bucketId, long parentId,
-                               long objectId, String keyName)
-      throws IOException {
-    Table<byte[], byte[]> table = dbStore.getTable(DIRECTORY_TABLE);
+  private void createDirectory(long volumeId, long bucketId, long parentId, long objectId, String keyName)
+      throws Exception {
+    try (Table<byte[], byte[]> table = dbStore.getTable(DIRECTORY_TABLE)) {
 
-    // format: /volumeId/bucketId/parentId(bucketId)/keyName
-    String key =
-        OM_KEY_PREFIX + volumeId + OM_KEY_PREFIX + bucketId + OM_KEY_PREFIX +
-            parentId + OM_KEY_PREFIX + keyName;
+      // format: /volumeId/bucketId/parentId(bucketId)/keyName
+      String key =
+          OM_KEY_PREFIX + volumeId + OM_KEY_PREFIX + bucketId + OM_KEY_PREFIX + parentId + OM_KEY_PREFIX + keyName;
 
-    OmDirectoryInfo value =
-        OMRequestTestUtils.createOmDirectoryInfo(keyName, objectId, parentId);
+      OmDirectoryInfo value = OMRequestTestUtils.createOmDirectoryInfo(keyName, objectId, parentId);
 
-    table.put(key.getBytes(UTF_8), value.getProtobuf().toByteArray());
+      table.put(key.getBytes(UTF_8), value.getProtobuf().toByteArray());
+    }
   }
 
   private static OmKeyInfo getOmKeyInfo(String volumeName, String bucketName,
                                         String keyName, long containerId,
                                         long objectId,
                                         long parentId) {
-    return OMRequestTestUtils.createOmKeyInfo(volumeName, bucketName,
-        keyName, HddsProtos.ReplicationType.STAND_ALONE,
-        HddsProtos.ReplicationFactor.ONE, objectId, parentId, 1, 1, 1, false,
-        new ArrayList<>(
-            Collections.singletonList(
-                new OmKeyLocationInfo.Builder().setBlockID(
-                    new BlockID(containerId, 1)).build())));
+    return OMRequestTestUtils
+        .createOmKeyInfo(volumeName, bucketName, keyName, RatisReplicationConfig.getInstance(ONE), objectId, parentId,
+            new OmKeyLocationInfoGroup(0L, Collections.singletonList(new OmKeyLocationInfo.Builder()
+                .setBlockID(new BlockID(containerId, 1))
+                .build())))
+        .build();
   }
 
 }

hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java

@@ -510,7 +510,7 @@ default String getOzonePathKey(long volumeId, long bucketId,
   }
 
   /**
-   * Given ozone path key, component id, return the corresponding 
+   * Given ozone path key, component id, return the corresponding
    * DB path key for delete table.
    *
    * @param objectId - object Id
@@ -611,4 +611,26 @@ String getMultipartKey(long volumeId, long bucketId,
    */
   boolean containsIncompleteMPUs(String volume, String bucket)
       throws IOException;
+
+  /**
+   * Helper method to generate /volumeId/bucketId/ DB key prefix from given
+   * volume name and bucket name as a prefix for FSO buckets.
+   *
+   * @param volumeName volume name
+   * @param bucketName bucket name
+   * @return /volumeId/bucketId/
+   *    e.g. /-9223372036854772480/-9223372036854771968/
+   */
+  String getOzonePathKeyForFso(String volumeName, String bucketName)
+      throws IOException;
+
+  /**
+   * Helper method to generate /volumeId/bucketId DB key prefix from given
+   * volume id and bucket id as a prefix for FSO buckets.
+   * @param volumeId volume id
+   * @param bucketId bucket id
+   * @return /volumeId/bucketId
+   *    e.g. /-9223372036854772480/-9223372036854771968/
+   */
+  String getOzonePathKeyForFso(long volumeId, long bucketId);
 }