[SPARK-6650] [core] Stop ExecutorAllocationManager when context stops. by vanzin · Pull Request #5311 · apache/spark

vanzin · 2015-04-01T17:02:29Z

This fixes the thread leak. I also changed the unit test to keep track
of allocated contexts and make sure they're closed after tests are
run; this is needed since some tests use this pattern:

val sc = createContext()
doSomethingThatMayThrow()
sc.stop()

This fixes the thread leak. I also changed the unit test to keep track of allocated contexts and making sure they're closed after tests are run; this is needed since some tests use this pattern: val sc = createContext() doSomethingThatMayThrow() sc.stop()

SparkQA · 2015-04-01T17:08:24Z

Test build #29548 has started for PR 5311 at commit 9886f69.

sryza · 2015-04-01T17:24:10Z

core/src/test/scala/org/apache/spark/ExecutorAllocationManagerSuite.scala

Will this ever have more than a single element?

Yes. See test("verify min/max executors").

SparkQA · 2015-04-01T17:43:20Z

Test build #29551 has started for PR 5311 at commit cc5a744.

SparkQA · 2015-04-01T18:32:54Z

Test build #29548 has finished for PR 5311 at commit 9886f69.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-01T18:32:58Z

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

vanzin · 2015-04-01T18:42:35Z

Jenkins retest this please.

SparkQA · 2015-04-01T18:48:23Z

Test build #29554 has started for PR 5311 at commit cc5a744.

SparkQA · 2015-04-01T19:29:29Z

Test build #29551 has finished for PR 5311 at commit cc5a744.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-01T19:29:33Z

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

SparkQA · 2015-04-01T19:52:02Z

Test build #29554 has finished for PR 5311 at commit cc5a744.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-01T19:52:07Z

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

srowen · 2015-04-01T20:15:01Z

I'm not sure why the test failed, but on inspection it LGTM.

sryza · 2015-04-01T21:05:33Z

core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala

nit: alphabetize

sryza · 2015-04-01T21:06:05Z

One last nit. Otherwise LGTM.

SparkQA · 2015-04-01T21:23:27Z

Test build #29564 has started for PR 5311 at commit 652c73b.

SparkQA · 2015-04-01T22:46:28Z

Test build #29564 has finished for PR 5311 at commit 652c73b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-01T22:46:33Z

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

andrewor14 · 2015-04-03T02:48:30Z

LGTM2, thanks for fixing this merging into master and 1.3.

This fixes the thread leak. I also changed the unit test to keep track of allocated contexts and make sure they're closed after tests are run; this is needed since some tests use this pattern: val sc = createContext() doSomethingThatMayThrow() sc.stop() Author: Marcelo Vanzin <vanzin@cloudera.com> Closes #5311 from vanzin/SPARK-6650 and squashes the following commits: 652c73b [Marcelo Vanzin] Nits. 5711512 [Marcelo Vanzin] More exception safety. cc5a744 [Marcelo Vanzin] Stop alloc manager before scheduler. 9886f69 [Marcelo Vanzin] [SPARK-6650] [core] Stop ExecutorAllocationManager when context stops. (cherry picked from commit 45134ec) Signed-off-by: Andrew Or <andrew@databricks.com> Conflicts: core/src/main/scala/org/apache/spark/SparkContext.scala

This fixes the thread leak. I also changed the unit test to keep track of allocated contexts and make sure they're closed after tests are run; this is needed since some tests use this pattern: val sc = createContext() doSomethingThatMayThrow() sc.stop() Author: Marcelo Vanzin <vanzin@cloudera.com> Closes apache#5311 from vanzin/SPARK-6650 and squashes the following commits: 652c73b [Marcelo Vanzin] Nits. 5711512 [Marcelo Vanzin] More exception safety. cc5a744 [Marcelo Vanzin] Stop alloc manager before scheduler. 9886f69 [Marcelo Vanzin] [SPARK-6650] [core] Stop ExecutorAllocationManager when context stops. (cherry picked from commit 45134ec) Conflicts: core/src/main/scala/org/apache/spark/SparkContext.scala

PRs Merged 1. [Internal] Add AppleAwsClientFactory for Mascot (apache#577) 2. Hive: Log new metadata location in commit (apache#4681) 3. change timeout to 120 for now (apache#661) 4. Internal: Add hive_catalog parameter to SparkCatalog (apache#670) 5. Internal: Pull catalog setting to CachedClientPool (apache#673) 6. Core: Defer reading Avro metadata until ManifestFile is read (apache#5206) 7. API: Fix ID assignment in schema merging (apache#5395) 8. AWS: S3OutputStream - failure to close should persist on subsequent close calls (apache#5311) 9. API: Allow schema updates to find fields with case-insensitivity (apache#5440) 10. Spark 3.3: Spark mergeSchema to respect Spark Case Sensitivity Configuration (apache#5441)

sryza reviewed Apr 1, 2015
View reviewed changes

Stop alloc manager before scheduler.

cc5a744

More exception safety.

5711512

sryza reviewed Apr 1, 2015
View reviewed changes

core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala Outdated

Copy link

Contributor

sryza Apr 1, 2015

Choose a reason for hiding this comment

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

nit: alphabetize

Nits.

652c73b

vanzin mentioned this pull request Apr 2, 2015

[SPARK-4194] [core] Make SparkContext initialization exception-safe. #5335

Closed

asfgit closed this in 45134ec Apr 3, 2015

vanzin deleted the SPARK-6650 branch April 3, 2015 18:46

Conversation

vanzin commented Apr 1, 2015

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

sryza Apr 1, 2015

Choose a reason for hiding this comment

Uh oh!

vanzin Apr 1, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

AmplabJenkins commented Apr 1, 2015

Uh oh!

vanzin commented Apr 1, 2015

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

AmplabJenkins commented Apr 1, 2015

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

AmplabJenkins commented Apr 1, 2015

Uh oh!

srowen commented Apr 1, 2015

Uh oh!

sryza Apr 1, 2015

Choose a reason for hiding this comment

Uh oh!

sryza commented Apr 1, 2015

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

SparkQA commented Apr 1, 2015

Uh oh!

AmplabJenkins commented Apr 1, 2015

Uh oh!

andrewor14 commented Apr 3, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants