[SPARK-39622][SQL][TESTS] Add additional error message matching for SPARK-7837 Do not close output writer twice when commitTask() fail
#37245
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
LuciferYang
changed the title
SPARK-7837 Do not close output writer twice when commitTask() fail in ParquetIOSuiteSPARK-7837 Do not close output writer twice when commitTask() fail in ParquetIOSuite
|
cc @HyukjinKwon |
|
also cc @srowen @dongjoon-hyun |
|
c97a71a merge SPARK-39831 to pass GA |
dongjoon-hyun
approved these changes
Jul 21, 2022
LuciferYang
changed the title
SPARK-7837 Do not close output writer twice when commitTask() fail in ParquetIOSuiteSPARK-7837 Do not close output writer twice when commitTask() fail
|
Change the title to Add additional error message matching for @dongjoon-hyun Do you think this is OK? |
|
Thank you. Looks much better! |
|
|
|
This test code changes add |
|
Thanks @LuciferYang for dealing with this! |
|
Thanks all ~ |
HyukjinKwon
pushed a commit
that referenced
this pull request
Jul 27, 2022
…oot cause ### What changes were proposed in this pull request? The pr follow #37245 StageFailed event should attach with the root cause ### Why are the changes needed? **It may be a good way for users to know the reason of failure.** By carefully investigating the issue: https://issues.apache.org/jira/browse/SPARK-39622, I found the root cause of test failure: StageFailed don't attach the failed reason from executor. when OutputCommitCoordinator execute 'taskCompleted', the 'reason' is ignored. Scenario 1: receive TaskSetFailed (Success) > InsertIntoHadoopFsRelationCommand > FileFormatWriter.write > _**handleTaskSetFailed**_ (**attach root cause**) > abortStage > failJobAndIndependentStages > SparkListenerJobEnd Scenario 1: receive StageFailed (Fail) > InsertIntoHadoopFsRelationCommand > FileFormatWriter.write > _**handleStageFailed**_ (**don't attach root cause**) > abortStage > failJobAndIndependentStages > SparkListenerJobEnd ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manual run UT & Pass GitHub Actions Closes #37292 from panbingkun/SPARK-39868. Authored-by: panbingkun <pbk1982@gmail.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
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.
What changes were proposed in this pull request?
This PR adds additional assertions to fix flaky test
SPARK-7837 Do not close output writer twice when commitTask() failinParquetIOSuite.Why are the changes needed?
The test suite
SPARK-7837 Do not close output writer twice when commitTask() failsonly handle theTaskSetFailedevent before SPARK-39195 due tomaxTaskFailuresis 1 when local mode.But after SPARK-39195, In
OutputCommitCoordinator#taskCompleted, the processing ofstageState.authorizedCommitters(partition) == taskIdchanges from debug logging to post anStageFailedevent, the flaky test may handle to one of theTaskSetFailedevent andStageFailedevent, and the execution order of the two events is uncertain, and there may be the following two kinds of logs:- Scenario 1(Success)
- Scenario 2(Failed)
If
maxTaskFailuresis changed to>=2, Scenario 2 will appear stably, but the flaky test runs in the local mode andmaxTaskFailuresis always 1, so the this pr just adds additional assertions to workaround.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass GitHub Actions