Skip to content

HDDS-4873. Re-replication failure throws NPE#1986

Merged
cku328 merged 3 commits intoapache:masterfrom
sky76093016:HDDS-4873
Mar 5, 2021
Merged

HDDS-4873. Re-replication failure throws NPE#1986
cku328 merged 3 commits intoapache:masterfrom
sky76093016:HDDS-4873

Conversation

@sky76093016
Copy link
Contributor

What changes were proposed in this pull request?

Increase the log that helps users to use.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4873

How was this patch tested?

No test.

@sky76093016
Copy link
Contributor Author

@jojochuang Can you review this PR for me?Thank you.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sky76093016 for working on this.

We should not simply log "please retry one more time", as container replication is performed by datanodes automatically. It will be retried eventually when datanode receives the replication command from SCM again.

If download fails currently the code reaches the exception handler with NPE. The only problem with this is that it's noisy, and we could handle the condition more gracefully. I think if tempTarFile == null, it should simply set task status to failed, and skip the code in the try block.

@jojochuang
Copy link
Contributor

what Attila said. Thanks for the review!

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sky76093016 for updating the patch.

Copy link
Contributor

@cku328 cku328 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @sky76093016 for working on this.

@cku328 cku328 merged commit 0bdbadd into apache:master Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants