HDDS-3867. Extend the chunkinfo tool to display information from all nodes in the pipeline.

[sadanand48]

What changes were proposed in this pull request?

Currently the chunk-info tool (HDDS-3134) inside ozone debug displays information (chunk/block files that constitute the key) only from the first node of the pipeline. The change here is to extend it for the replicas such that the tool will display all the chunks/blocks in all the replicas

What is the link to the Apache JIRA

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

How was this patch tested?

Tested manually on docker.

Tool o/p:

{
  "KeyLocations": [
    [
      {
        "Datanode-HostName": "ozone_datanode_2.ozone_default",
        "Datanode-IP": "172.25.0.5",
        "Container-ID": 1,
        "Block-ID": 104477715974127616,
        "Locations": {
          "files": [
            "/data/hdds/hdds/5d3f3f5d-90a5-49ee-a6ee-8c1c79d045ea/current/containerDir0/1/chunks/104477715974127616.block"
          ],
          "pipelineID": "9e626531-3538-4930-bd5f-aa90e6fe9d74"
        }
      },
      {
        "Datanode-HostName": "ozone_datanode_3.ozone_default",
        "Datanode-IP": "172.25.0.8",
        "Container-ID": 1,
        "Block-ID": 104477715974127616,
        "Locations": {
          "files": [
            "/data/hdds/hdds/5d3f3f5d-90a5-49ee-a6ee-8c1c79d045ea/current/containerDir0/1/chunks/104477715974127616.block"
          ],
          "pipelineID": "9e626531-3538-4930-bd5f-aa90e6fe9d74"
        }
      },
      {
        "Datanode-HostName": "ozone_datanode_1.ozone_default",
        "Datanode-IP": "172.25.0.6",
        "Container-ID": 1,
        "Block-ID": 104477715974127616,
        "Locations": {
          "files": [
            "/data/hdds/hdds/5d3f3f5d-90a5-49ee-a6ee-8c1c79d045ea/current/containerDir0/1/chunks/104477715974127616.block"
          ],
          "pipelineID": "9e626531-3538-4930-bd5f-aa90e6fe9d74"
        }
      }
    ],
    [
      {
        "Datanode-HostName": "ozone_datanode_2.ozone_default",
        "Datanode-IP": "172.25.0.5",
        "Container-ID": 1,
        "Block-ID": 104477715974586369,
        "Locations": {
          "files": [
            "/data/hdds/hdds/5d3f3f5d-90a5-49ee-a6ee-8c1c79d045ea/current/containerDir0/1/chunks/104477715974586369.block"
          ],
          "pipelineID": "9e626531-3538-4930-bd5f-aa90e6fe9d74"
        }
      },
      {
        "Datanode-HostName": "ozone_datanode_3.ozone_default",
        "Datanode-IP": "172.25.0.8",
        "Container-ID": 1,
        "Block-ID": 104477715974586369,
        "Locations": {
          "files": [
            "/data/hdds/hdds/5d3f3f5d-90a5-49ee-a6ee-8c1c79d045ea/current/containerDir0/1/chunks/104477715974586369.block"
          ],
          "pipelineID": "9e626531-3538-4930-bd5f-aa90e6fe9d74"
        }
      },
      {
        "Datanode-HostName": "ozone_datanode_1.ozone_default",
        "Datanode-IP": "172.25.0.6",
        "Container-ID": 1,
        "Block-ID": 104477715974586369,
        "Locations": {
          "files": [
            "/data/hdds/hdds/5d3f3f5d-90a5-49ee-a6ee-8c1c79d045ea/current/containerDir0/1/chunks/104477715974586369.block"
          ],
          "pipelineID": "9e626531-3538-4930-bd5f-aa90e6fe9d74"
        }
      }
    ]
  ]
}

[elek]

Thanks to create this patch @sadanand48

I have one design concern related to this change. I am not sure what is the good answer, but trying to share my view:

This patch modifies the client interface to provide more functionality for a debug tool. I like this more functionality, but I think we shouldn't add any debug specific feature to the main client API. Client API should be backward compatible between releases and it might have multiple implementations.

I would prefer to separate the client api from the admin api (used by this tool).

As an example: Let's say I would like to implement a new client for Erasure Coding. If the client interface contains debug specific methods I need to implement all of them, which makes introduce any new feature harder.

I am not sure how is it possible to keep this separation (I need your feedback). One possible solution is ot move the logic of looping over the datanodes to the ChunkKeyHandler. But it's not clear how can it work with RATIS pipeline. Or do you use STANDALONE client to read the data wich is persisted with RATIS?

[bshashikant]

Thanks to create this patch @sadanand48

I have one design concern related to this change. I am not sure what is the good answer, but trying to share my view:

This patch modifies the client interface to provide more functionality for a debug tool. I like this more functionality, but I think we shouldn't add any debug specific feature to the main client API. Client API should be backward compatible between releases and it might have multiple implementations.

I would prefer to separate the client api from the admin api (used by this tool).

