[GitHub] [hadoop-ozone] elek commented on a change in pull request #1298: HDDS-3869. Use different column families for datanode block and metadata

[GitBox <gi...@apache.org>]

elek commented on a change in pull request #1298:
URL: https://github.com/apache/hadoop-ozone/pull/1298#discussion_r483555964



##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueBlockIterator.java
##########
@@ -88,196 +86,314 @@ public void setUp() throws Exception {
     conf = new OzoneConfiguration();
     conf.set(HDDS_DATANODE_DIR_KEY, testRoot.getAbsolutePath());
     volumeSet = new MutableVolumeSet(UUID.randomUUID().toString(), conf);
+
+    containerData = new KeyValueContainerData(105L,
+            layout,
+            (long) StorageUnit.GB.toBytes(1), UUID.randomUUID().toString(),
+            UUID.randomUUID().toString());
+    // Init the container.
+    container = new KeyValueContainer(containerData, conf);
+    container.create(volumeSet, new RoundRobinVolumeChoosingPolicy(), UUID
+            .randomUUID().toString());
+    db = BlockUtils.getDB(containerData, conf);
   }
 
 
   @After
-  public void tearDown() {
+  public void tearDown() throws Exception {
+    db.close();
+    db.cleanup();
     volumeSet.shutdown();
     FileUtil.fullyDelete(testRoot);
   }
 
   @Test
   public void testKeyValueBlockIteratorWithMixedBlocks() throws Exception {
-
-    long containerID = 100L;
-    int deletedBlocks = 5;
+    int deletingBlocks = 5;
     int normalBlocks = 5;
-    createContainerWithBlocks(containerID, normalBlocks, deletedBlocks);
-    String containerPath = new File(containerData.getMetadataPath())
-        .getParent();
-    try(KeyValueBlockIterator keyValueBlockIterator = new KeyValueBlockIterator(
-        containerID, new File(containerPath))) {
+    Map<String, List<Long>> blockIDs = createContainerWithBlocks(CONTAINER_ID,
+            normalBlocks,
+            deletingBlocks);

Review comment:
       > @elek, any idea if continuation indent should be 4 or does it not matter? Checkstyle CI seems to not care about it.
   
   I don't think there is any rule. It can be reasonable to introduce new one (to reduce noise during merges) but in that case we should provide good auto-formatting rules to avoid any overhead.
   
   (Similar to the import format, we can improve checkstyle, but we also need autoformatter rules).
   
   I think @maobaolong had a few ideas, but the effort is blocked by #921 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org