You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sodonnel (via GitHub)" <gi...@apache.org> on 2023/01/31 16:15:16 UTC

[GitHub] [ozone] sodonnel opened a new pull request, #4228: HDDS-7859. ICR processing does not remove container reference in NodeManager for a deleted replica

sodonnel opened a new pull request, #4228:
URL: https://github.com/apache/ozone/pull/4228

   ## What changes were proposed in this pull request?
   
   If a container is deleted on a datanode, then the deleted state is reported via an Incremental Container Report. This ICR processing will remove the deleted replica from the ContainerManager replica map, but it does not remove it from the NodeManager which has a mapping of all containers on a datanode.
   
   This can result in problems with decommissioning, where SCM thinks a node has a replica, but then ContainerManager reports no replicas.
   
   Most likely the issue would resolve itself when the next FCR is issued, as it will handle updating NodeManager for any containers which are present there but not in the full report.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-7859
   
   ## How was this patch tested?
   
   Modified a unit test to reproduce it and then fixed the issue.
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] kerneltime merged pull request #4228: HDDS-7859. ICR processing does not remove container reference in NodeManager for a deleted replica

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime merged PR #4228:
URL: https://github.com/apache/ozone/pull/4228


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] kerneltime commented on a diff in pull request #4228: HDDS-7859. ICR processing does not remove container reference in NodeManager for a deleted replica

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #4228:
URL: https://github.com/apache/ozone/pull/4228#discussion_r1092842733


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##########
@@ -91,6 +91,8 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
             if (!replicaProto.getState().equals(
                 ContainerReplicaProto.State.DELETED)) {
               nodeManager.addContainer(dd, id);
+            } else {

Review Comment:
   Should we check for `replicaProto.getState().equals(ContainerReplicaProto.State.DELETED)` to remove the container?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] sodonnel commented on a diff in pull request #4228: HDDS-7859. ICR processing does not remove container reference in NodeManager for a deleted replica

Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #4228:
URL: https://github.com/apache/ozone/pull/4228#discussion_r1092981536


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##########
@@ -91,6 +91,8 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
             if (!replicaProto.getState().equals(
                 ContainerReplicaProto.State.DELETED)) {
               nodeManager.addContainer(dd, id);
+            } else {

Review Comment:
   I switched the order around to make it more clear what is going on.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] sodonnel commented on a diff in pull request #4228: HDDS-7859. ICR processing does not remove container reference in NodeManager for a deleted replica

Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #4228:
URL: https://github.com/apache/ozone/pull/4228#discussion_r1092975416


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##########
@@ -91,6 +91,8 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
             if (!replicaProto.getState().equals(
                 ContainerReplicaProto.State.DELETED)) {
               nodeManager.addContainer(dd, id);
+            } else {

Review Comment:
   I did originally have an `else(DELETED)` statement, but then I realized the IF part is for "not deleted", so else can only be deleted. It may be safer for the future to make it an `else if`.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org