You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2023/03/18 02:21:25 UTC

[GitHub] [ozone] smengcl opened a new pull request, #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

smengcl opened a new pull request, #4420:
URL: https://github.com/apache/ozone/pull/4420

   ## What changes were proposed in this pull request?
   
   1. Print proper JSON **object** (`{ }`) if `--with-keys=true`.
   2. Print proper JSON **array** (`[ ]`) if `--with-keys=false`.
   3. `--with-keys` now defaults to `true`. Tweak some option names. Improve error messages. Should have no compatiblity concern what-so-ever as this is intended to be a debug tool.
   4. Rewritten and parameterized `TestLDBCli`. New test cases are added.
   5. Refactor `DBScanner` for readability and maintainability.
   
   ### Note
   
   Regarding the core serialization logic in `DBScanner`, I've chosen to stick to the current `Gson` approach that serializes **each** entry and immediately printing it. It should consume less memory than [gathering all entries](https://github.com/apache/ozone/blob/04cd54ce6593024dc98a9867e1fd829c4f25f85a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java#L129-L135) then serializing and printing it (could OOM if batch limit is too high like a few billion entries, while the current approach should work just fine).
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-6064
   
   ## How was this patch tested?
   
   - Rewritten `TestLDBCli` and added new test cases.
     - Fully migrated to JUnit 5.
     - Fully parameterized tests. Able to easily add more params combination in the future.
       - Use `Named.of()` to describe each parameter for maintainability.
       - Pls feel free to contribute more test cases now that adding a new case is just a matter of a few lines. :)
     - Rewritten `keyTable` tests, which is heavily inspired by @adoroszlai 's change in #2917 (in which `testOMDB` is split into multiple test cases).
     - Datanode DB `block_data` table schema V3 and V2 tests are completely rewritten.
     - This took longer for me than fixing and refactoring `DBScanner` :D
   
   <img width="605" alt="IntelliJ" src="https://user-images.githubusercontent.com/50227127/226076360-2c518e58-77b6-4e83-a00b-138ec57d7c79.png">
   
   ## Future potential improvement
   
   - [ ] For SchemaV3 `block_data` table, we could further nest entries inside another map. With the outer-most layer being `containerId`.
   
   e.g. Currently (unchanged in this PR), the JSON key for an entry inside V3 `block_data` is `containerId: blockId`:
   
   ```shell
   { "2: 3": {
     "blockID": {
       "containerBlockID": {
         "containerID": 2,
         "localID": 3
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   }, "2: 4": {
     "blockID": {
       "containerBlockID": {
         "containerID": 2,
         "localID": 4
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   } }
   ```
   
   With another layer, the output could become even cleaner, making it easier to be filtered using `jq`:
   
   ```shell
   {
     "2": {
       "3": {
         "blockID": {
           "containerBlockID": {
             "containerID": 2,
             "localID": 3
           },
           "blockCommitSequenceId": 0
         },
         "metadata": {},
         "size": 0
       },
       "4": {
         "blockID": {
           "containerBlockID": {
             "containerID": 2,
             "localID": 4
           },
           "blockCommitSequenceId": 0
         },
         "metadata": {},
         "size": 0
       }
     }
   }
   ```
   
   ## Example output with this PR
   
   Outputs are from integration test's `stdout` with dummy data for demo purpose. Command lines are reconstructed from test parameters. Actual CLI output can differ.
   
   ### `ozone debug ldb --db=/data/metadata/om.db scan --column-family=keyTable --limit=1`
   
   ```shell
   { "key1": {
     "volumeName": "vol1",
     "bucketName": "buck1",
     "keyName": "key1",
     "dataSize": 1000,
     "keyLocationVersions": [
       {
         "version": 0,
         "locationVersionMap": {
           "0": []
         },
         "isMultipartKey": false
       }
     ],
     "creationTime": 1679105144793,
     "modificationTime": 1679105144793,
     "replicationConfig": {
       "replicationFactor": "ONE"
     },
     "isFile": false,
     "fileName": "key1",
     "acls": [],
     "parentObjectID": 0,
     "objectID": 0,
     "updateID": 0,
     "metadata": {}
   } }
   ```
   
   ### `ozone debug ldb --db=/data/metadata/om.db scan --column-family=keyTable`
   
   ```shell
   { "key1": {
     "volumeName": "vol1",
     "bucketName": "buck1",
     "keyName": "key1",
     "dataSize": 1000,
     "keyLocationVersions": [
       {
         "version": 0,
         "locationVersionMap": {
           "0": []
         },
         "isMultipartKey": false
       }
     ],
     "creationTime": 1679102602165,
     "modificationTime": 1679102602166,
     "replicationConfig": {
       "replicationFactor": "ONE"
     },
     "isFile": false,
     "fileName": "key1",
     "acls": [],
     "parentObjectID": 0,
     "objectID": 0,
     "updateID": 0,
     "metadata": {}
   }, "key2": {
     "volumeName": "vol1",
     "bucketName": "buck1",
     "keyName": "key2",
     "dataSize": 1000,
     "keyLocationVersions": [
       {
         "version": 0,
         "locationVersionMap": {
           "0": []
         },
         "isMultipartKey": false
       }
     ],
     "creationTime": 1679102602279,
     "modificationTime": 1679102602279,
     "replicationConfig": {
       "replicationFactor": "ONE"
     },
     "isFile": false,
     "fileName": "key2",
     "acls": [],
     "parentObjectID": 0,
     "objectID": 0,
     "updateID": 0,
     "metadata": {}
   }, "key3": {
     "volumeName": "vol1",
     "bucketName": "buck1",
     "keyName": "key3",
     "dataSize": 1000,
     "keyLocationVersions": [
       {
         "version": 0,
         "locationVersionMap": {
           "0": []
         },
         "isMultipartKey": false
       }
     ],
     "creationTime": 1679102602282,
     "modificationTime": 1679102602282,
     "replicationConfig": {
       "replicationFactor": "ONE"
     },
     "isFile": false,
     "fileName": "key3",
     "acls": [],
     "parentObjectID": 0,
     "objectID": 0,
     "updateID": 0,
     "metadata": {}
   }, "key4": {
     "volumeName": "vol1",
     "bucketName": "buck1",
     "keyName": "key4",
     "dataSize": 1000,
     "keyLocationVersions": [
       {
         "version": 0,
         "locationVersionMap": {
           "0": []
         },
         "isMultipartKey": false
       }
     ],
     "creationTime": 1679102602284,
     "modificationTime": 1679102602284,
     "replicationConfig": {
       "replicationFactor": "ONE"
     },
     "isFile": false,
     "fileName": "key4",
     "acls": [],
     "parentObjectID": 0,
     "objectID": 0,
     "updateID": 0,
     "metadata": {}
   }, "key5": {
     "volumeName": "vol1",
     "bucketName": "buck1",
     "keyName": "key5",
     "dataSize": 1000,
     "keyLocationVersions": [
       {
         "version": 0,
         "locationVersionMap": {
           "0": []
         },
         "isMultipartKey": false
       }
     ],
     "creationTime": 1679102602286,
     "modificationTime": 1679102602286,
     "replicationConfig": {
       "replicationFactor": "ONE"
     },
     "isFile": false,
     "fileName": "key5",
     "acls": [],
     "parentObjectID": 0,
     "objectID": 0,
     "updateID": 0,
     "metadata": {}
   } }
   ```
   
   ### `ozone debug ldb --db=/data/metadata/om.db scan --column-family=keyTable --limit=2 --with-keys=false`
   
   ```shell
   
   ```
   
   ### `ozone debug ldb --db=/data/hdds/hdds/CID-UUID1/DS-UUID2/container.db scan --column-family=block_data --dn-schema=V3 --container-id=2 --limit=2`
   
   ```shell
   { "2: 3": {
     "blockID": {
       "containerBlockID": {
         "containerID": 2,
         "localID": 3
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   }, "2: 4": {
     "blockID": {
       "containerBlockID": {
         "containerID": 2,
         "localID": 4
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   } }
   ```
   
   ### `ozone debug ldb --db=/data/hdds/hdds/CID-UUID1/DS-UUID2/container.db scan --column-family=block_data --dn-schema=V2 --limit=4`
   
   ```shell
   { "1": {
     "blockID": {
       "containerBlockID": {
         "containerID": 1,
         "localID": 1
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   }, "2": {
     "blockID": {
       "containerBlockID": {
         "containerID": 1,
         "localID": 2
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   }, "3": {
     "blockID": {
       "containerBlockID": {
         "containerID": 2,
         "localID": 3
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   }, "4": {
     "blockID": {
       "containerBlockID": {
         "containerID": 2,
         "localID": 4
       },
       "blockCommitSequenceId": 0
     },
     "metadata": {},
     "size": 0
   } }
   ```


-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146899627


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   Yes. But Schema V3 table key is not fully composed of ASCII characters (intended for seekability I presume).
   
   e.g. for `block_data`:
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L42-L43
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L150-L154
   
   The logic to print it is left untouched. Arguably all valuable key info is extracted here already.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1148099011


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,210 +173,192 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
-      DBColumnFamilyDefinition dbColumnFamilyDefinition, boolean schemaV3)
-          throws IOException {
-    List<Object> outputs = new ArrayList<>();
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               boolean schemaV3)
+      throws IOException {
 
-    if (startKey != null) {
-      iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
+    if (fileName == null) {
+      // Print to stdout
+      return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
-
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
-        Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
-        } else {
-          System.out.println(result.toString());
-        }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
-      }
+    // Write to file output
+    try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+      return displayTable(iterator, dbColumnFamilyDef, out, schemaV3);
     }
