errose28 commented on PR #7266: URL: https://github.com/apache/ozone/pull/7266#issuecomment-2941523851
Hi @slfan1989 I appreciate your response and the work you've done on these changes. My suggestion to split this PR down was not meant to diminish any of the work that has been put in here, but to speed up in incorporating the work into Ozone. A change like this approaching 1k lines, 70+ review comments, and encompassing multiple items is going to be large for any reviewer, and I think we could make faster progress by splitting it. For example, I think we could iterate on the volume info RPC in SCM pretty quickly and get that change merged first. I'm ok with adding volume failure time and capacity lost to SCM as well, but it will be easier to review those as their own change. This way most of the work here can be merged while we discuss the CLI. > The design of [#7266](https://github.com/apache/ozone/pull/7266) is inspired by HDFS's disk failure detection mechanism, with the goal of improving the system's ability to identify and locate failed disks. For users migrating from HDFS to Ozone, using the volume command to directly view failed disks can offer a more intuitive and convenient operational experience. Can you add more details about how this is similar to HDFS? I'm familiar with `hdfs dfsadmin -report` to list failed volumes, but that breaks the information down at the node level with failed volume counters for each node, which seems more similar to an `ozone admin datanode list` command. > I believe this feature does not impact [HDDS-13096](https://issues.apache.org/jira/browse/HDDS-13096) or [HDDS-13097](https://issues.apache.org/jira/browse/HDDS-13097). Yes we could have both `ozone admin datanode list/info` and `ozone admin datanode volume list` without code conflicts, but we need to build a maintainable and intuitive CLI, which means we should avoid commands that do the same or similar things. In this case I think we should standardize one way to get volume information from the CLI. I propose keeping this within the `datanode info` command because volumes are completely contained within a node, unlike cross cutting concepts like containers and pipelines which have their own subcommands. Based on early comments like [this](https://github.com/apache/ozone/pull/8405#discussion_r2106224868) I think we would end up needing node filtering in `ozone admin datanode volume list` at which point it becomes very similar to `ozone admin datanode list/info`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
