HDDS-2534. scmcli container delete not working #389
Conversation
|
@xiaoyuyao please review. relates to #214 |
xiaoyuyao
requested review from
anuengineer,
bharatviswa504,
elek,
nandakumar131 and
xiaoyuyao
| Set<ContainerReplica> replicas = | ||
| containerStateManager.getContainerReplicas(containerID); | ||
| for(ContainerReplica replica : replicas){ | ||
| nodeManager.removeContainers(replica.getDatanodeDetails(), Stream.of( |
There was a problem hiding this comment.
Can we create the set of containerID outside the loop with a simpler Java APIs:
Set containerIdSet = Collections.singleton(containerID);
replias.forEach(replica->nodeManager.removeContainers(replica.getDatanodeDetails(), containerIdSet));
...-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
Outdated
Show resolved
Hide resolved
| " {} from datanode {},delete it.", | ||
| replicaProto.getContainerID(), | ||
| datanodeDetails); | ||
| publisher.fireEvent( |
There was a problem hiding this comment.
Should we fireEvent upon receive the scmcli request? Can we add some high level design description wrt the flow of the delete in the PR description? That will help the reviewers.
There was a problem hiding this comment.
+1 for @xiaoyuyao's description request. The delete container flow is quite important for Recon to work correctly and hence trying to understand this better. Will there be a case where a user will need to delete a container directly using SCM?
| @Override | ||
| public void removeContainers(DatanodeDetails datanodeDetails, | ||
| Set<ContainerID> containerIds) throws NodeNotFoundException { | ||
| nodeStateManager.removeContainers(datanodeDetails.getUuid(), containerIds); |
There was a problem hiding this comment.
Do we need to remove the container from the pipelineContainerMap in SCMPipelineManager?
There was a problem hiding this comment.
Before delete a container, we should close it; When we close a container , The function SCMContainerManager#updateContainerState will remove the container from pipelineContainerMap in SCMPipelineManager.
xiaoyuyao
left a comment
xiaoyuyao
left a comment
There was a problem hiding this comment.
Thanks @chimney-lee for working on this. I have some questions commented inline.
Add datanode info in Exception message
|
/pending Questions/suggestions from @xiaoyuyao are not yet addressed in the last commit. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Marking this issue as un-mergeable as requested.
Please use /ready comment when it's resolved.
Questions/suggestions from @xiaoyuyao are not yet addressed in the last commit.
There was a problem hiding this comment.
Marking this issue as un-mergeable as requested.
Please use /ready comment when it's resolved.
Questions/suggestions from @xiaoyuyao are not yet addressed in the last commit.
|
@chimney-lee Thank you for working on this, do you still working on? |
|
|
@maobaolong Sorry for this , it has been a long time, glad if someone could contine this work |
|
@chimney-lee Thank you for the past working on this, i will continue this work. |
|
Thanks @chimney-lee for the work on this so far, and @maobaolong for taking it up. I assume @maobaolong will open a new PR for the updated change, so closing this. |
7 participants
What changes were proposed in this pull request?
What changes were proposed in this pull request?
fix scmcli container delete not working
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2534
#214
How was this patch tested?
exec bin/ozone scmcli container delete