-    return outputs;
   }
 
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               PrintWriter out,
+                               boolean schemaV3)
+      throws IOException {
 
-  public RDBParser getParent() {
-    return parent;
-  }
+    if (startKey != null) {
+      iterator.get().seek(getValueObject(dbColumnFamilyDef));
+    }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDef.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
+        Gson gson = new GsonBuilder().setPrettyPrinting().create();
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String keyStr = key.toString();
+          if (index > keyStr.length()) {
+            err().println("Error: Invalid SchemaV3 table key length. "
+                + "Is this a V2 table? Try again with --dn-schema=V2");
+            return false;
+          }
+          String cid = keyStr.substring(0, index);
+          String blockId = keyStr.substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) +
+              keySeparatorSchemaV3 +
+              blockId));
+        } else {
+          sb.append(gson.toJson(key));
+        }
+        sb.append(": ");

Review Comment:
   I believe Schema V1/V2 keys are already printed as is on Line 234. The `: ` is a manually constructed JSON separator between a key-value pair.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146956308


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   So just change the printed separator from `: ` to `|` then?
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L98



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146957755


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   Yes. Instead of `: ` as the separator, use `DatanodeConfiguration#getContainerSchemaV3Separator`.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1140903416


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,209 +154,168 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
         } else {
-          System.out.println(result.toString());
+          sb.append(gson.toJson(key));
         }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
+        sb.append(": ");
       }
-    }
-    return outputs;
-  }
-
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
-
-  public RDBParser getParent() {
-    return parent;
-  }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
-
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
-
-  public List<Object> getScannedObjects() {
-    return scannedObjects;
-  }
+      Gson gson = new GsonBuilder().setPrettyPrinting().create();
+      Object o = dbColumnFamilyDefinition.getValueCodec()
+          .fromPersistedFormat(iterator.get().value());
+      sb.append(gson.toJson(o));
 
-  public static void setFileName(String name) {
-    DBScanner.fileName = name;
-  }
-
-  public static void setContainerId(long id) {
-    DBScanner.containerId = id;
-  }
+      iterator.get().next();
+      ++count;
+      if (withinLimit(count) && iterator.get().isValid()) {
+        // If this is not the last entry, append comma
+        sb.append(", ");
+      }
 
-  public static void setDnDBSchemaVersion(String version) {
-    DBScanner.dnDBSchemaVersion = version;
-  }
+      out.print(sb);
+    }
 
-  public static void setWithKey(boolean withKey) {
-    DBScanner.withKey = withKey;
+    if (withKey) {
+      // End JSON object
+      out.println(" }");
+    } else {
+      // End JSON array
+      out.println(" ]");
+    }
   }
 
-  public static void setShowCount(boolean showCount) {
-    DBScanner.showCount = showCount;
+  private boolean withinLimit(long i) {
+    return limit == -1L || i < limit;
   }
 
-  private static ColumnFamilyHandle getColumnFamilyHandle(
+  private ColumnFamilyHandle getColumnFamilyHandle(
             byte[] name, List<ColumnFamilyHandle> columnFamilyHandles) {
     return columnFamilyHandles
             .stream()
             .filter(
               handle -> {
                 try {
                   return Arrays.equals(handle.getName(), name);
-                    } catch (Exception ex) {
+                } catch (Exception ex) {
                   throw new RuntimeException(ex);
-                    }
+                }
               })
             .findAny()
             .orElse(null);
   }
 
-  private void constructColumnFamilyMap(DBDefinition dbDefinition) {
-    if (dbDefinition == null) {
-      System.out.println("Incorrect Db Path");
-      return;
-    }
-    this.columnFamilyMap = new HashMap<>();
-    DBColumnFamilyDefinition[] columnFamilyDefinitions = dbDefinition
-            .getColumnFamilies();
-    for (DBColumnFamilyDefinition definition:columnFamilyDefinitions) {
-      LOG.info("Added definition for table: {}", definition.getTableName());
-      this.columnFamilyMap.put(definition.getTableName(), definition);
-    }
-  }
-
-  @Override
-  public Void call() throws Exception {
-    List<ColumnFamilyDescriptor> cfs =
-        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
-
-    final List<ColumnFamilyHandle> columnFamilyHandleList =
-        new ArrayList<>();
-    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),
-            cfs, columnFamilyHandleList);
-    this.printAppropriateTable(columnFamilyHandleList,
-           rocksDB, parent.getDbPath());
-    return null;
-  }
-
-  private void printAppropriateTable(
-          List<ColumnFamilyHandle> columnFamilyHandleList,
-          ManagedRocksDB rocksDB, String dbPath)
+  /**
+   * Main table printing logic.
+   * User-provided args are not in the arg list.
+   */
+  private void printTable(List<ColumnFamilyHandle> columnFamilyHandleList,
+      ManagedRocksDB rocksDB, String dbPath)
       throws IOException, RocksDBException {
+
     if (limit < 1 && limit != -1) {
       throw new IllegalArgumentException(
               "List length should be a positive number. Only allowed negative" +
                   " number is -1 which is to dump entire table");
     }
     dbPath = removeTrailingSlashIfNeeded(dbPath);
     DBDefinitionFactory.setDnDBSchemaVersion(dnDBSchemaVersion);
-    this.constructColumnFamilyMap(DBDefinitionFactory.
-            getDefinition(Paths.get(dbPath), new OzoneConfiguration()));
-    if (this.columnFamilyMap != null) {
-      if (!this.columnFamilyMap.containsKey(tableName)) {
-        System.out.print("Table with name:" + tableName + " does not exist");
-      } else {
-        DBColumnFamilyDefinition columnFamilyDefinition =
-                this.columnFamilyMap.get(tableName);
-        ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
-                columnFamilyDefinition.getTableName()
-                        .getBytes(StandardCharsets.UTF_8),
-                columnFamilyHandleList);
-        if (columnFamilyHandle == null) {
-          throw new IllegalArgumentException("columnFamilyHandle is null");
-        }
-        if (showCount) {
-          long keyCount = rocksDB.get().getLongProperty(columnFamilyHandle,
-              RocksDatabase.ESTIMATE_NUM_KEYS);
-          System.out.println(keyCount);
-          return;
-        }
-        ManagedRocksIterator iterator;
-        if (containerId > 0 && dnDBSchemaVersion != null &&
-            dnDBSchemaVersion.equals("V3")) {
-          ManagedReadOptions readOptions = new ManagedReadOptions();
-          readOptions.setIterateUpperBound(new ManagedSlice(
-              FixedLengthStringUtils.string2Bytes(
-                  DatanodeSchemaThreeDBDefinition.getContainerKeyPrefix(
-                  containerId + 1))));
-          iterator = new ManagedRocksIterator(
-              rocksDB.get().newIterator(columnFamilyHandle, readOptions));
-          iterator.get().seek(FixedLengthStringUtils.string2Bytes(
+    DBDefinition dbDefinition = DBDefinitionFactory.getDefinition(
+        Paths.get(dbPath), new OzoneConfiguration());
+    if (dbDefinition == null) {
+      err().println("Error: Incorrect DB Path");
+      return;
+    }
+
+    Map<String, DBColumnFamilyDefinition> columnFamilyMap = new HashMap<>();
+    for (DBColumnFamilyDefinition cfDef : dbDefinition.getColumnFamilies()) {
+      LOG.info("Found table: {}", cfDef.getTableName());
+      columnFamilyMap.put(cfDef.getTableName(), cfDef);
+    }
+    if (!columnFamilyMap.containsKey(tableName)) {
+      err().print("Error: Table with name '" + tableName + "' not found");
+      return;
+    }
+
+    DBColumnFamilyDefinition columnFamilyDefinition =
+        columnFamilyMap.get(tableName);
+    ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
+        columnFamilyDefinition.getTableName().getBytes(UTF_8),
+        columnFamilyHandleList);
+    if (columnFamilyHandle == null) {
+      throw new IllegalStateException("columnFamilyHandle is null");

Review Comment:
   fyi, rationale behind the switch from `IllegalArgumentException` to `IllegalStateException`:
   
   https://github.com/apache/ozone/pull/4376#discussion_r1139455970



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1140903416


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,209 +154,168 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
         } else {
-          System.out.println(result.toString());
+          sb.append(gson.toJson(key));
         }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
+        sb.append(": ");
       }
-    }
-    return outputs;
-  }
-
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
-
-  public RDBParser getParent() {
-    return parent;
-  }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
-
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
-
-  public List<Object> getScannedObjects() {
-    return scannedObjects;
-  }
+      Gson gson = new GsonBuilder().setPrettyPrinting().create();
+      Object o = dbColumnFamilyDefinition.getValueCodec()
+          .fromPersistedFormat(iterator.get().value());
+      sb.append(gson.toJson(o));
 
-  public static void setFileName(String name) {
-    DBScanner.fileName = name;
-  }
-
-  public static void setContainerId(long id) {
-    DBScanner.containerId = id;
-  }
+      iterator.get().next();
+      ++count;
+      if (withinLimit(count) && iterator.get().isValid()) {
+        // If this is not the last entry, append comma
+        sb.append(", ");
+      }
 
-  public static void setDnDBSchemaVersion(String version) {
-    DBScanner.dnDBSchemaVersion = version;
-  }
+      out.print(sb);
+    }
 
-  public static void setWithKey(boolean withKey) {
-    DBScanner.withKey = withKey;
+    if (withKey) {
+      // End JSON object
+      out.println(" }");
+    } else {
+      // End JSON array
+      out.println(" ]");
+    }
   }
 
