-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-31831][SQL][TESTS] HiveSessionImplSuite flakiness fix via mocking instances earlier than initializing HiveSessionImpl #29039
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
…than initializing HiveSessionImpl
|
Rationalization: the failures always come from classloader issue; while I don't have clear idea what is happening, there's no part possibly dealing with classloader, except initializing It's less flaky then CliSuite and only checked once per build, so to verify multiple times I just create 9 duplicated suites to test 10 times per build. |
|
retest this, please |
2 similar comments
|
retest this, please |
|
retest this, please |
|
Test build #125365 has finished for PR 29039 at commit
|
|
Test build #125363 has finished for PR 29039 at commit
|
|
retest this, please |
|
Test build #125371 has started for PR 29039 at commit |
|
retest this, please |
|
Test build #125375 has started for PR 29039 at commit |
|
Test build #125361 has finished for PR 29039 at commit
|
|
Test build #125368 has finished for PR 29039 at commit
|
|
Summary of 4 builds:
HiveSessionImplSuite(s) passed
HiveSessionImplSuite(s) passed
HiveSessionImplSuite(s) passed
HiveSessionImplSuite(s) passed I'll trigger more builds to confirm, and once the new builds also pass HiveSessionImplSuite(s), turn the PR to be ready to review. |
|
retest this, please |
6 similar comments
|
retest this, please |
|
retest this, please |
|
retest this, please |
|
retest this, please |
|
retest this, please |
|
retest this, please |
|
Test build #125405 has finished for PR 29039 at commit
|
|
Test build #125410 has finished for PR 29039 at commit
|
|
Test build #125416 has finished for PR 29039 at commit
|
|
Test build #125412 has finished for PR 29039 at commit
|
|
Test build #125420 has finished for PR 29039 at commit
|
|
Another summary for next set of builds
All suites passed
Lots of tests in CliSuite failed - #29036 is to fix flakiness, so please cross-check this and #29036 together
All suites passed
Lots of tests in CliSuite failed
Lots of tests in CliSuite failed |
This reverts commit 9c48467.
srowen
left a comment
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.
Likewise looks good if it passes and fixes the issue.
|
Test build #125473 has finished for PR 29039 at commit
|
|
Test build #125475 has finished for PR 29039 at commit
|
|
Thank you for pinging me, @HeartSaVioR . |
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, LGTM. Thank you, @HeartSaVioR and @srowen .
Merged to master because the affected version of SPARK-31831 is only 3.1.0.
|
Thanks for reviewing and merging! We can port back anytime when we find the flakiness in other branches, so it should be OK to start with only master branch. |
What changes were proposed in this pull request?
This patch changes the HiveSessionImplSuite to mock instances "before" initializing HiveSessionImpl, to avoid possible classloader issue.
Why are the changes needed?
The failures of HiveSessionImplSuite always come from classloader issue. While I don't have clear idea what is happening, there's no part possibly dealing with classloader, except initializing HiveSessionImpl. We can move the mock initializations earlier than initialing HiveSessionImpl so that it can avoid possible classloader issue.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Verified with multiple triggers of Jenkins builds