HDDS-2651 Make startId parameter non-mandatory while listing containe…#283
HDDS-2651 Make startId parameter non-mandatory while listing containe…#283lokeshj1703 merged 1 commit intoapache:masterfrom
Conversation
lokeshj1703
left a comment
lokeshj1703
left a comment
There was a problem hiding this comment.
@nilotpalnandi Thanks for working on the PR! The changes look good to me. Pending jenkins.
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @nilotpalnandi for addressing this usability issue. It has been bugging me for some time now, too.
There was a problem hiding this comment.
The interface doc says startId is exclusive:
https://github.com/apache/hadoop-ozone/blob/cd3f8c9f8316ae742ffeb497ab77581a4519173c/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManager.java#L77-L92
I don't know how "backward compatible" the interface needs to be at this stage, but I think it would be better to change the default startId to 0 in scmcli instead. Otherwise, please update the javadoc, too.
CC @anuengineer
There was a problem hiding this comment.
Agree, +1 to change the startId to 0 for the scmcli list container command.
There was a problem hiding this comment.
+1 from me too. we can break backward compact now, no issues at all. We are still at the Alpha tag. As soon as we move to beta rules change :)
There was a problem hiding this comment.
If the user specifies startID =1 , it lists containers starting from container id#2. (not container id#1)
I think it would be good if the startID is kept intact and make the iteration of container inclusive of startID for better user experience.
There was a problem hiding this comment.
@anuengineer , @xiaoyuyao - Can you please take a look ?
There was a problem hiding this comment.
+1 on changing the startId to 0.
If we change the behavior of listContainer it will make it complex to implement a remote iterator to continuously list all the containers.
…rs through shell command
nilotpalnandi
force-pushed
the
HDDS-2651
branch
from
93cb7f9 to
c0e9298
Compare
|
@nilotpalnandi Thanks for the contribution! @adoroszlai @anuengineer @xiaoyuyao @nandakumar131 Thanks for the reviews! I have merged the PR to master branch. |
Merge in SDPOZONE/component-ozone from SDPOZN-1825 to sdp-ozone-1.4 * commit '640346bad04cb0ada17ace79b0cb35806a1f564c': (3 commits) HDDS-7188. Read chunk files using netty ChunkedNioFile. (apache#7625) HDDS-10488. Datanode OOM due to run out of mmap handler (apache#6690) ...
6 participants
…rs through shell command
What changes were proposed in this pull request?
Currently the startId "--start | -s " is a mandatory parameter for container list shell command
Need to make it as non-mandatory parameter as the default value for startId parameter is already defined.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2651
How was this patch tested?
Applied the patch and rebuilt ozone and then tested it by creating docker cluster using docker-compose