-  public static void setShowCount(boolean showCount) {
-    DBScanner.showCount = showCount;
+  private boolean withinLimit(long i) {
+    return limit == -1L || i < limit;
   }
 
-  private static ColumnFamilyHandle getColumnFamilyHandle(
+  private ColumnFamilyHandle getColumnFamilyHandle(
             byte[] name, List<ColumnFamilyHandle> columnFamilyHandles) {
     return columnFamilyHandles
             .stream()
             .filter(
               handle -> {
                 try {
                   return Arrays.equals(handle.getName(), name);
-                    } catch (Exception ex) {
+                } catch (Exception ex) {
                   throw new RuntimeException(ex);
-                    }
+                }
               })
             .findAny()
             .orElse(null);
   }
 
-  private void constructColumnFamilyMap(DBDefinition dbDefinition) {
-    if (dbDefinition == null) {
-      System.out.println("Incorrect Db Path");
-      return;
-    }
-    this.columnFamilyMap = new HashMap<>();
-    DBColumnFamilyDefinition[] columnFamilyDefinitions = dbDefinition
-            .getColumnFamilies();
-    for (DBColumnFamilyDefinition definition:columnFamilyDefinitions) {
-      LOG.info("Added definition for table: {}", definition.getTableName());
-      this.columnFamilyMap.put(definition.getTableName(), definition);
-    }
-  }
-
-  @Override
-  public Void call() throws Exception {
-    List<ColumnFamilyDescriptor> cfs =
-        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
-
-    final List<ColumnFamilyHandle> columnFamilyHandleList =
-        new ArrayList<>();
-    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),
-            cfs, columnFamilyHandleList);
-    this.printAppropriateTable(columnFamilyHandleList,
-           rocksDB, parent.getDbPath());
-    return null;
-  }
-
-  private void printAppropriateTable(
-          List<ColumnFamilyHandle> columnFamilyHandleList,
-          ManagedRocksDB rocksDB, String dbPath)
+  /**
+   * Main table printing logic.
+   * User-provided args are not in the arg list.
+   */
+  private void printTable(List<ColumnFamilyHandle> columnFamilyHandleList,
+      ManagedRocksDB rocksDB, String dbPath)
       throws IOException, RocksDBException {
+
     if (limit < 1 && limit != -1) {
       throw new IllegalArgumentException(
               "List length should be a positive number. Only allowed negative" +
                   " number is -1 which is to dump entire table");
     }
     dbPath = removeTrailingSlashIfNeeded(dbPath);
     DBDefinitionFactory.setDnDBSchemaVersion(dnDBSchemaVersion);
-    this.constructColumnFamilyMap(DBDefinitionFactory.
-            getDefinition(Paths.get(dbPath), new OzoneConfiguration()));
-    if (this.columnFamilyMap != null) {
-      if (!this.columnFamilyMap.containsKey(tableName)) {
-        System.out.print("Table with name:" + tableName + " does not exist");
-      } else {
-        DBColumnFamilyDefinition columnFamilyDefinition =
-                this.columnFamilyMap.get(tableName);
-        ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
-                columnFamilyDefinition.getTableName()
-                        .getBytes(StandardCharsets.UTF_8),
-                columnFamilyHandleList);
-        if (columnFamilyHandle == null) {
-          throw new IllegalArgumentException("columnFamilyHandle is null");
-        }
-        if (showCount) {
-          long keyCount = rocksDB.get().getLongProperty(columnFamilyHandle,
-              RocksDatabase.ESTIMATE_NUM_KEYS);
-          System.out.println(keyCount);
-          return;
-        }
-        ManagedRocksIterator iterator;
-        if (containerId > 0 && dnDBSchemaVersion != null &&
-            dnDBSchemaVersion.equals("V3")) {
-          ManagedReadOptions readOptions = new ManagedReadOptions();
-          readOptions.setIterateUpperBound(new ManagedSlice(
-              FixedLengthStringUtils.string2Bytes(
-                  DatanodeSchemaThreeDBDefinition.getContainerKeyPrefix(
-                  containerId + 1))));
-          iterator = new ManagedRocksIterator(
-              rocksDB.get().newIterator(columnFamilyHandle, readOptions));
-          iterator.get().seek(FixedLengthStringUtils.string2Bytes(
+    DBDefinition dbDefinition = DBDefinitionFactory.getDefinition(
+        Paths.get(dbPath), new OzoneConfiguration());
+    if (dbDefinition == null) {
+      err().println("Error: Incorrect DB Path");
+      return;
+    }
+
+    Map<String, DBColumnFamilyDefinition> columnFamilyMap = new HashMap<>();
+    for (DBColumnFamilyDefinition cfDef : dbDefinition.getColumnFamilies()) {
+      LOG.info("Found table: {}", cfDef.getTableName());
+      columnFamilyMap.put(cfDef.getTableName(), cfDef);
+    }
+    if (!columnFamilyMap.containsKey(tableName)) {
+      err().print("Error: Table with name '" + tableName + "' not found");
+      return;
+    }
+
+    DBColumnFamilyDefinition columnFamilyDefinition =
+        columnFamilyMap.get(tableName);
+    ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
+        columnFamilyDefinition.getTableName().getBytes(UTF_8),
+        columnFamilyHandleList);
+    if (columnFamilyHandle == null) {
+      throw new IllegalStateException("columnFamilyHandle is null");

Review Comment:
   fyi, rationale behind the switch from `IllegalArgumentException` to `IllegalStateException`:
   
   https://github.com/apache/ozone/pull/4376#discussion_r1139456078



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1148099011


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,210 +173,192 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
-      DBColumnFamilyDefinition dbColumnFamilyDefinition, boolean schemaV3)
-          throws IOException {
-    List<Object> outputs = new ArrayList<>();
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               boolean schemaV3)
+      throws IOException {
 
-    if (startKey != null) {
-      iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
+    if (fileName == null) {
+      // Print to stdout
+      return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
-
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
-        Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
-        } else {
-          System.out.println(result.toString());
-        }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
-      }
+    // Write to file output
+    try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+      return displayTable(iterator, dbColumnFamilyDef, out, schemaV3);
     }
-    return outputs;
   }
 
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               PrintWriter out,
+                               boolean schemaV3)
+      throws IOException {
 
-  public RDBParser getParent() {
-    return parent;
-  }
+    if (startKey != null) {
+      iterator.get().seek(getValueObject(dbColumnFamilyDef));
+    }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDef.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
+        Gson gson = new GsonBuilder().setPrettyPrinting().create();
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String keyStr = key.toString();
+          if (index > keyStr.length()) {
+            err().println("Error: Invalid SchemaV3 table key length. "
+                + "Is this a V2 table? Try again with --dn-schema=V2");
+            return false;
+          }
+          String cid = keyStr.substring(0, index);
+          String blockId = keyStr.substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) +
+              keySeparatorSchemaV3 +
+              blockId));
+        } else {
+          sb.append(gson.toJson(key));
+        }
+        sb.append(": ");

Review Comment:
   I believe Schema V1/V2 keys are already printed as is on Line 234. The `: ` is a manually constructed JSON separator between a key-value pair, not inserted into the DB key.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1145958049


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,209 +154,168 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
         } else {
-          System.out.println(result.toString());
+          sb.append(gson.toJson(key));
         }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
+        sb.append(": ");
       }
-    }
-    return outputs;
-  }
-
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
-
-  public RDBParser getParent() {
-    return parent;
-  }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
-
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
-
-  public List<Object> getScannedObjects() {
-    return scannedObjects;
-  }
+      Gson gson = new GsonBuilder().setPrettyPrinting().create();
+      Object o = dbColumnFamilyDefinition.getValueCodec()
+          .fromPersistedFormat(iterator.get().value());
+      sb.append(gson.toJson(o));
 
-  public static void setFileName(String name) {
-    DBScanner.fileName = name;
-  }
-
-  public static void setContainerId(long id) {
-    DBScanner.containerId = id;
-  }
+      iterator.get().next();
+      ++count;
+      if (withinLimit(count) && iterator.get().isValid()) {
+        // If this is not the last entry, append comma
+        sb.append(", ");
+      }
 
-  public static void setDnDBSchemaVersion(String version) {
-    DBScanner.dnDBSchemaVersion = version;
-  }
+      out.print(sb);
+    }
 
-  public static void setWithKey(boolean withKey) {
-    DBScanner.withKey = withKey;
+    if (withKey) {
+      // End JSON object
+      out.println(" }");
+    } else {
+      // End JSON array
+      out.println(" ]");
+    }
   }
 
