ozone
ozone copied to clipboard
apache
HDDS-8188. Support max allowed length in response of ozone admin container list
What changes were proposed in this pull request?
ozone admin container list has a --count parameter that defaults to 20. This has caused confusion among users, and it does not provide a way to list all containers. We should support --all to list all containers(within max allowed count of 'hdds.container.list.max.count'. If the total count exceeds max allowed count, CLI prints a message to stdout indicating the result is truncated.) (To view the full list of container, need to implement pagination https://issues.apache.org/jira/browse/HDDS-10687 for list-container operation.)
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8188
How was this patch tested?
unit test.
Please see HDDS-8887 for similar functionality implemented in a more user-friendly way in other list commands.
It's good to have an "all" option, but I don't think we should make it default, as in a production cluster, there will be hundreds of thousands of containers there.
The entire protobuf message has to be staged on the server before it is returned to the client, right? So what is the overhead in memory of making this the default with millions of containers in the system?
I don't think listing all should be the default, and if you want it to be, the client should bring them down from the server in pages and make it invisible to the end user.
Agreed with Stephen and Sammi, if we have to load all the containers into one proto and send it over that is probably not optimal. When I suggested this should be the default last year I think I was imagining something like our ls implementation, where we use pagination to abstract the listing within the client. For example, user passes --all but internally the client fetches containers in batches of 1000 or some other number and prints them as it goes.
If the SCM doesn't do this already then that might be a change for a different PR, and here we can just add a non-default --all option.
For context, --all as default helps avoid issues in remote troubleshooting where someone asks to share output of this command and the user runs it, doesn't realize they've gotten a truncated list, and sends the truncated list so both people think it is the full output. This can also be mitigated by printing a message to stderr indicating the result is truncated.
Added a ScmConfigKeys#hdds.container.list.max.count to allow the user to configure themselves the max count of containers allowed to be included in list-container operation's response. If the result is over the max count allowed, printing a message to stdout indicating the result is truncated. To view the rest of container list, need to implement pagination (https://issues.apache.org/jira/browse/HDDS-10687) for list-container operation.
hello team~ feel free to let me know if there is any further question about this pr. Thanks! (cc. @ChenSammi @Galsza @errose28
Sorry for getting back late, I've left two minor comments otherwise LGTM
yeah! refactored two listContainer methods, to make them reuse common code as much as possible. Thanks @Galsza for the suggestion!
Hello @Galsza ! would you mind granting an approval for this pr if there is no further questions? Thanks!