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/08/17 19:19:19 UTC
[GitHub] [ozone] aswinshakil opened a new pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
aswinshakil opened a new pull request #2548:
URL: https://github.com/apache/ozone/pull/2548
## What changes were proposed in this pull request?
To fix the NullPointerException when markContainerForClose method is invoked. The exception is caused when we access the getContainerState method for a container object which is null.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5611
## How was this patch tested?
The patch was tested with Unit tests and GitHub CI test.
https://github.com/aswinshakil/ozone/runs/3345654012
The integration test TestPipelineClose.testPipelineCloseWithLogFailure is failing due to some issue with flaky tests. Attached the jira for that issue here.
https://issues.apache.org/jira/browse/HDDS-5626
--
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] ChenSammi commented on pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#issuecomment-900821957
Thanks @aswinshakil for report the issue and fix it. I left several comments inline.
--
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 change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r692493595
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
Review comment:
Thank you, I will update them.
--
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 change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r692493295
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
+ Set<Long> missingContainerSet = containerSet.getMissingContainerSet();
+ if (missingContainerSet.contains(containerId)) {
+ LOG.warn("The Container is in the MissingContainerSet " +
+ "hence we can't close it. ContainerID: {}", containerId);
+ } else {
+ LOG.warn("The Container is not found. ContainerID: {}", containerId);
+ }
+ } else {
+ if (container.getContainerState() == State.OPEN) {
+ getHandler(container).markContainerForClose(container);
+ }
Review comment:
Thank you for your input. I have place Log here so that we could have a better understanding of whether the container is present in the MissingContainerSet or not which allows us to debug better. @avijayanhwx your thoughts on that?
--
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] ChenSammi commented on pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#issuecomment-900821957
Thanks @aswinshakil for report the issue and fix it. I left several comments inline.
--
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] ChenSammi commented on a change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r690911646
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -44,6 +47,8 @@
private final ContainerSet containerSet;
private final Map<ContainerType, Handler> handlers;
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerController.class);
Review comment:
The indent of wrapped line is 4 space instead of 8. This is a general code style of ozone. Kindly also check the other part of the code which involes wrapped lines.
--
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] avijayanhwx commented on pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#issuecomment-906634281
Acceptance (misc) failure seems unrelated.
--
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] avijayanhwx merged pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #2548:
URL: https://github.com/apache/ozone/pull/2548
--
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] ChenSammi commented on a change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r690929859
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
+ Set<Long> missingContainerSet = containerSet.getMissingContainerSet();
+ if (missingContainerSet.contains(containerId)) {
+ LOG.warn("The Container is in the MissingContainerSet " +
+ "hence we can't close it. ContainerID: {}", containerId);
+ } else {
+ LOG.warn("The Container is not found. ContainerID: {}", containerId);
+ }
+ } else {
+ if (container.getContainerState() == State.OPEN) {
+ getHandler(container).markContainerForClose(container);
+ }
Review comment:
This is an usually case that container is not found. I would suggest throw an container not found storage exception here, the outer has the logic to handle the exception.
--
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] ChenSammi commented on a change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r690911646
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -44,6 +47,8 @@
private final ContainerSet containerSet;
private final Map<ContainerType, Handler> handlers;
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerController.class);
Review comment:
The indent of wrapped line is 4 space instead of 8. This is a general code style of ozone.
--
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] ChenSammi commented on a change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r690911952
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
Review comment:
Please leave a space before and after the == operation. This is also a general code style in Ozone.
--
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] ChenSammi commented on a change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r690911646
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -44,6 +47,8 @@
private final ContainerSet containerSet;
private final Map<ContainerType, Handler> handlers;
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerController.class);
Review comment:
The indent of wrapped line is 4 space instead of 8. This is a general code style of ozone.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
Review comment:
Please leave a space before and after the == operation. This is also a general code style in Ozone.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -44,6 +47,8 @@
private final ContainerSet containerSet;
private final Map<ContainerType, Handler> handlers;
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerController.class);
Review comment:
The indent of wrapped line is 4 space instead of 8. This is a general code style of ozone. Kindly also check the other part of the code which involes wrapped lines.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
Review comment:
Please leave a space before and after the operator, such as ==, + , - , etc. This is also a general code style in Ozone.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
+ Set<Long> missingContainerSet = containerSet.getMissingContainerSet();
+ if (missingContainerSet.contains(containerId)) {
+ LOG.warn("The Container is in the MissingContainerSet " +
+ "hence we can't close it. ContainerID: {}", containerId);
+ } else {
+ LOG.warn("The Container is not found. ContainerID: {}", containerId);
+ }
+ } else {
+ if (container.getContainerState() == State.OPEN) {
+ getHandler(container).markContainerForClose(container);
+ }
Review comment:
This is an usually case that container is not found. I would suggest throw an container not found storage exception here, the outer has the logic to handle the exception.
--
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] ChenSammi commented on a change in pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#discussion_r690911952
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java
##########
@@ -79,9 +84,18 @@ public String getContainerLocation(final long containerId) {
public void markContainerForClose(final long containerId)
throws IOException {
Container container = containerSet.getContainer(containerId);
-
- if (container.getContainerState() == State.OPEN) {
- getHandler(container).markContainerForClose(container);
+ if (container==null) {
Review comment:
Please leave a space before and after the operator, such as ==, + , - , etc. This is also a general code style in Ozone.
--
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] avijayanhwx commented on pull request #2548: HDDS-5611. Fixed NullPointerException in ContainerStateMachine during Pipeline Close.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #2548:
URL: https://github.com/apache/ozone/pull/2548#issuecomment-906634678
Thanks for the fix @aswinshakil and the review @ChenSammi.
--
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