-  public static void setShowCount(boolean showCount) {
-    DBScanner.showCount = showCount;
+  private boolean withinLimit(long i) {
+    return limit == -1L || i < limit;
   }
 
-  private static ColumnFamilyHandle getColumnFamilyHandle(
+  private ColumnFamilyHandle getColumnFamilyHandle(
             byte[] name, List<ColumnFamilyHandle> columnFamilyHandles) {
     return columnFamilyHandles
             .stream()
             .filter(
               handle -> {
                 try {
                   return Arrays.equals(handle.getName(), name);
-                    } catch (Exception ex) {
+                } catch (Exception ex) {
                   throw new RuntimeException(ex);
-                    }
+                }
               })
             .findAny()
             .orElse(null);
   }
 
-  private void constructColumnFamilyMap(DBDefinition dbDefinition) {
-    if (dbDefinition == null) {
-      System.out.println("Incorrect Db Path");
-      return;
-    }
-    this.columnFamilyMap = new HashMap<>();
-    DBColumnFamilyDefinition[] columnFamilyDefinitions = dbDefinition
-            .getColumnFamilies();
-    for (DBColumnFamilyDefinition definition:columnFamilyDefinitions) {
-      LOG.info("Added definition for table: {}", definition.getTableName());
-      this.columnFamilyMap.put(definition.getTableName(), definition);
-    }
-  }
-
-  @Override
-  public Void call() throws Exception {
-    List<ColumnFamilyDescriptor> cfs =
-        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
-
-    final List<ColumnFamilyHandle> columnFamilyHandleList =
-        new ArrayList<>();
-    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),
-            cfs, columnFamilyHandleList);
-    this.printAppropriateTable(columnFamilyHandleList,
-           rocksDB, parent.getDbPath());
-    return null;
-  }
-
-  private void printAppropriateTable(
-          List<ColumnFamilyHandle> columnFamilyHandleList,
-          ManagedRocksDB rocksDB, String dbPath)
+  /**
+   * Main table printing logic.
+   * User-provided args are not in the arg list.
+   */
+  private void printTable(List<ColumnFamilyHandle> columnFamilyHandleList,
+      ManagedRocksDB rocksDB, String dbPath)
       throws IOException, RocksDBException {
+
     if (limit < 1 && limit != -1) {
       throw new IllegalArgumentException(
               "List length should be a positive number. Only allowed negative" +
                   " number is -1 which is to dump entire table");
     }
     dbPath = removeTrailingSlashIfNeeded(dbPath);
     DBDefinitionFactory.setDnDBSchemaVersion(dnDBSchemaVersion);
-    this.constructColumnFamilyMap(DBDefinitionFactory.
-            getDefinition(Paths.get(dbPath), new OzoneConfiguration()));
-    if (this.columnFamilyMap != null) {
-      if (!this.columnFamilyMap.containsKey(tableName)) {
-        System.out.print("Table with name:" + tableName + " does not exist");
-      } else {
-        DBColumnFamilyDefinition columnFamilyDefinition =
-                this.columnFamilyMap.get(tableName);
-        ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
-                columnFamilyDefinition.getTableName()
-                        .getBytes(StandardCharsets.UTF_8),
-                columnFamilyHandleList);
-        if (columnFamilyHandle == null) {
-          throw new IllegalArgumentException("columnFamilyHandle is null");
-        }
-        if (showCount) {
-          long keyCount = rocksDB.get().getLongProperty(columnFamilyHandle,
-              RocksDatabase.ESTIMATE_NUM_KEYS);
-          System.out.println(keyCount);
-          return;
-        }
-        ManagedRocksIterator iterator;
-        if (containerId > 0 && dnDBSchemaVersion != null &&
-            dnDBSchemaVersion.equals("V3")) {
-          ManagedReadOptions readOptions = new ManagedReadOptions();
-          readOptions.setIterateUpperBound(new ManagedSlice(
-              FixedLengthStringUtils.string2Bytes(
-                  DatanodeSchemaThreeDBDefinition.getContainerKeyPrefix(
-                  containerId + 1))));
-          iterator = new ManagedRocksIterator(
-              rocksDB.get().newIterator(columnFamilyHandle, readOptions));
-          iterator.get().seek(FixedLengthStringUtils.string2Bytes(
+    DBDefinition dbDefinition = DBDefinitionFactory.getDefinition(
+        Paths.get(dbPath), new OzoneConfiguration());
+    if (dbDefinition == null) {
+      err().println("Error: Incorrect DB Path");
+      return;
+    }
+
+    Map<String, DBColumnFamilyDefinition> columnFamilyMap = new HashMap<>();
+    for (DBColumnFamilyDefinition cfDef : dbDefinition.getColumnFamilies()) {
+      LOG.info("Found table: {}", cfDef.getTableName());
+      columnFamilyMap.put(cfDef.getTableName(), cfDef);
+    }
+    if (!columnFamilyMap.containsKey(tableName)) {
+      err().print("Error: Table with name '" + tableName + "' not found");
+      return;
+    }
+
+    DBColumnFamilyDefinition columnFamilyDefinition =
+        columnFamilyMap.get(tableName);
+    ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
+        columnFamilyDefinition.getTableName().getBytes(UTF_8),
+        columnFamilyHandleList);
+    if (columnFamilyHandle == null) {
+      throw new IllegalStateException("columnFamilyHandle is null");
+    }
+
+    if (showCount) {
+      long keyCount = rocksDB.get().getLongProperty(columnFamilyHandle,
+          RocksDatabase.ESTIMATE_NUM_KEYS);
+      out().println(keyCount);
+      return;
+    }
+    ManagedRocksIterator iterator;

Review Comment:
   Done



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,209 +154,168 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
         } else {
-          System.out.println(result.toString());
+          sb.append(gson.toJson(key));
         }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
+        sb.append(": ");
       }
-    }
-    return outputs;
-  }
-
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
-
-  public RDBParser getParent() {
-    return parent;
-  }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
-
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
-
-  public List<Object> getScannedObjects() {
-    return scannedObjects;
-  }
+      Gson gson = new GsonBuilder().setPrettyPrinting().create();
+      Object o = dbColumnFamilyDefinition.getValueCodec()
+          .fromPersistedFormat(iterator.get().value());
+      sb.append(gson.toJson(o));
 
-  public static void setFileName(String name) {
-    DBScanner.fileName = name;
-  }
-
-  public static void setContainerId(long id) {
-    DBScanner.containerId = id;
-  }
+      iterator.get().next();
+      ++count;
+      if (withinLimit(count) && iterator.get().isValid()) {
+        // If this is not the last entry, append comma
+        sb.append(", ");
+      }
 
-  public static void setDnDBSchemaVersion(String version) {
-    DBScanner.dnDBSchemaVersion = version;
-  }
+      out.print(sb);
+    }
 
-  public static void setWithKey(boolean withKey) {
-    DBScanner.withKey = withKey;
+    if (withKey) {
+      // End JSON object
+      out.println(" }");
+    } else {
+      // End JSON array
+      out.println(" ]");
+    }
   }
 
