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 2021/02/07 16:38:09 UTC
[GitHub] [ozone] lamber-ken opened a new pull request #1904: HDDS-4802. Fix an internal variable always not null in PipelinePlacementPolicy
lamber-ken opened a new pull request #1904:
URL: https://github.com/apache/ozone/pull/1904
## What changes were proposed in this pull request?
`pick` always not null. `fallBackPickNodes` will throw exception if unable to find fall back node.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4802
## How was this patch tested?
No need new 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.
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] ayushtkn commented on pull request #1904: HDDS-4802. Fix an internal variable always not null in PipelinePlacementPolicy
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on pull request #1904:
URL: https://github.com/apache/ozone/pull/1904#issuecomment-776490718
The test is asserting the exception also which will not happen now
```
catch (SCMException e) {
Assert.assertEquals(SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE,
e.getResult());
}
```
This we can remove now, since it won't hit post this change.
In `testFallBackPickNodes` also we can get away from catching the SCMException, since that won't be thrown now.
----------------------------------------------------------------
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:
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] elek merged pull request #1904: HDDS-4802. Fix an internal variable always not null in PipelinePlacementPolicy
Posted by GitBox <gi...@apache.org>.
elek merged pull request #1904:
URL: https://github.com/apache/ozone/pull/1904
--
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:
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] lamber-ken edited a comment on pull request #1904: HDDS-4802. Fix an internal variable always not null in PipelinePlacementPolicy
Posted by GitBox <gi...@apache.org>.
lamber-ken edited a comment on pull request #1904:
URL: https://github.com/apache/ozone/pull/1904#issuecomment-776410614
> It is checking for the exception and error message from exception in other case, which it won't hit post this change.
Hi @ayushtkn:
The exception is from following logic, the logic has been moved to PipelinePlacementPolicy#getResultSet.
So in TestPipelineUpdataPolicy#testFallBackPickNodes, already use `Assert.assertNotNull(node);` to check.
If my understanding is wrong, please correct me, thanks
```
if (node == null) {
String msg = String.format("Unable to find fall back node in" +
" pipeline allocation. nodeSet size: %d", nodeSet.size());
LOG.warn(msg);
throw new SCMException(msg,
SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
}
```
----------------------------------------------------------------
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:
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] lamber-ken edited a comment on pull request #1904: HDDS-4802. Fix an internal variable always not null in PipelinePlacementPolicy
Posted by GitBox <gi...@apache.org>.
lamber-ken edited a comment on pull request #1904:
URL: https://github.com/apache/ozone/pull/1904#issuecomment-776410614
> It is checking for the exception and error message from exception in other case, which it won't hit post this change.
Hi @ayushtkn:
The exception is from following logic, the logic has been moved to PipelinePlacementPolicy#getResultSet.
So in TestPipelineUpdataPolicy#testFallBackPickNodes, already use `Assert.assertNotNull(node);` to check.
If my understanding is wrong, please correct me, thanks
```
if (node == null) {
String msg = String.format("Unable to find fall back node in" +
" pipeline allocation. nodeSet size: %d", nodeSet.size());
LOG.warn(msg);
throw new SCMException(msg,
SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
}
```
----------------------------------------------------------------
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:
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] lamber-ken commented on pull request #1904: HDDS-4802. Fix an internal variable always not null in PipelinePlacementPolicy
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1904:
URL: https://github.com/apache/ozone/pull/1904#issuecomment-776404162
> Makes sense to me, the other chooseNode methods seems to be returning null only when the can't find a datanode.
> We need to update TestPipelineUpdataPolicy#testFallBackPickNodes too.
> It is checking for the exception and error message from exception in other case, which it won't hit post this change.
OK，thanks for review，will check TestPipelineUpdataPolicy#testFallBackPickNodes.
----------------------------------------------------------------
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:
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] lamber-ken commented on pull request #1904: HDDS-4802. Fix an internal variable always not null in PipelinePlacementPolicy
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1904:
URL: https://github.com/apache/ozone/pull/1904#issuecomment-776410614
> It is checking for the exception and error message from exception in other case, which it won't hit post this change.
The exception is from following logic, the logic has been moved to PipelinePlacementPolicy#getResultSet.
So in TestPipelineUpdataPolicy#testFallBackPickNodes, already use `Assert.assertNotNull(node);` to check.
If my understanding is wrong, please correct me, thanks
```
if (node == null) {
String msg = String.format("Unable to find fall back node in" +
" pipeline allocation. nodeSet size: %d", nodeSet.size());
LOG.warn(msg);
throw new SCMException(msg,
SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org