As an example: Let's say I would like to implement a new client for Erasure Coding. If the client interface contains debug specific methods I need to implement all of them, which makes introduce any new feature harder.

I am not sure how is it possible to keep this separation (I need your feedback). One possible solution is ot move the logic of looping over the datanodes to the ChunkKeyHandler. But it's not clear how can it work with RATIS pipeline. Or do you use STANDALONE client to read the data which is persisted with RATIS?

I think we should remove XceiverClient interface altogether. Read and write path client functionality seem to diverge a lot .
Adding a new functionality in XceiverClientGrpc will just be fine then.

There is alredy a jira filed for the same https://issues.apache.org/jira/browse/HDDS-998.

[bshashikant]

Thanks to create this patch @sadanand48
I have one design concern related to this change. I am not sure what is the good answer, but trying to share my view:
This patch modifies the client interface to provide more functionality for a debug tool. I like this more functionality, but I think we shouldn't add any debug specific feature to the main client API. Client API should be backward compatible between releases and it might have multiple implementations.
I would prefer to separate the client api from the admin api (used by this tool).
As an example: Let's say I would like to implement a new client for Erasure Coding. If the client interface contains debug specific methods I need to implement all of them, which makes introduce any new feature harder.
I am not sure how is it possible to keep this separation (I need your feedback). One possible solution is ot move the logic of looping over the datanodes to the ChunkKeyHandler. But it's not clear how can it work with RATIS pipeline. Or do you use STANDALONE client to read the data which is persisted with RATIS?

I think we should remove XceiverClient interface altogether. Read and write path client functionality seem to diverge a lot .
Adding a new functionality in XceiverClientGrpc will just be fine then.

There is alredy a jira filed for the same https://issues.apache.org/jira/browse/HDDS-998.

@elek , I feel if we can go ahead with this change and then try to remove XceiverClient interface altogether in a separte jira. Your thoughts??

[elek]

@elek , I feel if we can go ahead with this change and then try to remove XceiverClient interface altogether in a separte jira. Your thoughts??

We need some kind of interfaces if we would like to support different type of write path. This interface might not be so low-level as today, but something is required (maybe just a OutputStream factory?).

But it doesn't answer my original concern about mixing client specific and admin specific methods in the same interface. Even with a dedicated interface, if it's part of the client API we need to make it backward compatible forever.

I am not fully against it, but I think we need a strong argument to extend client interface. For example why don't we use any CLI tool on the datanode which can read the db and blocks data directly? In that case we don't need to add complexity for RPC interface at all?

[bshashikant]

@elek , I feel if we can go ahead with this change and then try to remove XceiverClient interface altogether in a separte jira. Your thoughts??

We need some kind of interfaces if we would like to support different type of write path. This interface might not be so low-level as today, but something is required (maybe just a OutputStream factory?).

But it doesn't answer my original concern about mixing client specific and admin specific methods in the same interface. Even with a dedicated interface, if it's part of the client API we need to make it backward compatible forever.

I am not fully against it, but I think we need a strong argument to extend client interface. For example why don't we use any CLI tool on the datanode which can read the db and blocks data directly? In that case we don't need to add complexity for RPC interface at all?

Let's define a new admin interface and add the method there.

The idea of the tool is to read the info and do some sort of validation on each replica of the block and it should be possible to run it from any node.

[elek]

it should be possible to run it from any node.

Do you have any specific reason for this requirement? Admins can easily login to the datanodes and execute validation. (Not against it, just curious...)

[bshashikant]

it should be possible to run it from any node.

Do you have any specific reason for this requirement? Admins can easily login to the datanodes and execute validation. (Not against it, just curious...)

The cmd needs to connection to OM to fetch the block info first and then datanodes to read the chunk info to operate successfully.

Secondly, as similar to hdfs, admin cmds can be run from any node. The idea is to mimic the same here.

[elek]

Thanks to explain it @bshashikant, let me ask differently:

Secondly, as similar to hdfs, admin cmds can be run from any node. The idea is to mimic the same here.

Well, if ithere is no strong argument, we can follow different patterns IMHO.

The cmd needs to connection to OM to fetch the block info first and then datanodes to read the chunk info to operate successfully.

Seems to be a strong argument (thanks to share), we can do offline tool, but it requires different model (knowing the address of dn and blockid). If we alread use OM I understand why this is the plan.

[elek]

Ok, I checked again the code and I can acknowledge that I was wrong.

It's hard to say if it's admin related call which is added but a very generic one and (in the mean-time) I learned that we might need something similar for EC (for non-admin calls)

The client RPC API is not changed, it's just a client side optimization to initialize multiple, existing client calls.

I apologize for the delay what I caused. Or: thanks for the discussion, I like to have interesting discussions about possible design (as we have no other forum for that), but I know that it may require patience ...

In exchange, I am happy to merge this one (if @bshashikant has no other concerns).

[bshashikant]

@elek , please go ahead with the merge.