[SPARK-4754] Refactor SparkContext into ExecutorAllocationClient

[andrewor14]

This is such that the ExecutorAllocationManager does not take in the SparkContext with all of its dependencies as an argument. This prevents future developers of this class to tie down this class further with the SparkContext, which has really become quite a monstrous object.

cc'ing @pwendell who originally suggested this, and @JoshRosen who may have thoughts about the trait mix-in style of SparkContext.

[SparkQA]

Test build #24168 has started for PR 3614 at commit 347a348.

• This patch merges cleanly.

[SparkQA]

Test build #24168 has finished for PR 3614 at commit 347a348.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class SparkContext(config: SparkConf) extends Logging with ExecutorAllocationClient

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24168/
Test PASSed.

[SparkQA]

Test build #24286 has started for PR 3614 at commit 59baf6c.

• This patch merges cleanly.

[SparkQA]

Test build #24286 has finished for PR 3614 at commit 59baf6c.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class SparkContext(config: SparkConf) extends Logging with ExecutorAllocationClient

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24286/
Test PASSed.

[SparkQA]

Test build #24329 has started for PR 3614 at commit 187070d.

• This patch merges cleanly.

[JoshRosen]

+1; this looks like a good refactoring. I like how the new ExecutorAllocationManager constructor makes its dependencies more explicit, which should help us to avoid an entire category of "implicit initialization order dependencies" bugs that we've hit in the past with classes that accept partially-constructed SparkContexts in their constructors.

I think the mixin trait shouldn't be an issue here; I think that the problems that I ran into with this pattern were related to the closure cleaner and additional levels of $outer indirection, but that shouldn't be an issue here.

[JoshRosen]

Actually, one comment here: it might be nice to document the meaning of the return type. It looks like the scaladoc in CoarseGrainedSchedulerBackend already does this.

[andrewor14]

yeah I left that out accidentally

[SparkQA]

Test build #24329 timed out for PR 3614 at commit 187070d after a configured wait of 120m.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24329/
Test FAILed.

[andrewor14]

retest this please

[SparkQA]

Test build #24373 has started for PR 3614 at commit 187070d.

• This patch merges cleanly.

[SparkQA]

Test build #24373 has finished for PR 3614 at commit 187070d.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class SparkContext(config: SparkConf) extends Logging with ExecutorAllocationClient

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24373/
Test PASSed.

[ksakellis]

Just curious here, but is the eventual goal to pull this functionality(adding/removing executors) out of the SparkContext object?

[andrewor14]

No, this is literally just refactoring for better code style

[JoshRosen]

@andrewor14 This looks good to me. Any blockers to merging this?

[andrewor14]

Nope, I'll merge this into master and 1.2 now thanks (after fixing the comments)