Skip to content

Conversation

@liupc
Copy link

@liupc liupc commented Jun 1, 2017

What changes were proposed in this pull request?

This pull request fix the TaskScheulerImpl bug in some condition.
Detail see:
https://issues.apache.org/jira/browse/SPARK-20945

(Please fill in changes proposed in this fix)

How was this patch tested?

manual tests
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

@JoshRosen
Copy link
Contributor

Hi @liupc,

Thanks for the JIRA and PR. Could you edit the PR's title to incorporate the JIRA number? e.g.

[SPARK-20945] Fix TID key not found in TaskSchedulerImpl

Also, could you re-target this PR to be against the master branch? It looks like you've opened it against branch-2.1. Our workflow for these kind of bugs is generally to open PRs against the master branch: committers will take care of backporting your fix to appropriate maintenance branches.

@liupc liupc changed the base branch from branch-2.1 to master June 1, 2017 04:33
@liupc liupc force-pushed the Fix-tid-key-not-found-in-TaskSchedulerImpl branch from d6831fb to 1e35879 Compare June 1, 2017 04:40
@liupc liupc changed the title Fix tid key not found in TaskSchedulerImpl [SPARK-20945] Fix TID key not found in TaskSchedulerImpl Jun 1, 2017
backend.killTask(tid, execId, interruptThread, reason = "stage cancelled")
if (taskIdToExecutorId.contains(tid)) {
val execId = taskIdToExecutorId(tid)
backend.killTask(tid, execId, interruptThread)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you lost the value of "reason" here.
It's not a big deal but you could also write:

taskIdToExecutorId.get(tid).foreach(execId => backend.killTask(tid, execId, interruptThread, reason = "stage cancelled"))

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I will update soon, thanks!

@SparkQA
Copy link

SparkQA commented Jun 3, 2017

Test build #3775 has finished for PR 18171 at commit c4fff63.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Jun 5, 2017

Merged to master

@asfgit asfgit closed this in 2d39711 Jun 5, 2017
@jinxing64
Copy link

Why this is not merged into 2.2 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants