-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[SPARK-20407][TESTS] ParquetQuerySuite 'Enabling/disabling ignoreCorruptFiles' flaky test #17701
Conversation
Test build #75983 has started for PR 17701 at commit |
Do you know any Jenkins build where this test was failed? |
Fails rarely in our own jenkins. The test I added is just part of an existing test that is run 100 times, that one fails reliably. |
* this is part of test 'Enabling/disabling ignoreCorruptFiles' but run in a loop | ||
* to increase the chance of failure | ||
*/ | ||
test("SPARK-20407 ParquetQuerySuite 'Enabling/disabling ignoreCorruptFiles' flaky test") { |
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 might take a while to complete. Shall we ignore this? It is still useful to have for reference though.
DebugFilesystem.assertNoOpenStreams() | ||
// files can be closed from other threads, so wait a bit | ||
// normally this doesn't take more than 1s | ||
eventually(timeout(10.seconds)) { |
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 is a catch all for when we are not using withTempDir
right?
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 is so that DebugFilesystem.assertNoOpenStreams has a better chance of working. And in general, whatever else runs at the end of the test. For now it's just assertNoOpenStreams
A few minor comments, otherwise LGTM. |
Test build #75991 has finished for PR 17701 at commit
|
…uptFiles' flaky test ## What changes were proposed in this pull request? SharedSQLContext.afterEach now calls DebugFilesystem.assertNoOpenStreams inside eventually. SQLTestUtils withTempDir calls waitForTasksToFinish before deleting the directory. ## How was this patch tested? Added new test in ParquetQuerySuite based on the flaky test Author: Bogdan Raducanu <bogdan@databricks.com> Closes #17701 from bogdanrdc/SPARK-20407. (cherry picked from commit c5a31d1) Signed-off-by: Herman van Hovell <hvanhovell@databricks.com>
Merging to master/branch-2.2. Thanks! |
…uptFiles' flaky test ## What changes were proposed in this pull request? SharedSQLContext.afterEach now calls DebugFilesystem.assertNoOpenStreams inside eventually. SQLTestUtils withTempDir calls waitForTasksToFinish before deleting the directory. ## How was this patch tested? Added new test in ParquetQuerySuite based on the flaky test Author: Bogdan Raducanu <bogdan@databricks.com> Closes apache#17701 from bogdanrdc/SPARK-20407.
What changes were proposed in this pull request?
SharedSQLContext.afterEach now calls DebugFilesystem.assertNoOpenStreams inside eventually.
SQLTestUtils withTempDir calls waitForTasksToFinish before deleting the directory.
How was this patch tested?
Added new test in ParquetQuerySuite based on the flaky test