[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli
container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366639245
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##
@@ -563,6 +563,20 @@ public void setContainers(DatanodeDetails datanodeDetails,
return nodeStateManager.getContainers(datanodeDetails.getUuid());
}
+ /**
+ * Remove set of containers available on a datanode.
+ *
+ * @param datanodeDetails - DatanodeID
+ * @param containerIds- Set of containerIDs
+ * @throws NodeNotFoundException - if datanode is not known. For new datanode
+ * use addDatanodeInContainerMap call.
+ */
+ @Override
+ public void removeContainers(DatanodeDetails datanodeDetails,
+ Set containerIds) throws NodeNotFoundException {
+nodeStateManager.removeContainers(datanodeDetails.getUuid(), containerIds);
Review comment:
Do we need to remove the container from the pipelineContainerMap in
SCMPipelineManager?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli
container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366638690
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java
##
@@ -119,21 +127,33 @@ public void onMessage(final ContainerReportFromDatanode
reportFromDatanode,
* @param datanodeDetails Datanode from which this report was received
* @param replicas list of ContainerReplicaProto
*/
- private void processContainerReplicas(final DatanodeDetails datanodeDetails,
- final List replicas) {
+ private List processContainerReplicas(
+ final DatanodeDetails datanodeDetails,
+ final List replicas,
+ final EventPublisher publisher) {
+List containers = new ArrayList<>();
for (ContainerReplicaProto replicaProto : replicas) {
try {
processContainerReplica(datanodeDetails, replicaProto);
+containers.add(replicaProto);
} catch (ContainerNotFoundException e) {
-LOG.error("Received container report for an unknown container" +
-" {} from datanode {}.", replicaProto.getContainerID(),
-datanodeDetails, e);
+LOG.info("Received container report for an unknown container" +
+" {} from datanode {},delete it.",
+replicaProto.getContainerID(),
+datanodeDetails);
+publisher.fireEvent(
Review 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.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli
container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366605305
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##
@@ -282,6 +290,12 @@ public ContainerInfo allocateContainer(final
ReplicationType type,
public void deleteContainer(ContainerID containerID) throws IOException {
lock.lock();
try {
+ Set replicas =
+ containerStateManager.getContainerReplicas(containerID);
+ for(ContainerReplica replica : replicas){
+nodeManager.removeContainers(replica.getDatanodeDetails(), Stream.of(
Review 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));
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli
container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366606214
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##
@@ -293,6 +307,12 @@ public void deleteContainer(ContainerID containerID)
throws IOException {
" it's missing!", containerID);
}
scmContainerManagerMetrics.incNumSuccessfulDeleteContainers();
+} catch (NodeNotFoundException nnfe) {
+ scmContainerManagerMetrics.incNumFailureDeleteContainers();
+ throw new SCMException(
+ "Failed to delete container " + containerID + ", reason : " +
+ "node doesn't exist.",
Review comment:
Can we add the node info to the Exception? Is it part of the
NodeNotFoundException?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli
container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366605305
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##
@@ -282,6 +290,12 @@ public ContainerInfo allocateContainer(final
ReplicationType type,
public void deleteContainer(ContainerID containerID) throws IOException {
lock.lock();
try {
+ Set replicas =
+ containerStateManager.getContainerReplicas(containerID);
+ for(ContainerReplica replica : replicas){
+nodeManager.removeContainers(replica.getDatanodeDetails(), Stream.of(
Review comment:
Can we create the set of containerID outside the loop with a simpler Java
APIs:
Set containerIdSet = Collections.singleton(containerID);
replias.forEach(replica->replica.removeContainers(replica.getDatanodeDetails(),
containerIdSet));
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
