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/10/17 13:27:05 UTC
[GitHub] [ozone] pratapchandu opened a new pull request, #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient and …
pratapchandu opened a new pull request, #3847:
URL: https://github.com/apache/ozone/pull/3847
…enable TestRatisPipelineCreateAndDestroy
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
## What is the link to the Apache JIRA
(Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
--
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] pratapchandu closed pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu closed pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
URL: https://github.com/apache/ozone/pull/3847
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r1000109336
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
This issue was not fixed. Without any change the test still fails.
With out current change it throws exception with WRONG result code. so this change helps to fix the issue.
And changing expected code to FAILED_TO_FIND_NODES_WITH_SPACE in test , is accepting wrong result code .
so current change is better option when compared to above test change.
--
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] pratapchandu commented on pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on PR #3847:
URL: https://github.com/apache/ozone/pull/3847#issuecomment-1301934502
@neils-dev , updated code
it throws Exception with result code FAILED_TO_FIND_HEALTHY_NODES when no healthy node is available.
changed test to accept FAILED_TO_FIND_HEALTHY_NODES.
--
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] pratapchandu closed pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu closed pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
URL: https://github.com/apache/ozone/pull/3847
--
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 pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3847:
URL: https://github.com/apache/ozone/pull/3847#issuecomment-1282961460
Can you add some more explanation as to why adding the exception helped the test cases pass reliably?
--
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] aswinshakil commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r999926450
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
I don't think this assertion was the issue previously,
https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148
This issue was created long back, I think it was fixed by subsequent code changes over the year.
Even without adding this change, it would have thrown an exception as `nodesWithSpace` is a subset of `healthyNodes`. So I don't think we need to be checking it again. As for the test change I did this,
> 1. changing expected code to FAILED_TO_FIND_NODES_WITH_SPACE in test
--
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 pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient and …
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3847:
URL: https://github.com/apache/ozone/pull/3847#issuecomment-1281110122
cc @devmadhuu @neils-dev @aswinshakil
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998926440
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Existing code is not checking required condition healthyNodes.size() < nodesRequired (where new check was added in the current change) before calling filterNodesWithSpace.
--
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] pratapchandu commented on pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on PR #3847:
URL: https://github.com/apache/ozone/pull/3847#issuecomment-1303071487
Thanks @neils-dev, @kerneltime, @devmadhuu, @aswinshakil for review. It passed all checks after re-run.
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998933900
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
With the current fix , it is checking condition before calling filterNodesWithSpace, and throwing exception with code FAILED_TO_FIND_SUITABLE_NODE.
Test is passing.
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998926440
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Existing code is not checking required condition healthyNodes.size() < nodesRequired (where new check was added in the current change) before calling filterNodesWithSpace.
filterNodesWithSpace checking the condition and throwing the exception with code FAILED_TO_FIND_NODES_WITH_SPACE.
[ozone/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java](https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148-L149)
Test is failing with the following assert failure :
[ERROR] org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart Time elapsed: 55.337 s <<< FAILURE!
java.lang.AssertionError: expected:<FAILED_TO_FIND_SUITABLE_NODE> but was:<FAILED_TO_FIND_NODES_WITH_SPACE>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotEquals(Assert.java:835)
at org.junit.Assert.assertEquals(Assert.java:120)
at org.junit.Assert.assertEquals(Assert.java:146)
at org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart(TestRatisPipelineCreateAndDestroy.java:146
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r1012758135
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Updated the code to use FAILED_TO_FIND_HEALTHY_NODES .
--
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 #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
kerneltime merged PR #3847:
URL: https://github.com/apache/ozone/pull/3847
--
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] neils-dev commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r999921701
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
@kerneltime, this patch looks to correct the failing disabled integration test, `TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart.` The test fails because the exception thrown in the test differs from the expected exception. The patch ensures the expected exception is thrown. @pratapchandu, thanks for filing the PR. Updating the Jira with a brief description of the error would also be helpful.
> Can you add some more explanation as to why adding the exception helped the test cases pass reliably?
--
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] aswinshakil commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r1000951396
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
I'm good with the change. I was wondering if this can be moved up, Is there a downside to it?
https://github.com/apache/ozone/blob/1ff97328b7de88bc35c1665f1c90c0782b3cf7e2/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java#L130-L143
--
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] adoroszlai commented on pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3847:
URL: https://github.com/apache/ozone/pull/3847#issuecomment-1308936931
Unfortunately the PR only fixed one test case but enabled the whole test class. Now there are intermittent failures in the test cases that were not updated:
* https://github.com/adoroszlai/ozone-build-results/blob/master/2022/11/08/18312/it-hdds/hadoop-ozone/integration-test/org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.txt
* https://github.com/adoroszlai/ozone-build-results/blob/master/2022/11/08/18321/it-hdds/hadoop-ozone/integration-test/org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.txt
* https://github.com/adoroszlai/ozone-build-results/blob/master/2022/11/08/18331/it-hdds/hadoop-ozone/integration-test/org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.txt
--
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] neils-dev commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r1013043646
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Thanks. Looks good.
--
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] neils-dev commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r999937402
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
For the pipeline placement, the following seems to be expected when choosing nodes, first check healthy nodes, remove excluded and others, then check size and return finalized list. This is followed in the comments of `filterViableNodes `https://github.com/apache/ozone/blob/fc0854b391ddc7716215907f7915f4b3f88211d6/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java#L109 and in the `SCMCommonPlacementPolicy.chooseDatanodesInternal` and looks reasonable to follow here.
For the testcase,` testPipelineCreationOnNodeRestart()`, there are no datanodes, so the check for healthynodes should raise an exception **prior** to check for sufficient size raising the `FAILED_TO_FIND_NODES_WITH_SPACE` exception. The healthynode exception may actually be something other then `FAILED_TO_FIND_SUITABLE_NODE`, and perhaps `SCMException.ResultCodes.FAILED_TO_FIND_HEALTHY_NODES` to be consistent with the exception raised in `SCMCommonPlacementPolicy.chooseDatanodesInternal` ?
Something similar to,
```
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
if (healthyNodes.size() == 0) {
msg = "No healthy node found to allocate container.";
LOG.error(msg);
throw new SCMException(msg, SCMException.ResultCodes
.FAILED_TO_FIND_HEALTHY_NODES);
```
?
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998926440
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Existing code is not checking required condition healthyNodes.size() < nodesRequired (where new check was added in the current change) before calling filterNodesWithSpace.
filterNodesWithSpace checking the condition and throwing the exception FAILED_TO_FIND_NODES_WITH_SPACE.
[ozone/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java](https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148-L149)
Test is failing with the following assert failure :
[ERROR] org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart Time elapsed: 55.337 s <<< FAILURE!
java.lang.AssertionError: expected:<FAILED_TO_FIND_SUITABLE_NODE> but was:<FAILED_TO_FIND_NODES_WITH_SPACE>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotEquals(Assert.java:835)
at org.junit.Assert.assertEquals(Assert.java:120)
at org.junit.Assert.assertEquals(Assert.java:146)
at org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart(TestRatisPipelineCreateAndDestroy.java:146
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998926440
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Existing code is not checking required condition healthyNodes.size() < nodesRequired (where new check was added in the current change) before calling filterNodesWithSpace.
filterNodesWithSpace checking the condition and throwing the exception with code FAILED_TO_FIND_NODES_WITH_SPACE.
https://github.com/apache/ozone/blob/1ff97328b7de88bc35c1665f1c90c0782b3cf7e2/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java#L241
[ozone/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java](https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148-L149)
Test is failing with the following assert failure :
[ERROR] org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart Time elapsed: 55.337 s <<< FAILURE!
java.lang.AssertionError: expected:<FAILED_TO_FIND_SUITABLE_NODE> but was:<FAILED_TO_FIND_NODES_WITH_SPACE>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotEquals(Assert.java:835)
at org.junit.Assert.assertEquals(Assert.java:120)
at org.junit.Assert.assertEquals(Assert.java:146)
at org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart(TestRatisPipelineCreateAndDestroy.java:146
--
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] neils-dev commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r999921701
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
@kerneltime, this patch looks to correct the failing disabled integration test, `TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart.` The test fails because the exception thrown in the test differs from the expected exception. The patch ensures the expected exception is thrown. @pratapchandu, thanks for filing the PR. Updating the Jira with a brief description of the error would also be helpful.
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r1013671961
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
@aswinshakil thanks for review , resolving this
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998950546
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Two others alternative fixes are working fine
1) changing expected code to FAILED_TO_FIND_NODES_WITH_SPACE in test
https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148
But is it ok to expect FAILED_TO_FIND_NODES_WITH_SPACE after shutting down all data nodes ?
@aswinshakil is this same as the change tried by you ?
2) Removing assertEquals on result code
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998926440
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Existing code is not checking required condition healthyNodes.size() < nodesRequired (where new check was added in the current change) before calling filterNodesWithSpace.
filterNodesWithSpace checking the condition and throwing the exception FAILED_TO_FIND_NODES_WITH_SPACE.
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998950546
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Two others alternative fixes are working fine
1) changing expected code to FAILED_TO_FIND_NODES_WITH_SPACE in test
https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148
But is it ok to expect FAILED_TO_FIND_NODES_WITH_SPACE after shutting down all data nodes ?
2) Removing assertEquals on result code
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r1000109336
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
This issue was not fixed. Without any change the test still fails.
With out current change it throws exception with WRONG result code. so this change helps to fix the issue.
And changing expected to FAILED_TO_FIND_NODES_WITH_SPACE in test , is accepting wrong result code .
so current change is better option when compared to above test change.
--
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] aswinshakil commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998725874
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
We are checking this at multiple places here itself. For eg, We reassign `healthyNodes` with `filterNodesWithSpace()`, itself has check for `nodesWithSpace` which is returned back so theoretically `nodesWithSpace <= healthyNodes`.
https://github.com/apache/ozone/blob/1ff97328b7de88bc35c1665f1c90c0782b3cf7e2/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java#L226-L242
I don't think this is fixing the problem, I tried running the test with just modifying this line, and it worked.
https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148-L149
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r1000109336
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
This issue was not fixed, without any change the test still fails.
With out current change it throws exception with WRONG result code, this change helps to fix the issue,
and changing expected code to FAILED_TO_FIND_NODES_WITH_SPACE in test , is accepting wrong result code .
so current change is better option when compared to above test change.
--
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] pratapchandu commented on a diff in pull request #3847: HDDS-3419. Throw exception when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r998926440
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
Existing code is not checking required condition healthyNodes.size() < nodesRequired (where new check was added in the current change) before calling filterNodesWithSpace.
filterNodesWithSpace checking the condition and throwing the exception FAILED_TO_FIND_NODES_WITH_SPACE.
[ozone/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java](https://github.com/apache/ozone/blob/4454e6b368bc5aedaba509eeb85422daeda393bf/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java#L148-L149)
Test is failing with the following assert failure :
[ERROR] org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart Time elapsed: 55.337 s <<< FAILURE!
java.lang.AssertionError: expected:<FAILED_TO_FIND_SUITABLE_NODE> but was:<FAILED_TO_FIND_NODES_WITH_SPACE>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotEquals(Assert.java:835)
at org.junit.Assert.assertEquals(Assert.java:120)
at org.junit.Assert.assertEquals(Assert.java:146)
at org.apache.hadoop.hdds.scm.pipeline.TestRatisPipelineCreateAndDestroy.testPipelineCreationOnNodeRestart(TestRatisPipelineCreateAndDestroy.java:1
--
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] neils-dev commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r999937402
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
For the pipeline placement, the following seems to be expected when choosing nodes, first check healthy nodes, remove excluded and others, then check size and return finalized list. This is followed in the comments of `filterViableNodes `https://github.com/apache/ozone/blob/fc0854b391ddc7716215907f7915f4b3f88211d6/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java#L109 and in the `SCMCommonPlacementPolicy.chooseDatanodesInternal` and looks reasonable to follow here.
For the testcase,` testPipelineCreationOnNodeRestart()`, there are no datanodes, so the check for healthynodes should raise an exception **prior** to check for sufficient size raising the `FAILED_TO_FIND_NODES_WITH_SPACE` exception. The healthynode exception may actually be something other then `FAILED_TO_FIND_SUITABLE_NODE`, and perhaps `SCMException.ResultCodes.FAILED_TO_FIND_HEALTHY_NODES` to be consistent with the exception raised in `SCMCommonPlacementPolicy.chooseDatanodesInternal` ?
Something similar to,
```
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
if (healthyNodes.size() == 0) {
msg = "No healthy node found to allocate container.";
LOG.error(msg);
throw new SCMException(msg, SCMException.ResultCodes
.FAILED_TO_FIND_HEALTHY_NODES);
```
in `filterViableNodes`?
--
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] neils-dev commented on a diff in pull request #3847: HDDS-3419. Throw exception with correct code when healthy nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
neils-dev commented on code in PR #3847:
URL: https://github.com/apache/ozone/pull/3847#discussion_r999937402
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java:
##########
@@ -124,6 +124,15 @@ List<DatanodeDetails> filterViableNodes(
// get nodes in HEALTHY state
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
+ if (healthyNodes.size() < nodesRequired) {
Review Comment:
For the pipeline placement, the following seems to be expected when choosing nodes, first check healthy nodes, remove excluded and others, then check size and return finalized list. This is followed in the comments of `filterViableNodes `https://github.com/apache/ozone/blob/fc0854b391ddc7716215907f7915f4b3f88211d6/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java#L109 and in the `SCMCommonPlacementPolicy.chooseDatanodesInternal` and looks reasonable to follow here.
For the testcase,` testPipelineCreationOnNodeRestart()`, there are no datanodes, so the check for healthynodes should raise an exception **prior** to check for sufficient size raising the `FAILED_TO_FIND_NODES_WITH_SPACE` exception. The healthynode exception may actually be something other then `FAILED_TO_FIND_SUITABLE_NODE`, and perhaps `SCMException.ResultCodes.FAILED_TO_FIND_HEALTHY_NODES` to be consistent with the exception raised in `SCMCommonPlacementPolicy.chooseDatanodesInternal` ?
Something similar to in `filterViableNodes`,
```
List<DatanodeDetails> healthyNodes =
nodeManager.getNodes(NodeStatus.inServiceHealthy());
if (healthyNodes.size() == 0) {
msg = "No healthy node found to allocate container.";
LOG.error(msg);
throw new SCMException(msg, SCMException.ResultCodes
.FAILED_TO_FIND_HEALTHY_NODES);
```
?
--
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] pratapchandu closed pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
Posted by GitBox <gi...@apache.org>.
pratapchandu closed pull request #3847: HDDS-3419. Throw exception with correct code when available data nodes are not sufficient
URL: https://github.com/apache/ozone/pull/3847
--
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