Skip to content
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-13279] Remove unnecessary duplicate check in addPendingTask fu… #11175

Closed
wants to merge 1 commit into from

Conversation

sitalkedia
Copy link

…nction

@kayousterhout
Copy link
Contributor

Jenkins this is ok to test

@kayousterhout
Copy link
Contributor

Jenkins, test this please

if (!list.contains(index)) {
list += index
}
list += index
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It strikes me that addTo is now only making a negative contribution toward understanding the code -- mostly because the "...if it's not already there" comment is now wrong. I don't see why the handful of uses of addTo shouldn't just be replaced with, e.g.:

pendingTasksForExecutor.getOrElseUpdate(e.executorId, new ArrayBuffer) += index

@srowen
Copy link
Member

srowen commented Feb 14, 2016

Jenkins, test this please

@SparkQA
Copy link

SparkQA commented Feb 14, 2016

Test build #51266 has finished for PR 11175 at commit 26b4370.

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

@srowen
Copy link
Member

srowen commented Feb 15, 2016

@kayousterhout I'll let you push the button on this one when you're sure but it looks OK to me.

@sitalkedia
Copy link
Author

@kayousterhout did you get some time to look into this?

asfgit pushed a commit that referenced this pull request Feb 17, 2016
This commit removes an unnecessary duplicate check in addPendingTask that meant
that scheduling a task set took time proportional to (# tasks)^2.

Author: Sital Kedia <skedia@fb.com>

Closes #11175 from sitalkedia/fix_stuck_driver.

(cherry picked from commit 1e1e31e)
Signed-off-by: Kay Ousterhout <kayousterhout@gmail.com>
@asfgit asfgit closed this in 1e1e31e Feb 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants