-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19250. [Addendum] Fix test TestServiceInterruptHandling.testRegisterAndRaise. #7008
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-19250. [Addendum] Fix test TestServiceInterruptHandling.testRegisterAndRaise. #7008
Conversation
…ommit-run Centos 7
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@steveloughran Hi, can you please review this PR? |
💔 -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, but I'd like the followup to not include cutting a line for no obvious reason
...n-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/erasurecode/erasure_coder.c
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
merged and updated the jira to make clear both PRs should be cherrypicked. do you want to do a PR chain which does that for branch-3.4? |
…gisterAndRaise. (apache#7008) Contributed by Chenyu Zheng
@steveloughran OK, I have create #7020 to branch-3.4 |
…gisterAndRaise. (apache#7008) Contributed by Chenyu Zheng
…gisterAndRaise. (apache#7008) Contributed by Chenyu Zheng
Description of PR
I submitted #6987 before, but in that PR, because the native code of hadoop common was not changed, "precommit-run Centos 7" was not triggered. Therefore, this unit test may still fail in some PRs. So I debugged in #6972 and solved this problem.
I found SIGINT was ignored before executing TestServiceInterruptHandling::testRegisterAndRaise. In the java code, for SIGHUP, SIGINT, and SIGTERM, if its SignalHandler have been set to SIG_IGN, any further actions to set SignalHandler will be invalid. For details, see jdk signal
In fact, it is difficult for me to reproduce this bug on my computer. But if I add
Signal.handle(new Signal(IrqHandler.CONTROL_C), SignalHandler.SIG_IGN);
in front ofirqHandler.bind();
, it can be reproduced stably, which also proves that what I said before is correct.In order to further prove this problem on docker ci, I submitted commit, then will find the following error like this:
It also proves that SIGINT was ignored.
Since SIGINT may be ignored, let's use USR2.
How was this patch tested?
docker ci
For code changes: