You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Tejaskriya (via GitHub)" <gi...@apache.org> on 2024/04/18 07:12:39 UTC
[PR] HDDS-10464. Add integration tests for fail-early [ozone]
Tejaskriya opened a new pull request, #6555:
URL: https://github.com/apache/ozone/pull/6555
## What changes were proposed in this pull request?
[HDDS-10462](https://issues.apache.org/jira/browse/HDDS-10462) added fail early conditions for datanode decommission. And [HDDS-10463](https://issues.apache.org/jira/browse/HDDS-10463) added fail early conditions for datanode maintenance. This PR adds few integration tests in TestDecommissionAndMaintenance for the same.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10464
## How was this patch tested?
Green CI on my fork: (Waiting, will move out of draft once it's done)
--
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
Re: [PR] HDDS-10464. Add integration tests for fail-early [ozone]
Posted by "siddhantsangwan (via GitHub)" <gi...@apache.org>.
siddhantsangwan merged PR #6555:
URL: https://github.com/apache/ozone/pull/6555
--
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
Re: [PR] HDDS-10464. Add integration tests for fail-early [ozone]
Posted by "siddhantsangwan (via GitHub)" <gi...@apache.org>.
siddhantsangwan commented on code in PR #6555:
URL: https://github.com/apache/ozone/pull/6555#discussion_r1584596854
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java:
##########
@@ -310,6 +310,77 @@ public void testDecommissioningNodesCompleteDecommissionOnSCMRestart()
waitForDnToReachPersistedOpState(newDn, IN_SERVICE);
}
+ @Test
+ // Decommissioning few nodes which leave insufficient nodes for replication
+ // should not be allowed if the decommissioning is not forced.
+ public void testInsufficientNodesCannotBeDecommissioned()
+ throws Exception {
+ // Generate some data on the empty cluster to create some containers
+ generateData(20, "key", ratisRepConfig);
+
+ final List<DatanodeDetails> toDecommission = nm.getAllNodes();
+
+ // trying to decommission 5 nodes should leave the cluster with 2 nodes,
+ // which is not sufficient for RATIS.THREE replication. It should not be allowed.
+ scmClient.decommissionNodes(Arrays.asList(toDecommission.get(0).getIpAddress(),
+ toDecommission.get(1).getIpAddress(), toDecommission.get(2).getIpAddress(),
+ toDecommission.get(3).getIpAddress(), toDecommission.get(4).getIpAddress()), false);
+
+ // Ensure no nodes transitioned to DECOMMISSIONING
+ List<DatanodeDetails> decomNodes = nm.getNodes(
+ DECOMMISSIONED,
+ HEALTHY);
+ assertEquals(0, decomNodes.size());
Review Comment:
Shouldn't we ensure that the DN is neither decommissioned nor decommissioning? When a decommission command is received, the DN will transition from in service to decommissioning to decommissioned. So I think we should check for both of these states.
--
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
Re: [PR] HDDS-10464. Add integration tests for fail-early [ozone]
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #6555:
URL: https://github.com/apache/ozone/pull/6555#issuecomment-2068534201
@swamirishi thank you for the review and approval! I just refactored some variable names that were misleading, Could you please review it again?
--
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
Re: [PR] HDDS-10464. Add integration tests for fail-early [ozone]
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #6555:
URL: https://github.com/apache/ozone/pull/6555#discussion_r1587170021
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java:
##########
@@ -310,6 +310,77 @@ public void testDecommissioningNodesCompleteDecommissionOnSCMRestart()
waitForDnToReachPersistedOpState(newDn, IN_SERVICE);
}
+ @Test
+ // Decommissioning few nodes which leave insufficient nodes for replication
+ // should not be allowed if the decommissioning is not forced.
+ public void testInsufficientNodesCannotBeDecommissioned()
+ throws Exception {
+ // Generate some data on the empty cluster to create some containers
+ generateData(20, "key", ratisRepConfig);
+
+ final List<DatanodeDetails> toDecommission = nm.getAllNodes();
+
+ // trying to decommission 5 nodes should leave the cluster with 2 nodes,
+ // which is not sufficient for RATIS.THREE replication. It should not be allowed.
+ scmClient.decommissionNodes(Arrays.asList(toDecommission.get(0).getIpAddress(),
+ toDecommission.get(1).getIpAddress(), toDecommission.get(2).getIpAddress(),
+ toDecommission.get(3).getIpAddress(), toDecommission.get(4).getIpAddress()), false);
+
+ // Ensure no nodes transitioned to DECOMMISSIONING
+ List<DatanodeDetails> decomNodes = nm.getNodes(
+ DECOMMISSIONED,
+ HEALTHY);
+ assertEquals(0, decomNodes.size());
Review Comment:
Thanks for the review! I have added checks for both decommissioning and decommissioned now. Could you please take another look at it and approve the workflows if everything seems fine?
--
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