-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HADOOP-19073 WASB: Fix connection leak in FolderRenamePending #6534
Conversation
@ayushtkn @slfan1989 PTAL |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
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.
+1.
Well, this may not belong to the HDFS module. |
@zhangshuyan0 Thank you for your review! but I think this PR needs Steve’s help to review it. @xuzifu666 Thanks for the contribution! we need to wait for the results of Steve's review. |
OK, @steveloughran could you give the final review? or @slfan1989 could you help to ping the commiters related to the pr to review it?Thanks ~ |
moved to hadoop; azure component. now, the bad news: as it goes near a cloud store, you have to run the entire hadoop-azure tests for the wasb component, and tell us which region you tested against. https://hadoop.apache.org/docs/stable/hadoop-azure/testing_azure.html sorry, but yetus doesn't have any credentials. It doesn't take long and is designed to clean up afterwards |
OK,anything else me to do for the pull request?@steveloughran |
@xuzifu666 yes.
|
@steveloughran Hi,seems network connection available,test is hard for me to execute,but the pr may be a obvious connection leak,could you give a help to confirm it?Thank any way~ |
💔 -1 overall
This message was automatically generated. |
|
@steveloughran could you give the final review or close the pr? Thanks |
let me try and test this myself. |
(which I can't do today as both my hadoop source trees are testing my "support parallel tests against the same s3 bucket" right now... |
OK,Thanks,currently it can be work now? @steveloughran |
I've been away for a week and am catching up, so not tested yet, sorry... |
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.
+1 merging anyway as it clearly does what we need and is very simple to review
@xuzifu666 before I merge, what name do you want to be credited with in the patch...your github account doesn't have one. |
thanks, merged to trunk and updated the JIRA. If you do a cherrypick of this commit and submit as a PR against branch-3.4 I'll merge it there too, once yetus is happy |
…#6534) Contributed by xuyu
…#6534) Contributed by xuyu
Description of PR
Currently connection leak in FolderRenamePending in getting bytes. the pr aim to fix it
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?