You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/08 06:28:46 UTC

[GitHub] [ozone] siddhantsangwan opened a new pull request, #3661: HDDS-6967. EC: Handle maintenance replicas in ECUnderReplicationHandler

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

   ## What changes were proposed in this pull request?
   
   Changes to handle under replication when it's because of replicas in maintenance DNs. 
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-6967
   
   ## How was this patch tested?
   
   New unit tests.


-- 
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 #3661: HDDS-6967. EC: Handle maintenance replicas in ECUnderReplicationHandler

Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3661:
URL: https://github.com/apache/ozone/pull/3661#discussion_r952433248


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -196,6 +196,29 @@ public Set<Integer> maintenanceIndexes() {
     return maintenanceIndexes.keySet();
   }
 
+  /**
+   * Gets a set containing all maintenance only indexes. These are replicas
+   * that are only on maintenance nodes, without any copies on healthy nodes.
+   *
+   * @param includePendingAdd true if any indexes that are pending an add
+   * {@link ContainerReplicaOp.PendingOpType#ADD} should be excluded
+   * @return set containing maintenance only indexes or empty set if none are
+   * in maintenance
+   */
+  public Set<Integer> maintenanceOnlyIndexes(boolean includePendingAdd) {

Review Comment:
   I think we should remove the other version of maintenanceOnlyIndexes that returns a list, and make everything use this one. It keeps it consistent with decommissionOnlyIndexes



-- 
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 #3661: HDDS-6967. EC: Handle maintenance replicas in ECUnderReplicationHandler

Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3661:
URL: https://github.com/apache/ozone/pull/3661#discussion_r952436527


##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECContainerReplicaCount.java:
##########
@@ -229,7 +229,7 @@ public void testAdditionalMaintenanceCopiesAllMaintenance() {
             Collections.emptyList(), 1);
     Assertions.assertFalse(rcnt.isSufficientlyReplicated(false));
     Assertions.assertFalse(rcnt.isOverReplicated(true));
-    Assertions.assertEquals(4, rcnt.additionalMaintenanceCopiesNeeded());
+    Assertions.assertEquals(4, rcnt.additionalMaintenanceCopiesNeeded(false));

Review Comment:
   All the calls I can see in the this test calss call `rcnt.additionalMaintenanceCopiesNeeded(false)` with `false`. We should add at least one test with some pending add and test the `true` condition too.



-- 
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 merged pull request #3661: HDDS-6967. EC: Handle maintenance replicas in ECUnderReplicationHandler

Posted by GitBox <gi...@apache.org>.
sodonnel merged PR #3661:
URL: https://github.com/apache/ozone/pull/3661


-- 
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 pull request #3661: HDDS-6967. EC: Handle maintenance replicas in ECUnderReplicationHandler

Posted by GitBox <gi...@apache.org>.
sodonnel commented on PR #3661:
URL: https://github.com/apache/ozone/pull/3661#issuecomment-1223878740

   The changes look good overall - I just have 2 minor comments. If we can address those it will be good to commit.


-- 
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] siddhantsangwan commented on pull request #3661: HDDS-6967. EC: Handle maintenance replicas in ECUnderReplicationHandler

Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on PR #3661:
URL: https://github.com/apache/ozone/pull/3661#issuecomment-1227138640

   @sodonnel thanks for the review. I've addressed your comments.


-- 
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