HBASE-27857 Fix timeout exception handling in HBaseClassTestRule #5231
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HBaseClassTestRule applies a timeout and a system exit rule to tests. The timeout rule throws an exception if it hits the timeout threshold. Since the timeout rule is applied after the system exit rule, the system exit rule does not see the exception and does not re-enable the system exit behavior which can cause maven to hang on some tests. This change applies the timeout rule before the system exit rule so that normal system exit can be restored before the surefire forked node is shutdown. Signed-off-by: Jonathan Albrecht <jonathan.albrecht@ibm.com>
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Mind explain a bit more? IIRC, the SystemExitRule is to forbid System.exit call, so what is actual problem here? |
Yes, the SystemExitRule is supposed to forbid the System.exit call while a test is running so that the various test servers running in the surefire forked jvm don't cause it to exit before a test has finished IIUC. The SystemExitRule will restore the normal System.exit behaviour when the test completes. The problem is, if a test times out, the test doesn't actually complete. The forked jvm really does need to exit but the SystemExitRule hasn't restored System.exit so it can't. Note that the timeout exception originates in the timeout rule part of HBaseClassTestRule. The timeout rule actually runs the test method in a separate thread in a Future with a timeout value. The change in this PR applies the systemExitRule first so when the timeout rule throws an exception it will go through the try...finally block in the systemExitRule which restores the normal System.exit and the forked jvm can exit normally when it needs to. Without this change, you can see that org.apache.hadoop.hbase.TestSecurityManager shows up in the stacktrace if a test times out: |
Apache9
approved these changes
May 12, 2023
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
pushed a commit
that referenced
this pull request
May 13, 2023
) HBaseClassTestRule applies a timeout and a system exit rule to tests. The timeout rule throws an exception if it hits the timeout threshold. Since the timeout rule is applied after the system exit rule, the system exit rule does not see the exception and does not re-enable the system exit behavior which can cause maven to hang on some tests. This change applies the timeout rule before the system exit rule so that normal system exit can be restored before the surefire forked node is shutdown. Signed-off-by: Duo Zhang <zhangduo@apache.org> (cherry picked from commit 220eacf)
Apache9
pushed a commit
that referenced
this pull request
May 13, 2023
) HBaseClassTestRule applies a timeout and a system exit rule to tests. The timeout rule throws an exception if it hits the timeout threshold. Since the timeout rule is applied after the system exit rule, the system exit rule does not see the exception and does not re-enable the system exit behavior which can cause maven to hang on some tests. This change applies the timeout rule before the system exit rule so that normal system exit can be restored before the surefire forked node is shutdown. Signed-off-by: Duo Zhang <zhangduo@apache.org> (cherry picked from commit 220eacf)
Apache9
pushed a commit
that referenced
this pull request
May 13, 2023
) HBaseClassTestRule applies a timeout and a system exit rule to tests. The timeout rule throws an exception if it hits the timeout threshold. Since the timeout rule is applied after the system exit rule, the system exit rule does not see the exception and does not re-enable the system exit behavior which can cause maven to hang on some tests. This change applies the timeout rule before the system exit rule so that normal system exit can be restored before the surefire forked node is shutdown. Signed-off-by: Duo Zhang <zhangduo@apache.org> (cherry picked from commit 220eacf)
|
Thanks for reviewing and merging @Apache9! |
vinayakphegde
pushed a commit
to vinayakphegde/hbase
that referenced
this pull request
Apr 4, 2024
…ache#5231) HBaseClassTestRule applies a timeout and a system exit rule to tests. The timeout rule throws an exception if it hits the timeout threshold. Since the timeout rule is applied after the system exit rule, the system exit rule does not see the exception and does not re-enable the system exit behavior which can cause maven to hang on some tests. This change applies the timeout rule before the system exit rule so that normal system exit can be restored before the surefire forked node is shutdown. Signed-off-by: Duo Zhang <zhangduo@apache.org> (cherry picked from commit 220eacf) (cherry picked from commit 4df157d) Change-Id: Id051ff2cf38fcdd3ef47f7e8d9ec99a8de88b6c8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HBaseClassTestRule applies a timeout and a system exit rule to tests. The timeout rule throws an exception if it hits the timeout threshold. Since the timeout rule is applied after the system exit rule, the system exit rule does not see the exception and does not re-enable the system exit behavior which can cause maven to hang on some tests.
This change applies the timeout rule before the system exit rule so that normal system exit can be restored before the surefire forked node is shutdown.