-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDDS-1395. Key write fails with BlockOutputStream has been closed exception #749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
Thanks for the patch Shashi, this patch has some unit test failure related to the patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the changes for BlockOutputStreamEntryPool.java, be done as part of a different pull request. The current patch becomes difficult to review because of the number of changes in the patch.
|
Thanks Mukul. It is difficult to divide the patch in multiple patches as it reorganizes KeyOutputStream. I will look into the unit test failures. |
|
There is a unit test failure where a watch request is not failing with GroupMismatchException even after the pipeline gets removed but fails with Timeout Exception which i need to check further. But it would really help if we have it as a single patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This need not be public. If you need it for testing, please annotate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be pulic because this function gets invoked from BlockoutputStreamEntryPool#isClosed() and they are in different packages.
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, patch looks good to me, only some minor comments.
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/CommitWatcher.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not remove the streamEntry here. It will be easier for debugging, or code readability. The empty entries can be pruned at the time of reporting to OM, and also logged that certain block was not written at all.
|
💔 -1 overall
This message was automatically generated. |
|
There is a related test failure with a test added with the change which requires RATIS-532. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
/retest |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
+1 for the patch, if the test failures are unrelated. |
|
/retest |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
/retest |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…eption (#749). Contributed by Shashikant Banerjee
|
The 1 test failure reported is not related to the patch. |
- Fix titles for blog posts - Fix a rendering issue with hyperlinks on the front-page - Add links from homepage sections to relevant documentation pages Author: Jagadish <[email protected]> Reviewers: Jagadish<[email protected]> Closes apache#749 from vjagadish1989/website-reorg21
No description provided.