[SPARK-4883][Shuffle] Add a name to the directoryCleaner thread #3734

zsxwing · 2014-12-18T09:32:57Z

No description provided.

SparkQA · 2014-12-18T09:37:28Z

Test build #24586 has started for PR 3734 at commit 71156d6.

This patch merges cleanly.

SparkQA · 2014-12-18T11:01:33Z

Test build #24586 has finished for PR 3734 at commit 71156d6.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- trait ParquetTest
- protected class CaseInsensitiveMap(map: Map[String, String]) extends Map[String, String]
- class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener

AmplabJenkins · 2014-12-18T11:01:37Z

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

aarondav · 2014-12-19T01:30:54Z

network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockManager.java

@@ -60,8 +61,11 @@
  private final TransportConf conf;

  public ExternalShuffleBlockManager(TransportConf conf) {
-    // TODO: Give this thread a name.
-    this(conf, Executors.newSingleThreadExecutor());
+    this(conf, Executors.newSingleThreadExecutor(


Maybe use NettyUtils.createThreadFactory() (could move that to JavaUtils too, less important).

Should it be daemon or not? If it's daemon, it may not have enough time to delete all files when the service is exiting.

Actually, this should be daemon, I think, otherwise it may prevent the JVM from exiting even when it's not doing anything. If we wanted to prevent the service from exiting before cleanup has completed, I would rather block in a stop() method until the queue of directories to delete has been drained, but I do not think this too important. Unlike Spark Executors, the graceful termination of this service should be uncorrelated with buildup of shuffle data.

Agree. Use NettyUtils.createThreadFactor now

SparkQA · 2014-12-19T07:42:29Z

Test build #24634 has started for PR 3734 at commit cc74727.

This patch merges cleanly.

SparkQA · 2014-12-19T09:05:14Z

Test build #24634 has finished for PR 3734 at commit cc74727.

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

AmplabJenkins · 2014-12-19T09:05:18Z

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

aarondav · 2014-12-19T18:51:16Z

network/shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockManager.java

-    this(conf, Executors.newSingleThreadExecutor());
+    this(conf, Executors.newSingleThreadExecutor(
+        // Add `spark` prefix because it will run in NM in Yarn mode.
+        NettyUtils.createThreadFactory("spark-shuffle-directory-clean")));


super nit: "spark-shuffle-directory-cleaner" may be a slightly better name

aarondav · 2014-12-19T18:52:22Z

LGTM, just want to add an "-er" suffix to the name. Thanks for doing this!

SparkQA · 2014-12-20T13:12:26Z

Test build #24669 has started for PR 3734 at commit e6f2b61.

This patch merges cleanly.

zsxwing · 2014-12-20T14:21:24Z

Done

SparkQA · 2014-12-20T14:36:03Z

Test build #24669 has finished for PR 3734 at commit e6f2b61.

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

AmplabJenkins · 2014-12-20T14:36:07Z

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

andrewor14 · 2014-12-22T20:11:23Z

Ok looks good to me too. I'm merging this into master and 1.2 thanks!

Author: zsxwing <zsxwing@gmail.com> Closes #3734 from zsxwing/SPARK-4883 and squashes the following commits: e6f2b61 [zsxwing] Fix the name cc74727 [zsxwing] Add a name to the directoryCleaner thread (cherry picked from commit 8773705) Signed-off-by: Andrew Or <andrew@databricks.com>

aarondav reviewed Dec 19, 2014
View reviewed changes

Add a name to the directoryCleaner thread

cc74727

aarondav reviewed Dec 19, 2014
View reviewed changes

Fix the name

e6f2b61

asfgit closed this in 8773705 Dec 22, 2014

zsxwing deleted the SPARK-4883 branch December 23, 2014 02:11

[SPARK-4883][Shuffle] Add a name to the directoryCleaner thread #3734

[SPARK-4883][Shuffle] Add a name to the directoryCleaner thread #3734

Uh oh!

Conversation

zsxwing commented Dec 18, 2014

Uh oh!

SparkQA commented Dec 18, 2014

Uh oh!

SparkQA commented Dec 18, 2014

Uh oh!

AmplabJenkins commented Dec 18, 2014

Uh oh!

aarondav Dec 19, 2014

Choose a reason for hiding this comment

Uh oh!

zsxwing Dec 19, 2014

Choose a reason for hiding this comment

Uh oh!

aarondav Dec 19, 2014

Choose a reason for hiding this comment

Uh oh!

zsxwing Dec 19, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 19, 2014

Uh oh!

SparkQA commented Dec 19, 2014

Uh oh!

AmplabJenkins commented Dec 19, 2014

Uh oh!

aarondav Dec 19, 2014

Choose a reason for hiding this comment

Uh oh!

aarondav commented Dec 19, 2014

Uh oh!

SparkQA commented Dec 20, 2014

Uh oh!

zsxwing commented Dec 20, 2014

Uh oh!

SparkQA commented Dec 20, 2014

Uh oh!

AmplabJenkins commented Dec 20, 2014

Uh oh!

andrewor14 commented Dec 22, 2014

Uh oh!

Uh oh!