-  public static void setShowCount(boolean showCount) {
-    DBScanner.showCount = showCount;
+  private boolean withinLimit(long i) {
+    return limit == -1L || i < limit;
   }
 
-  private static ColumnFamilyHandle getColumnFamilyHandle(
+  private ColumnFamilyHandle getColumnFamilyHandle(
             byte[] name, List<ColumnFamilyHandle> columnFamilyHandles) {
     return columnFamilyHandles
             .stream()
             .filter(
               handle -> {
                 try {
                   return Arrays.equals(handle.getName(), name);
-                    } catch (Exception ex) {
+                } catch (Exception ex) {
                   throw new RuntimeException(ex);
-                    }
+                }
               })
             .findAny()
             .orElse(null);
   }
 
-  private void constructColumnFamilyMap(DBDefinition dbDefinition) {
-    if (dbDefinition == null) {
-      System.out.println("Incorrect Db Path");
-      return;
-    }
-    this.columnFamilyMap = new HashMap<>();
-    DBColumnFamilyDefinition[] columnFamilyDefinitions = dbDefinition
-            .getColumnFamilies();
-    for (DBColumnFamilyDefinition definition:columnFamilyDefinitions) {
-      LOG.info("Added definition for table: {}", definition.getTableName());
-      this.columnFamilyMap.put(definition.getTableName(), definition);
-    }
-  }
-
-  @Override
-  public Void call() throws Exception {
-    List<ColumnFamilyDescriptor> cfs =
-        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
-
-    final List<ColumnFamilyHandle> columnFamilyHandleList =
-        new ArrayList<>();
-    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),
-            cfs, columnFamilyHandleList);
-    this.printAppropriateTable(columnFamilyHandleList,
-           rocksDB, parent.getDbPath());
-    return null;
-  }
-
-  private void printAppropriateTable(
-          List<ColumnFamilyHandle> columnFamilyHandleList,
-          ManagedRocksDB rocksDB, String dbPath)
+  /**
+   * Main table printing logic.
+   * User-provided args are not in the arg list.

Review Comment:
   Meaning those are instance variables parsed by picocli. e.g. `limit`



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146899627


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   I get your idea. But Schema V3 table key is not fully composed of ASCII characters (intended for seekability I presume).
   
   e.g. for `block_data`:
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L42-L43
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L150-L154
   
   The logic to print it is left untouched. Arguably all valuable key info is extracted here already.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #4420:
URL: https://github.com/apache/ozone/pull/4420#issuecomment-1483922733

   Thanks for the improvement @smengcl 


-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] kerneltime commented on pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #4420:
URL: https://github.com/apache/ozone/pull/4420#issuecomment-1476538909

   @swamirishi @tanvipenumudy @DaveTeng0  can you please take a look


-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1148099011


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,210 +173,192 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
-      DBColumnFamilyDefinition dbColumnFamilyDefinition, boolean schemaV3)
-          throws IOException {
-    List<Object> outputs = new ArrayList<>();
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               boolean schemaV3)
+      throws IOException {
 
-    if (startKey != null) {
-      iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
+    if (fileName == null) {
+      // Print to stdout
+      return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
-
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
-        Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
-        } else {
-          System.out.println(result.toString());
-        }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
-      }
+    // Write to file output
+    try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+      return displayTable(iterator, dbColumnFamilyDef, out, schemaV3);
     }
-    return outputs;
   }
 
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               PrintWriter out,
+                               boolean schemaV3)
+      throws IOException {
 
-  public RDBParser getParent() {
-    return parent;
-  }
+    if (startKey != null) {
+      iterator.get().seek(getValueObject(dbColumnFamilyDef));
+    }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDef.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
+        Gson gson = new GsonBuilder().setPrettyPrinting().create();
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String keyStr = key.toString();
+          if (index > keyStr.length()) {
+            err().println("Error: Invalid SchemaV3 table key length. "
+                + "Is this a V2 table? Try again with --dn-schema=V2");
+            return false;
+          }
+          String cid = keyStr.substring(0, index);
+          String blockId = keyStr.substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) +
+              keySeparatorSchemaV3 +
+              blockId));
+        } else {
+          sb.append(gson.toJson(key));
+        }
+        sb.append(": ");

Review Comment:
   I believe Schema V1/V2 keys are already printed as-is. The `: ` is a manually constructed JSON separator between a key-value pair.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1148433790


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,210 +173,192 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
-      DBColumnFamilyDefinition dbColumnFamilyDefinition, boolean schemaV3)
-          throws IOException {
-    List<Object> outputs = new ArrayList<>();
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               boolean schemaV3)
+      throws IOException {
 
-    if (startKey != null) {
-      iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
+    if (fileName == null) {
+      // Print to stdout
+      return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
-
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
-        Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
-        } else {
-          System.out.println(result.toString());
-        }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
-      }
+    // Write to file output
+    try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+      return displayTable(iterator, dbColumnFamilyDef, out, schemaV3);
     }
-    return outputs;
   }
 
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               PrintWriter out,
+                               boolean schemaV3)
+      throws IOException {
 
-  public RDBParser getParent() {
-    return parent;
-  }
+    if (startKey != null) {
+      iterator.get().seek(getValueObject(dbColumnFamilyDef));
+    }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDef.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
+        Gson gson = new GsonBuilder().setPrettyPrinting().create();
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String keyStr = key.toString();
+          if (index > keyStr.length()) {
+            err().println("Error: Invalid SchemaV3 table key length. "
+                + "Is this a V2 table? Try again with --dn-schema=V2");
+            return false;
+          }
+          String cid = keyStr.substring(0, index);
+          String blockId = keyStr.substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) +
+              keySeparatorSchemaV3 +
+              blockId));
+        } else {
+          sb.append(gson.toJson(key));
+        }
+        sb.append(": ");

Review Comment:
   Got it, I misread this.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1148099011


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,210 +173,192 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
-      DBColumnFamilyDefinition dbColumnFamilyDefinition, boolean schemaV3)
-          throws IOException {
-    List<Object> outputs = new ArrayList<>();
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               boolean schemaV3)
+      throws IOException {
 
-    if (startKey != null) {
-      iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
+    if (fileName == null) {
+      // Print to stdout
+      return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
-
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
-        Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
-        } else {
-          System.out.println(result.toString());
-        }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
-      }
+    // Write to file output
+    try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+      return displayTable(iterator, dbColumnFamilyDef, out, schemaV3);
     }
-    return outputs;
   }
 
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               PrintWriter out,
+                               boolean schemaV3)
+      throws IOException {
 
-  public RDBParser getParent() {
-    return parent;
-  }
+    if (startKey != null) {
+      iterator.get().seek(getValueObject(dbColumnFamilyDef));
+    }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDef.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
+        Gson gson = new GsonBuilder().setPrettyPrinting().create();
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String keyStr = key.toString();
+          if (index > keyStr.length()) {
+            err().println("Error: Invalid SchemaV3 table key length. "
+                + "Is this a V2 table? Try again with --dn-schema=V2");
+            return false;
+          }
+          String cid = keyStr.substring(0, index);
+          String blockId = keyStr.substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) +
+              keySeparatorSchemaV3 +
+              blockId));
+        } else {
+          sb.append(gson.toJson(key));
+        }
+        sb.append(": ");

Review Comment:
   I believe Schema V1/V2 keys are already printed as-is on Line 234. The `: ` is a manually constructed JSON separator between a key-value pair.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146785466


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   I think we should print the key as it is in the DB in all cases since this is a debugging tool.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146994772


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   Good idea. Done.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146899627


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   I get your idea. But Schema V3 table key is not fully composed of ASCII characters (intended for seekability and space-saving I presume).
   
   e.g. for `block_data`:
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L42-L43
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L150-L154
   
   The logic to print it is left untouched. Arguably all valuable key info is extracted here already.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146949712


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   It is just an encoding of the container ID + the separator + block ID, we can render it like that. For example,
   "\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001|111677748019200001" which is printed on master would become "1|111677748019200001".



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146956308


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   You mean just changing the printed separator from `: ` to `|` then?
   
   ~~The actual `separator` is an empty string.~~ It is `|`
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L91



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] errose28 commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1147943390


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,210 +173,192 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
-      DBColumnFamilyDefinition dbColumnFamilyDefinition, boolean schemaV3)
-          throws IOException {
-    List<Object> outputs = new ArrayList<>();
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               boolean schemaV3)
+      throws IOException {
 
-    if (startKey != null) {
-      iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
+    if (fileName == null) {
+      // Print to stdout
+      return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
-
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
-        Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
-        } else {
-          System.out.println(result.toString());
-        }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
-      }
+    // Write to file output
+    try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+      return displayTable(iterator, dbColumnFamilyDef, out, schemaV3);
     }
-    return outputs;
   }
 
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               PrintWriter out,
+                               boolean schemaV3)
+      throws IOException {
 
-  public RDBParser getParent() {
-    return parent;
-  }
+    if (startKey != null) {
+      iterator.get().seek(getValueObject(dbColumnFamilyDef));
+    }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDef.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
+        Gson gson = new GsonBuilder().setPrettyPrinting().create();
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String keyStr = key.toString();
+          if (index > keyStr.length()) {
+            err().println("Error: Invalid SchemaV3 table key length. "
+                + "Is this a V2 table? Try again with --dn-schema=V2");
+            return false;
+          }
+          String cid = keyStr.substring(0, index);
+          String blockId = keyStr.substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) +
+              keySeparatorSchemaV3 +
+              blockId));
+        } else {
+          sb.append(gson.toJson(key));
+        }
+        sb.append(": ");

Review Comment:
   Can we print the key as is for schema v1/v2 as well? This should just be the block ID.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1145153749


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestLDBCli.java:
##########
@@ -36,295 +38,281 @@
 import org.apache.hadoop.ozone.debug.RDBParser;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
-import org.apache.ozone.test.GenericTestUtils;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.Assert;
-import org.junit.rules.TemporaryFolder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import picocli.CommandLine;
 
-import java.io.BufferedReader;
-import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.InputStreamReader;
-import java.io.PrintStream;
-import java.time.LocalDateTime;
-import java.util.List;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import java.util.stream.Stream;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-
 /**
- * This class tests the Debug LDB CLI that reads from rocks db file.
+ * This class tests `ozone debug ldb` CLI that reads from a RocksDB directory.
  */
 public class TestLDBCli {
+  private static final String KEY_TABLE = "keyTable";
+  private static final String BLOCK_DATA = "block_data";
+  private static final ObjectMapper MAPPER = new ObjectMapper();
   private OzoneConfiguration conf;
+  private DBStore dbStore;
+  @TempDir
+  private File tempDir;
+  private StringWriter stdout, stderr;
+  private CommandLine cmd;
+  private NavigableMap<String, Map<String, ?>> dbMap;
+
+  @BeforeEach
+  public void setup() throws IOException {
+    conf = new OzoneConfiguration();
+    stdout = new StringWriter();
+    stderr = new StringWriter();

Review Comment:
   Should we close these? Same for `PrintWriter` in line number 90 and 91.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -67,62 +66,81 @@
 @MetaInfServices(SubcommandWithParent.class)
 public class DBScanner implements Callable<Void>, SubcommandWithParent {
 
-  public static final Logger LOG =
-      LoggerFactory.getLogger(DBScanner.class);
+  public static final Logger LOG = LoggerFactory.getLogger(DBScanner.class);
+  private static final String SCHEMA_V3 = "V3";
+
+  @CommandLine.Spec
+  private CommandLine.Model.CommandSpec spec;
+
+  @CommandLine.ParentCommand
+  private RDBParser parent;
 
-  @CommandLine.Option(names = {"--column_family", "--column-family"},
+  @CommandLine.Option(names = {"--column_family", "--column-family", "--cf"},
       required = true,
       description = "Table name")
   private String tableName;
 
   @CommandLine.Option(names = {"--with-keys"},
-      description = "List Key -> Value instead of just Value.",
-      defaultValue = "false",
-      showDefaultValue = CommandLine.Help.Visibility.ALWAYS)
-  private static boolean withKey;
+      description = "Print a JSON object of key->value pairs (default)"
+          + " instead of a JSON array of only values.",
+      defaultValue = "true")
+  private boolean withKey;
 
-  @CommandLine.Option(names = {"--length", "-l"},
-          description = "Maximum number of items to list.")
-  private static int limit = -1;
+  @CommandLine.Option(names = {"--length", "--limit", "-l"},
+      description = "Maximum number of items to list.",
+      defaultValue = "-1")
+  private long limit;
 
   @CommandLine.Option(names = {"--out", "-o"},
       description = "File to dump table scan data")
-  private static String fileName;
+  private String fileName;
 
-  @CommandLine.Option(names = {"--startkey", "-sk"},
+  @CommandLine.Option(names = {"--startkey", "--sk", "-s"},
       description = "Key from which to iterate the DB")
-  private static String startKey;
+  private String startKey;
 
-  @CommandLine.Option(names = {"--dnSchema", "-d", "--dn-schema"},
-      description = "Datanode DB Schema Version : V1/V2/V3",
+  @CommandLine.Option(names = {"--dnSchema", "--dn-schema", "-d"},
+      description = "Datanode DB Schema Version: V1/V2/V3",
       defaultValue = "V2")
-  private static String dnDBSchemaVersion;
+  private String dnDBSchemaVersion;
 
-  @CommandLine.Option(names = {"--container-id", "-cid"},
-      description = "Container ID when datanode DB Schema is V3",
+  @CommandLine.Option(names = {"--container-id", "--cid"},
+      description = "Container ID. Applicable if datanode DB Schema is V3",
       defaultValue = "-1")
-  private static long containerId;
+  private long containerId;
 
-  @CommandLine.Option(names = { "--show-count",
-      "-count" }, description = "Get estimated key count for a"
-      + " given column family in the db",
+  @CommandLine.Option(names = { "--show-count", "--count" },
+      description = "Get estimated key count for the given DB column family",
       defaultValue = "false",
       showDefaultValue = CommandLine.Help.Visibility.ALWAYS)
-  private static boolean showCount;
+  private boolean showCount;
 
+  @Override
+  public Void call() throws Exception {
+    List<ColumnFamilyDescriptor> cfs =
+        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
 
-  @CommandLine.ParentCommand
-  private RDBParser parent;
+    final List<ColumnFamilyHandle> columnFamilyHandleList = new ArrayList<>();
+    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),

Review Comment:
   Move `ManagedRocksDB rocksDB` inside try-with-resources.



##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java:
##########
@@ -447,6 +447,7 @@ public static OmKeyInfo createOmKeyInfo(String volumeName, String bucketName,
         .setVolumeName(volumeName)
         .setBucketName(bucketName)
         .setKeyName(keyName)
+        .setFileName(keyName)

Review Comment:
   Is it typo or `fileName` and `keyName` are actually same?



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,209 +154,168 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
         } else {
-          System.out.println(result.toString());
+          sb.append(gson.toJson(key));
         }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
+        sb.append(": ");
       }
-    }
-    return outputs;
-  }
-
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
-
-  public RDBParser getParent() {
-    return parent;
-  }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
-
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
-
-  public List<Object> getScannedObjects() {
-    return scannedObjects;
-  }
+      Gson gson = new GsonBuilder().setPrettyPrinting().create();
+      Object o = dbColumnFamilyDefinition.getValueCodec()
+          .fromPersistedFormat(iterator.get().value());
+      sb.append(gson.toJson(o));
 
-  public static void setFileName(String name) {
-    DBScanner.fileName = name;
-  }
-
-  public static void setContainerId(long id) {
-    DBScanner.containerId = id;
-  }
+      iterator.get().next();
+      ++count;
+      if (withinLimit(count) && iterator.get().isValid()) {
+        // If this is not the last entry, append comma
+        sb.append(", ");
+      }
 
-  public static void setDnDBSchemaVersion(String version) {
-    DBScanner.dnDBSchemaVersion = version;
-  }
+      out.print(sb);
+    }
 
-  public static void setWithKey(boolean withKey) {
-    DBScanner.withKey = withKey;
+    if (withKey) {
+      // End JSON object
+      out.println(" }");
+    } else {
+      // End JSON array
+      out.println(" ]");
+    }
   }
 
-  public static void setShowCount(boolean showCount) {
-    DBScanner.showCount = showCount;
+  private boolean withinLimit(long i) {
+    return limit == -1L || i < limit;
   }
 
-  private static ColumnFamilyHandle getColumnFamilyHandle(
+  private ColumnFamilyHandle getColumnFamilyHandle(
             byte[] name, List<ColumnFamilyHandle> columnFamilyHandles) {
     return columnFamilyHandles
             .stream()
             .filter(
               handle -> {
                 try {
                   return Arrays.equals(handle.getName(), name);
-                    } catch (Exception ex) {
+                } catch (Exception ex) {
                   throw new RuntimeException(ex);
-                    }
+                }
               })
             .findAny()
             .orElse(null);
   }
 
-  private void constructColumnFamilyMap(DBDefinition dbDefinition) {
-    if (dbDefinition == null) {
-      System.out.println("Incorrect Db Path");
-      return;
-    }
-    this.columnFamilyMap = new HashMap<>();
-    DBColumnFamilyDefinition[] columnFamilyDefinitions = dbDefinition
-            .getColumnFamilies();
-    for (DBColumnFamilyDefinition definition:columnFamilyDefinitions) {
-      LOG.info("Added definition for table: {}", definition.getTableName());
-      this.columnFamilyMap.put(definition.getTableName(), definition);
-    }
-  }
-
-  @Override
-  public Void call() throws Exception {
-    List<ColumnFamilyDescriptor> cfs =
-        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
-
-    final List<ColumnFamilyHandle> columnFamilyHandleList =
-        new ArrayList<>();
-    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),
-            cfs, columnFamilyHandleList);
-    this.printAppropriateTable(columnFamilyHandleList,
-           rocksDB, parent.getDbPath());
-    return null;
-  }
-
-  private void printAppropriateTable(
-          List<ColumnFamilyHandle> columnFamilyHandleList,
-          ManagedRocksDB rocksDB, String dbPath)
+  /**
+   * Main table printing logic.
+   * User-provided args are not in the arg list.
+   */
+  private void printTable(List<ColumnFamilyHandle> columnFamilyHandleList,
+      ManagedRocksDB rocksDB, String dbPath)
       throws IOException, RocksDBException {
+
     if (limit < 1 && limit != -1) {
       throw new IllegalArgumentException(
               "List length should be a positive number. Only allowed negative" +
                   " number is -1 which is to dump entire table");
     }
     dbPath = removeTrailingSlashIfNeeded(dbPath);
     DBDefinitionFactory.setDnDBSchemaVersion(dnDBSchemaVersion);
-    this.constructColumnFamilyMap(DBDefinitionFactory.
-            getDefinition(Paths.get(dbPath), new OzoneConfiguration()));
-    if (this.columnFamilyMap != null) {
-      if (!this.columnFamilyMap.containsKey(tableName)) {
-        System.out.print("Table with name:" + tableName + " does not exist");
-      } else {
-        DBColumnFamilyDefinition columnFamilyDefinition =
-                this.columnFamilyMap.get(tableName);
-        ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
-                columnFamilyDefinition.getTableName()
-                        .getBytes(StandardCharsets.UTF_8),
-                columnFamilyHandleList);
-        if (columnFamilyHandle == null) {
-          throw new IllegalArgumentException("columnFamilyHandle is null");
-        }
-        if (showCount) {
-          long keyCount = rocksDB.get().getLongProperty(columnFamilyHandle,
-              RocksDatabase.ESTIMATE_NUM_KEYS);
-          System.out.println(keyCount);
-          return;
-        }
-        ManagedRocksIterator iterator;
-        if (containerId > 0 && dnDBSchemaVersion != null &&
-            dnDBSchemaVersion.equals("V3")) {
-          ManagedReadOptions readOptions = new ManagedReadOptions();
-          readOptions.setIterateUpperBound(new ManagedSlice(
-              FixedLengthStringUtils.string2Bytes(
-                  DatanodeSchemaThreeDBDefinition.getContainerKeyPrefix(
-                  containerId + 1))));
-          iterator = new ManagedRocksIterator(
-              rocksDB.get().newIterator(columnFamilyHandle, readOptions));
-          iterator.get().seek(FixedLengthStringUtils.string2Bytes(
+    DBDefinition dbDefinition = DBDefinitionFactory.getDefinition(
+        Paths.get(dbPath), new OzoneConfiguration());
+    if (dbDefinition == null) {
+      err().println("Error: Incorrect DB Path");
+      return;
+    }
+
+    Map<String, DBColumnFamilyDefinition> columnFamilyMap = new HashMap<>();
+    for (DBColumnFamilyDefinition cfDef : dbDefinition.getColumnFamilies()) {
+      LOG.info("Found table: {}", cfDef.getTableName());
+      columnFamilyMap.put(cfDef.getTableName(), cfDef);
+    }
+    if (!columnFamilyMap.containsKey(tableName)) {
+      err().print("Error: Table with name '" + tableName + "' not found");
+      return;
+    }
+
+    DBColumnFamilyDefinition columnFamilyDefinition =
+        columnFamilyMap.get(tableName);
+    ColumnFamilyHandle columnFamilyHandle = getColumnFamilyHandle(
+        columnFamilyDefinition.getTableName().getBytes(UTF_8),
+        columnFamilyHandleList);
+    if (columnFamilyHandle == null) {
+      throw new IllegalStateException("columnFamilyHandle is null");
+    }
+
+    if (showCount) {
+      long keyCount = rocksDB.get().getLongProperty(columnFamilyHandle,
+          RocksDatabase.ESTIMATE_NUM_KEYS);
+      out().println(keyCount);
+      return;
+    }
+    ManagedRocksIterator iterator;

Review Comment:
   Not sure, how big it is a concern for CLI, but as general practice, `ManagedRocksIterator` should be closed after the use.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,209 +154,168 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
         } else {
-          System.out.println(result.toString());
+          sb.append(gson.toJson(key));
         }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
+        sb.append(": ");
       }
-    }
-    return outputs;
-  }
-
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
-
-  public RDBParser getParent() {
-    return parent;
-  }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
-
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
-
-  public List<Object> getScannedObjects() {
-    return scannedObjects;
-  }
+      Gson gson = new GsonBuilder().setPrettyPrinting().create();
+      Object o = dbColumnFamilyDefinition.getValueCodec()
+          .fromPersistedFormat(iterator.get().value());
+      sb.append(gson.toJson(o));
 
-  public static void setFileName(String name) {
-    DBScanner.fileName = name;
-  }
-
-  public static void setContainerId(long id) {
-    DBScanner.containerId = id;
-  }
+      iterator.get().next();
+      ++count;
+      if (withinLimit(count) && iterator.get().isValid()) {
+        // If this is not the last entry, append comma
+        sb.append(", ");
+      }
 
-  public static void setDnDBSchemaVersion(String version) {
-    DBScanner.dnDBSchemaVersion = version;
-  }
+      out.print(sb);
+    }
 
-  public static void setWithKey(boolean withKey) {
-    DBScanner.withKey = withKey;
+    if (withKey) {
+      // End JSON object
+      out.println(" }");
+    } else {
+      // End JSON array
+      out.println(" ]");
+    }
   }
 
-  public static void setShowCount(boolean showCount) {
-    DBScanner.showCount = showCount;
+  private boolean withinLimit(long i) {
+    return limit == -1L || i < limit;
   }
 
-  private static ColumnFamilyHandle getColumnFamilyHandle(
+  private ColumnFamilyHandle getColumnFamilyHandle(
             byte[] name, List<ColumnFamilyHandle> columnFamilyHandles) {
     return columnFamilyHandles
             .stream()
             .filter(
               handle -> {
                 try {
                   return Arrays.equals(handle.getName(), name);
-                    } catch (Exception ex) {
+                } catch (Exception ex) {
                   throw new RuntimeException(ex);
-                    }
+                }
               })
             .findAny()
             .orElse(null);
   }
 
-  private void constructColumnFamilyMap(DBDefinition dbDefinition) {
-    if (dbDefinition == null) {
-      System.out.println("Incorrect Db Path");
-      return;
-    }
-    this.columnFamilyMap = new HashMap<>();
-    DBColumnFamilyDefinition[] columnFamilyDefinitions = dbDefinition
-            .getColumnFamilies();
-    for (DBColumnFamilyDefinition definition:columnFamilyDefinitions) {
-      LOG.info("Added definition for table: {}", definition.getTableName());
-      this.columnFamilyMap.put(definition.getTableName(), definition);
-    }
-  }
-
-  @Override
-  public Void call() throws Exception {
-    List<ColumnFamilyDescriptor> cfs =
-        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
-
-    final List<ColumnFamilyHandle> columnFamilyHandleList =
-        new ArrayList<>();
-    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),
-            cfs, columnFamilyHandleList);
-    this.printAppropriateTable(columnFamilyHandleList,
-           rocksDB, parent.getDbPath());
-    return null;
-  }
-
-  private void printAppropriateTable(
-          List<ColumnFamilyHandle> columnFamilyHandleList,
-          ManagedRocksDB rocksDB, String dbPath)
+  /**
+   * Main table printing logic.
+   * User-provided args are not in the arg list.

Review Comment:
   What does `User-provided args are not in the arg list.` line mean here?



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,209 +154,168 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
         } else {
-          System.out.println(result.toString());
+          sb.append(gson.toJson(key));
         }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
+        sb.append(": ");
       }
-    }
-    return outputs;
-  }
-
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
-
-  public RDBParser getParent() {
-    return parent;
-  }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
-
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
-
-  public List<Object> getScannedObjects() {
-    return scannedObjects;
-  }
+      Gson gson = new GsonBuilder().setPrettyPrinting().create();
+      Object o = dbColumnFamilyDefinition.getValueCodec()
+          .fromPersistedFormat(iterator.get().value());
+      sb.append(gson.toJson(o));
 
-  public static void setFileName(String name) {
-    DBScanner.fileName = name;
-  }
-
-  public static void setContainerId(long id) {
-    DBScanner.containerId = id;
-  }
+      iterator.get().next();
+      ++count;
+      if (withinLimit(count) && iterator.get().isValid()) {
+        // If this is not the last entry, append comma
+        sb.append(", ");
+      }
 
-  public static void setDnDBSchemaVersion(String version) {
-    DBScanner.dnDBSchemaVersion = version;
-  }
+      out.print(sb);
+    }
 
-  public static void setWithKey(boolean withKey) {
-    DBScanner.withKey = withKey;
+    if (withKey) {
+      // End JSON object
+      out.println(" }");
+    } else {
+      // End JSON array
+      out.println(" ]");
+    }
   }
 
-  public static void setShowCount(boolean showCount) {
-    DBScanner.showCount = showCount;
+  private boolean withinLimit(long i) {
+    return limit == -1L || i < limit;
   }
 
-  private static ColumnFamilyHandle getColumnFamilyHandle(
+  private ColumnFamilyHandle getColumnFamilyHandle(
             byte[] name, List<ColumnFamilyHandle> columnFamilyHandles) {
     return columnFamilyHandles
             .stream()
             .filter(
               handle -> {
                 try {
                   return Arrays.equals(handle.getName(), name);
-                    } catch (Exception ex) {
+                } catch (Exception ex) {
                   throw new RuntimeException(ex);
-                    }
+                }
               })
             .findAny()
             .orElse(null);
   }
 
-  private void constructColumnFamilyMap(DBDefinition dbDefinition) {
-    if (dbDefinition == null) {
-      System.out.println("Incorrect Db Path");
-      return;
-    }
-    this.columnFamilyMap = new HashMap<>();
-    DBColumnFamilyDefinition[] columnFamilyDefinitions = dbDefinition
-            .getColumnFamilies();
-    for (DBColumnFamilyDefinition definition:columnFamilyDefinitions) {
-      LOG.info("Added definition for table: {}", definition.getTableName());
-      this.columnFamilyMap.put(definition.getTableName(), definition);
-    }
-  }
-
-  @Override
-  public Void call() throws Exception {
-    List<ColumnFamilyDescriptor> cfs =
-        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
-
-    final List<ColumnFamilyHandle> columnFamilyHandleList =
-        new ArrayList<>();
-    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),
-            cfs, columnFamilyHandleList);
-    this.printAppropriateTable(columnFamilyHandleList,
-           rocksDB, parent.getDbPath());
-    return null;
-  }
-
-  private void printAppropriateTable(
-          List<ColumnFamilyHandle> columnFamilyHandleList,
-          ManagedRocksDB rocksDB, String dbPath)
+  /**
+   * Main table printing logic.
+   * User-provided args are not in the arg list.
+   */
+  private void printTable(List<ColumnFamilyHandle> columnFamilyHandleList,

Review Comment:
   I don't know what's correct format in Ozone for this type of scenarios but for me it should be either 
   ```
     private void printTable(List<ColumnFamilyHandle> columnFamilyHandleList,
                             ManagedRocksDB rocksDB, String dbPath)
         throws IOException, RocksDBException {
       ...
     }
   ```
   
   or 
   ```
     private void printTable(
         List<ColumnFamilyHandle> columnFamilyHandleList,
         ManagedRocksDB rocksDB, String dbPath)
         throws IOException, RocksDBException 
     {
       ...
     }
   
     ```



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1145957603


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestLDBCli.java:
##########
@@ -36,295 +38,281 @@
 import org.apache.hadoop.ozone.debug.RDBParser;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
-import org.apache.ozone.test.GenericTestUtils;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.Assert;
-import org.junit.rules.TemporaryFolder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import picocli.CommandLine;
 
-import java.io.BufferedReader;
-import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.InputStreamReader;
-import java.io.PrintStream;
-import java.time.LocalDateTime;
-import java.util.List;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import java.util.stream.Stream;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-
 /**
- * This class tests the Debug LDB CLI that reads from rocks db file.
+ * This class tests `ozone debug ldb` CLI that reads from a RocksDB directory.
  */
 public class TestLDBCli {
+  private static final String KEY_TABLE = "keyTable";
+  private static final String BLOCK_DATA = "block_data";
+  private static final ObjectMapper MAPPER = new ObjectMapper();
   private OzoneConfiguration conf;
+  private DBStore dbStore;
+  @TempDir
+  private File tempDir;
+  private StringWriter stdout, stderr;
+  private CommandLine cmd;
+  private NavigableMap<String, Map<String, ?>> dbMap;
+
+  @BeforeEach
+  public void setup() throws IOException {
+    conf = new OzoneConfiguration();
+    stdout = new StringWriter();
+    stderr = new StringWriter();

Review Comment:
   Done



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -67,62 +66,81 @@
 @MetaInfServices(SubcommandWithParent.class)
 public class DBScanner implements Callable<Void>, SubcommandWithParent {
 
-  public static final Logger LOG =
-      LoggerFactory.getLogger(DBScanner.class);
+  public static final Logger LOG = LoggerFactory.getLogger(DBScanner.class);
+  private static final String SCHEMA_V3 = "V3";
+
+  @CommandLine.Spec
+  private CommandLine.Model.CommandSpec spec;
+
+  @CommandLine.ParentCommand
+  private RDBParser parent;
 
-  @CommandLine.Option(names = {"--column_family", "--column-family"},
+  @CommandLine.Option(names = {"--column_family", "--column-family", "--cf"},
       required = true,
       description = "Table name")
   private String tableName;
 
   @CommandLine.Option(names = {"--with-keys"},
-      description = "List Key -> Value instead of just Value.",
-      defaultValue = "false",
-      showDefaultValue = CommandLine.Help.Visibility.ALWAYS)
-  private static boolean withKey;
+      description = "Print a JSON object of key->value pairs (default)"
+          + " instead of a JSON array of only values.",
+      defaultValue = "true")
+  private boolean withKey;
 
-  @CommandLine.Option(names = {"--length", "-l"},
-          description = "Maximum number of items to list.")
-  private static int limit = -1;
+  @CommandLine.Option(names = {"--length", "--limit", "-l"},
+      description = "Maximum number of items to list.",
+      defaultValue = "-1")
+  private long limit;
 
   @CommandLine.Option(names = {"--out", "-o"},
       description = "File to dump table scan data")
-  private static String fileName;
+  private String fileName;
 
-  @CommandLine.Option(names = {"--startkey", "-sk"},
+  @CommandLine.Option(names = {"--startkey", "--sk", "-s"},
       description = "Key from which to iterate the DB")
-  private static String startKey;
+  private String startKey;
 
-  @CommandLine.Option(names = {"--dnSchema", "-d", "--dn-schema"},
-      description = "Datanode DB Schema Version : V1/V2/V3",
+  @CommandLine.Option(names = {"--dnSchema", "--dn-schema", "-d"},
+      description = "Datanode DB Schema Version: V1/V2/V3",
       defaultValue = "V2")
-  private static String dnDBSchemaVersion;
+  private String dnDBSchemaVersion;
 
-  @CommandLine.Option(names = {"--container-id", "-cid"},
-      description = "Container ID when datanode DB Schema is V3",
+  @CommandLine.Option(names = {"--container-id", "--cid"},
+      description = "Container ID. Applicable if datanode DB Schema is V3",
       defaultValue = "-1")
-  private static long containerId;
+  private long containerId;
 
-  @CommandLine.Option(names = { "--show-count",
-      "-count" }, description = "Get estimated key count for a"
-      + " given column family in the db",
+  @CommandLine.Option(names = { "--show-count", "--count" },
+      description = "Get estimated key count for the given DB column family",
       defaultValue = "false",
       showDefaultValue = CommandLine.Help.Visibility.ALWAYS)
-  private static boolean showCount;
+  private boolean showCount;
 
+  @Override
+  public Void call() throws Exception {
+    List<ColumnFamilyDescriptor> cfs =
+        RocksDBUtils.getColumnFamilyDescriptors(parent.getDbPath());
 
-  @CommandLine.ParentCommand
-  private RDBParser parent;
+    final List<ColumnFamilyHandle> columnFamilyHandleList = new ArrayList<>();
+    ManagedRocksDB rocksDB = ManagedRocksDB.openReadOnly(parent.getDbPath(),

Review Comment:
   Done



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146899627


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   Yes. But Schema V3 block_data table key is not fully composed of ASCII characters. The logic to print it is untouched.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   Yes. But Schema V3 block_data table key is not fully composed of ASCII characters. The logic to print it is left untouched.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1148099011


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,210 +173,192 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
-      DBColumnFamilyDefinition dbColumnFamilyDefinition, boolean schemaV3)
-          throws IOException {
-    List<Object> outputs = new ArrayList<>();
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               boolean schemaV3)
+      throws IOException {
 
-    if (startKey != null) {
-      iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
+    if (fileName == null) {
+      // Print to stdout
+      return displayTable(iterator, dbColumnFamilyDef, out(), schemaV3);
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
-
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
-        Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
-        } else {
-          System.out.println(result.toString());
-        }
-        limit--;
-        iterator.get().next();
-        if (limit == 0) {
-          break;
-        }
-      }
-    } finally {
-      if (printWriter != null) {
-        printWriter.close();
-      }
-      if (fileWriter != null) {
-        fileWriter.close();
-      }
+    // Write to file output
+    try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+      return displayTable(iterator, dbColumnFamilyDef, out, schemaV3);
     }
-    return outputs;
   }
 
-  public void setTableName(String tableName) {
-    this.tableName = tableName;
-  }
+  private boolean displayTable(ManagedRocksIterator iterator,
+                               DBColumnFamilyDefinition dbColumnFamilyDef,
+                               PrintWriter out,
+                               boolean schemaV3)
+      throws IOException {
 
-  public RDBParser getParent() {
-    return parent;
-  }
+    if (startKey != null) {
+      iterator.get().seek(getValueObject(dbColumnFamilyDef));
+    }
 
-  public void setParent(RDBParser parent) {
-    this.parent = parent;
-  }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-  public static void setLimit(int limit) {
-    DBScanner.limit = limit;
-  }
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDef.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
+        Gson gson = new GsonBuilder().setPrettyPrinting().create();
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String keyStr = key.toString();
+          if (index > keyStr.length()) {
+            err().println("Error: Invalid SchemaV3 table key length. "
+                + "Is this a V2 table? Try again with --dn-schema=V2");
+            return false;
+          }
+          String cid = keyStr.substring(0, index);
+          String blockId = keyStr.substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) +
+              keySeparatorSchemaV3 +
+              blockId));
+        } else {
+          sb.append(gson.toJson(key));
+        }
+        sb.append(": ");

Review Comment:
   I believe Schema V1/V2 keys are already printed as is on Line 234. This `: ` is a key-value separator manually constructed for a JSON k-v pair, not inserted into the JSON/DB key.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4420:
URL: https://github.com/apache/ozone/pull/4420#issuecomment-1487398839

   Thanks @hemantk-12 and @errose28 for reviewing this


-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146451862


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java:
##########
@@ -447,6 +447,7 @@ public static OmKeyInfo createOmKeyInfo(String volumeName, String bucketName,
         .setVolumeName(volumeName)
         .setBucketName(bucketName)
         .setKeyName(keyName)
+        .setFileName(keyName)

Review Comment:
   Got it.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1145432350


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java:
##########
@@ -447,6 +447,7 @@ public static OmKeyInfo createOmKeyInfo(String volumeName, String bucketName,
         .setVolumeName(volumeName)
         .setBucketName(bucketName)
         .setKeyName(keyName)
+        .setFileName(keyName)

Review Comment:
   Good catch. In the test cases they are the same. But in prod it will extract the last element (just the file name):
   
   https://github.com/apache/ozone/blob/3e83eb6800a269eafb6a77a63303a46ba5882e5c/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java#L698-L699
   
   I will make the test helper more robust by wrapping it in `OzoneFSUtils.getFileName()` just like the prod one.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146899627


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   Yes. But Schema V3 block_data table key is not fully ASCII printable character. The logic to print it is untouched.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   Yes. But Schema V3 block_data table key are not fully ASCII printable characters. The logic to print it is untouched.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146956308


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   You mean just changing the separator from `: ` to `|` then?
   
   The actual `separator` is an empty string.
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L91



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   You mean just changing the printed separator from `: ` to `|` then?
   
   The actual `separator` is an empty string.
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L91



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146956308


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   You mean just changing the separator from `: ` to `|` then?



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4420:
URL: https://github.com/apache/ozone/pull/4420#discussion_r1146899627


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -136,208 +156,177 @@ public static byte[] getValueObject(
     }
   }
 
-  private static List<Object> displayTable(ManagedRocksIterator iterator,
+  private void displayTable(ManagedRocksIterator iterator,
       DBColumnFamilyDefinition dbColumnFamilyDefinition) throws IOException {
-    List<Object> outputs = new ArrayList<>();
+
+    if (fileName != null) {
+      try (PrintWriter out = new PrintWriter(fileName, UTF_8.name())) {
+        displayTable(iterator, dbColumnFamilyDefinition, out);
+      }
+    } else {
+      displayTable(iterator, dbColumnFamilyDefinition, out());
+    }
+  }
+
+  private void displayTable(ManagedRocksIterator iterator,
+      DBColumnFamilyDefinition dbColumnFamilyDefinition, PrintWriter out)
+      throws IOException {
 
     if (startKey != null) {
       iterator.get().seek(getValueObject(dbColumnFamilyDefinition));
     }
 
-    Writer fileWriter = null;
-    PrintWriter printWriter = null;
-    try {
-      if (fileName != null) {
-        fileWriter = new OutputStreamWriter(
-            new FileOutputStream(fileName), StandardCharsets.UTF_8);
-        printWriter = new PrintWriter(fileWriter);
-      }
+    if (withKey) {
+      // Start JSON object (map)
+      out.print("{ ");
+    } else {
+      // Start JSON array
+      out.print("[ ");
+    }
 
-      boolean schemaV3 = dnDBSchemaVersion != null &&
-          dnDBSchemaVersion.equals("V3");
-      while (iterator.get().isValid()) {
-        StringBuilder result = new StringBuilder();
-        if (withKey) {
-          Object key = dbColumnFamilyDefinition.getKeyCodec()
-              .fromPersistedFormat(iterator.get().key());
-          Gson gson = new GsonBuilder().setPrettyPrinting().create();
-          if (schemaV3) {
-            int index =
-                DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
-            String cid = key.toString().substring(0, index);
-            String blockId = key.toString().substring(index);
-            result.append(gson.toJson(Longs.fromByteArray(
-                FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));
-          } else {
-            result.append(gson.toJson(key));
-          }
-          result.append(" -> ");
-        }
-        Object o = dbColumnFamilyDefinition.getValueCodec()
-            .fromPersistedFormat(iterator.get().value());
-        outputs.add(o);
+    boolean schemaV3 = dnDBSchemaVersion != null &&
+        dnDBSchemaVersion.equalsIgnoreCase(SCHEMA_V3);
+
+    // Count number of keys printed so far
+    long count = 0;
+    while (withinLimit(count) && iterator.get().isValid()) {
+      StringBuilder sb = new StringBuilder();
+      if (withKey) {
+        Object key = dbColumnFamilyDefinition.getKeyCodec()
+            .fromPersistedFormat(iterator.get().key());
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
-        result.append(gson.toJson(o));
-        if (fileName != null) {
-          printWriter.println(result);
+        if (schemaV3) {
+          int index =
+              DatanodeSchemaThreeDBDefinition.getContainerKeyPrefixLength();
+          String cid = key.toString().substring(0, index);
+          String blockId = key.toString().substring(index);
+          sb.append(gson.toJson(Longs.fromByteArray(
+              FixedLengthStringUtils.string2Bytes(cid)) + ": " + blockId));

Review Comment:
   I get your idea. But Schema V3 table key is not fully composed of ASCII characters (gibberish if printed as-is). That is intended for seekability and space-saving I presume.
   
   e.g. for `block_data`:
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L42-L43
   
   https://github.com/apache/ozone/blob/886fc3d419e07a5736a2112fd748c3b212c2f11f/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java#L150-L154
   
   The logic to print it is left untouched. Arguably all valuable key info is extracted here already.



-- 
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: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] errose28 merged pull request #4420: HDDS-6064. Print proper JSON from `ozone debug ldb scan`

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 merged PR #4420:
URL: https://github.com/apache/ozone/pull/4420


-- 
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: issues-unsubscribe@ozone.apache.org

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


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