[SPARK-23881][CORE][TEST] Fix flaky test JobCancellationSuite."interruptible iterator of shuffle reader" #20993
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
|
cc @cloud-fan @advancedxy @dongjoon-hyun PTAL |
cloud-fan
reviewed
Apr 6, 2018
| // 2. task in reduce stage is blocked as taskCancelledSemaphore is not released until | ||
| // JobCancelled event is posted. | ||
| // After job being cancelled, task in reduce stage will be cancelled asynchronously, thus | ||
| // partial of the inputs should not get processed. |
There was a problem hiding this comment.
thus partial of the inputs should not get processed. ->
It's very unlikely that Spark can process 10000 elements between JobCancelled is posted and task is really killed.
|
LGTM |
|
Test build #88973 has finished for PR 20993 at commit
|
|
@jiangxb1987 can you post a Jenkins job link referring the flaky test? |
|
They're in the JIRA task description
Xianjin YE <[email protected]>于2018年4月6日 周五下午8:58写道：
… @jiangxb1987 <https://github.com/jiangxb1987> can you post a Jenkins job
link referring the flaky test?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20993 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEkCjoG-VU-0NyllJGGMgGIw7h_ay_Lvks5tl2ZkgaJpZM4TJoBn>
.
|
advancedxy
approved these changes
Apr 6, 2018
| // JobCancelled event is posted. | ||
| // After job being cancelled, task in reduce stage will be cancelled asynchronously, thus | ||
| // partial of the inputs should not get processed (It's very unlikely that Spark can process | ||
| // 10000 elements between JobCancelled is posted and task is really killed). |
There was a problem hiding this comment.
I re-checked the killTask code. I believe there's still possibility(very unlikely) that the reduce task processes all the input elements before task is really killed, then we cannot observe the reduce task being interruptible.
One way to reduce possibility would be increasing the num of input elements. So I believe we should add comments in val numElements = 10000 to make laters know that we choose 10000 for a reason.
|
Test build #88987 has finished for PR 20993 at commit
|
|
Test build #89060 has finished for PR 20993 at commit
|
|
The latest commit is just to add new comments in the code. The previous test already passed. Merged to master/2.3 |
asfgit
pushed a commit
that referenced
this pull request
Apr 9, 2018
…uptible iterator of shuffle reader" ## What changes were proposed in this pull request? The test case JobCancellationSuite."interruptible iterator of shuffle reader" has been flaky because `KillTask` event is handled asynchronously, so it can happen that the semaphore is released but the task is still running. Actually we only have to check if the total number of processed elements is less than the input elements number, so we know the task get cancelled. ## How was this patch tested? The new test case still fails without the purposed patch, and succeeded in current master. Author: Xingbo Jiang <[email protected]> Closes #20993 from jiangxb1987/JobCancellationSuite. (cherry picked from commit d81f29e) Signed-off-by: gatorsmile <[email protected]>
peter-toth
pushed a commit
to peter-toth/spark
that referenced
this pull request
Oct 6, 2018
…uptible iterator of shuffle reader" The test case JobCancellationSuite."interruptible iterator of shuffle reader" has been flaky because `KillTask` event is handled asynchronously, so it can happen that the semaphore is released but the task is still running. Actually we only have to check if the total number of processed elements is less than the input elements number, so we know the task get cancelled. The new test case still fails without the purposed patch, and succeeded in current master. Author: Xingbo Jiang <[email protected]> Closes apache#20993 from jiangxb1987/JobCancellationSuite. (cherry picked from commit d81f29e) Signed-off-by: gatorsmile <[email protected]> Change-Id: I595412f8b6e93223ebf780e30f986c7d0fa99c6e
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?
The test case JobCancellationSuite."interruptible iterator of shuffle reader" has been flaky because
KillTaskevent is handled asynchronously, so it can happen that the semaphore is released but the task is still running.Actually we only have to check if the total number of processed elements is less than the input elements number, so we know the task get cancelled.
How was this patch tested?
The new test case still fails without the purposed patch, and succeeded in current master.