xichen01 commented on code in PR #7467:
URL: https://github.com/apache/ozone/pull/7467#discussion_r1855228190


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -240,11 +250,28 @@ private boolean displayTable(ManagedRocksIterator 
iterator,
       return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
+    // If there are no parent directories, create them
+    File dirFile = new File(fileName);
+    if (!dirFile.exists()) {
+      boolean flg = dirFile.mkdirs();

Review Comment:
   If we not give the `--max-records-per-file` we need not create the 
directory, in this scenario, `filename` should be a file.
   



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -240,11 +250,28 @@ private boolean displayTable(ManagedRocksIterator 
iterator,
       return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
+    // If there are no parent directories, create them
+    File dirFile = new File(fileName);

Review Comment:
   How about just add suffix (like: x.0, x.1 x.1 ...)on the output file, 
instead of crate a directory? just like previous design of this PR.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/debug/TestLDBCli.java:
##########
@@ -341,6 +341,43 @@ void testScanOfPipelinesWhenNoData() throws IOException {
     assertEquals("", stderr.toString());
   }
 
+  @Test
+  void testScanWithRecordsPerFile() throws IOException {
+    // Prepare dummy table
+    prepareTable(KEY_TABLE, false);
+
+    String scanDir1 = tempDir.getAbsolutePath() + "/scandir1";
+    // Prepare scan args
+    List<String> completeScanArgs1 = new ArrayList<>(Arrays.asList(
+        "--db", dbStore.getDbLocation().getAbsolutePath(),
+        "scan",
+        "--column-family", KEY_TABLE, "--out", scanDir1,
+        "--max-records-per-file", "2"));
+
+    File tmpDir1 = new File(scanDir1);
+    tmpDir1.deleteOnExit();
+
+    int exitCode1 = cmd.execute(completeScanArgs1.toArray(new String[0]));
+    assertEquals(0, exitCode1);
+    assertTrue(tmpDir1.isDirectory());
+    assertEquals(3, tmpDir1.listFiles().length);

Review Comment:
   Can use `records_count / max_records_per_file # need Math.ceil` replace this 
hard code, we can add a parameter  records_count  for method `prepareTable`.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/debug/TestLDBCli.java:
##########
@@ -341,6 +341,43 @@ void testScanOfPipelinesWhenNoData() throws IOException {
     assertEquals("", stderr.toString());
   }
 
+  @Test
+  void testScanWithRecordsPerFile() throws IOException {
+    // Prepare dummy table
+    prepareTable(KEY_TABLE, false);
+
+    String scanDir1 = tempDir.getAbsolutePath() + "/scandir1";
+    // Prepare scan args
+    List<String> completeScanArgs1 = new ArrayList<>(Arrays.asList(
+        "--db", dbStore.getDbLocation().getAbsolutePath(),
+        "scan",
+        "--column-family", KEY_TABLE, "--out", scanDir1,
+        "--max-records-per-file", "2"));
+
+    File tmpDir1 = new File(scanDir1);
+    tmpDir1.deleteOnExit();
+
+    int exitCode1 = cmd.execute(completeScanArgs1.toArray(new String[0]));

Review Comment:
   We can add an assert to confirm that the output file is correct JSON format.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to