HDDS-6064. ozone debug ldb should print proper JSON with --with-keys=true

[sky76093016]

What changes were proposed in this pull request?

Use --with-keys=true to print the proper JSON.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-6064

How was this patch tested?

docker-compose test

[sky76093016]

@errose28 Please take a look.

[errose28]

Thanks for working on this command @sky76093016. I've often felt the existing implementation had some issues so hopefully we can get it fixed up here. I think the exit criteria here should be that the command always prints valid json to stdout.

Here's some items I think we can cover:

1. Modify TestOmDBCli to send all stdout to a different stream, and read this into gson to test that it is valid json.

   • See TestOzoneShellHA for an example of a test that does this kind of redirection.
   • The test data is not large, so having it all in memory in gson at once for the test should be ok.

2. Send all DBScanner scanner output that isn't json (like the 'added definition ...' output) to stderr.

   • TBH I'm not sure why this was added in HDDS-4595 but it looks pretty deliberate so I think separating it from the json to stderr is a good compromise.
   • I think if we log this instead of directly printing it, it will go to stderr by default, but try it out to see.

Of note, although making json by mixing library code like gson with manual string concatenation is usually bad, it's used here because the json size could be huge in a real DB, and we don't want that in memory. I think having a test for valid json output from the command will let us do this without errors.

[errose28]

Hi @adoroszlai thanks for picking this back up. I think this patch needs some more work. Changing -> to : is still not valid json. Piping the output to jq gives parse error: Expected string key before ':' at line 1, column 8. I think we should wrap the whole withKeys output block in a dictionary {} with commas between entries.

Some additional fixes are still outstanding as well:

• Modify TestOmDBCli to send all stdout to a different stream, and read this into gson to test that it is valid json.
• Send all DBScanner scanner output that isn't json (like the 'added definition ...' output) to stderr
• Flag change mentioned here
  • This may be a problem with the new compatibility guarantees we have discussed, but IMO debug commands are intended for developers to use on the fly, not for end users to put in scripts, so they should allow looser compatibility requirements.

[adoroszlai]

@errose28 Can you please take a look at the updated patch?

[smengcl]

Need to rebase this one. Not working on a recent OM DB:

❯ ./bin/ozone debug ldb --db=${HOME}/om.db scan --column_family=fileTable
2023-03-16 16:46:06,016 [main] INFO codec.RepeatedOmKeyInfoCodec: RepeatedOmKeyInfoCodec ignorePipeline = true
2023-03-16 16:46:06,018 [main] INFO codec.OmKeyInfoCodec: OmKeyInfoCodec ignorePipeline = true
2023-03-16 16:46:06,018 [main] INFO codec.OmKeyInfoCodec: OmKeyInfoCodec ignorePipeline = true
2023-03-16 16:46:06,021 [main] INFO codec.OmKeyInfoCodec: OmKeyInfoCodec ignorePipeline = true
2023-03-16 16:46:06,021 [main] INFO codec.OmKeyInfoCodec: OmKeyInfoCodec ignorePipeline = true
2023-03-16 16:46:06,021 [main] INFO codec.OmKeyInfoCodec: OmKeyInfoCodec ignorePipeline = true
Added definition for table:deletedTable
Added definition for table:userTable
Added definition for table:volumeTable
Added definition for table:openKeyTable
Added definition for table:keyTable
Added definition for table:bucketTable
Added definition for table:multipartInfoTable
Added definition for table:prefixTable
Added definition for table:dTokenTable
Added definition for table:s3SecretTable
Added definition for table:transactionInfoTable
Added definition for table:directoryTable
Added definition for table:fileTable
Added definition for table:openFileTable
Added definition for table:deletedDirectoryTable
Added definition for table:metaTable
Added definition for table:tenantAccessIdTable
Added definition for table:principalToAccessIdsTable
Added definition for table:tenantStateTable
No serializer found for class com.google.protobuf.UnknownFieldSet$Parser and no properties discovered to create BeanSerializer (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS) (through reference chain: java.util.LinkedHashMap["/-9223372036854774528/-9223372036854774016/-9223372036854773503/key1"]->org.apache.hadoop.ozone.om.helpers.OmKeyInfo["keyLocationVersions"]->java.util.ArrayList[0]->org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup["locationList"]->java.util.ArrayList[0]->org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo["blockID"]->org.apache.hadoop.hdds.client.BlockID["containerBlockID"]->org.apache.hadoop.hdds.client.ContainerBlockID["protobuf"]->org.apache.hadoop.hdds.protocol.proto.HddsProtos$ContainerBlockID["unknownFields"]->com.google.protobuf.UnknownFieldSet["parserForType"])

while the ldb tool compiled from latest master works:

❯ ./bin/ozone debug ldb --db=${HOME}/om.db scan --column_family=fileTable
{
  "volumeName": "vol1",
  "bucketName": "buck1",
  "keyName": "key1",
  "dataSize": 7,
  "keyLocationVersions": [
    {
      "version": 0,
      "locationVersionMap": {
        "0": [
          {
            "blockID": {
...

[smengcl]

With the container V3 support in DBScanner and significant test refactoring that happened in master branch I deem this not worth it to do a merge rebase. I will open a new PR with some diffs later.