[MESOS][SPARK-8873] Clean up external shuffle data on driver exit with Mesos. #7820

tnachen · 2015-07-31T07:15:44Z

@andrewor14 @dragos

SparkQA · 2015-07-31T09:26:31Z

Test build #39186 has finished for PR 7820 at commit fb11a33.

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

SparkQA · 2015-07-31T09:34:45Z

Test build #39188 has finished for PR 7820 at commit 1910bd5.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class RegisterMesosDriver(appId: String, address: RpcAddress)
- class MulticlassClassificationEvaluator (override val uid: String)
- class NaiveBayes(JavaEstimator, HasFeaturesCol, HasLabelCol, HasPredictionCol):
- class NaiveBayesModel(JavaModel):
- class MulticlassClassificationEvaluator(JavaEvaluator, HasLabelCol, HasPredictionCol):

andrewor14 · 2015-07-31T22:00:10Z

retest this please

SparkQA · 2015-08-01T00:16:56Z

Test build #39281 has finished for PR 7820 at commit 1910bd5.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class RegisterMesosDriver(appId: String, address: RpcAddress)

andrewor14 · 2015-08-01T07:49:38Z

retest this please

andrewor14 · 2015-08-01T07:50:10Z

core/src/main/scala/org/apache/spark/deploy/mesos/MesosExternalShuffleService.scala

private[spark]

also please add a simple java doc, even something like A message sent from the driver to register with this shuffle service. would do.

andrewor14 · 2015-08-01T08:17:59Z

@tnachen looks great. My comments are mostly minor but I think this is very close.

SparkQA · 2015-08-01T10:17:40Z

Test build #39344 has finished for PR 7820 at commit 1910bd5.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class RegisterMesosDriver(appId: String, address: RpcAddress)

SparkQA · 2015-08-02T09:02:22Z

Test build #39419 has finished for PR 7820 at commit e0f963b.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class ExternalShuffleService(
- public class JavaKMeansExample
- public class MesosExternalShuffleClient extends ExternalShuffleClient
- public class RegisterDriver extends BlockTransferMessage

SparkQA · 2015-08-02T09:45:14Z

Test build #39421 has finished for PR 7820 at commit aca93da.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class ExternalShuffleService(
- public class JavaKMeansExample
- public class MesosExternalShuffleClient extends ExternalShuffleClient
- public class RegisterDriver extends BlockTransferMessage

SparkQA · 2015-08-02T19:10:43Z

Test build #39434 has finished for PR 7820 at commit fadff89.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class RequestExecutors(appId: String, requestedTotal: Int)
- case class KillExecutors(appId: String, executorIds: Seq[String])
- class ExternalShuffleService(
- public class JavaKMeansExample
- public class MesosExternalShuffleClient extends ExternalShuffleClient
- public class RegisterDriver extends BlockTransferMessage
- class SpecificSafeProjection extends $
- case class FromUTCTimestamp(left: Expression, right: Expression)
- case class ToUTCTimestamp(left: Expression, right: Expression)
- case class DateDiff(endDate: Expression, startDate: Expression)
- case class InitCap(child: Expression) extends UnaryExpression with ImplicitCastInputTypes

andrewor14 · 2015-08-02T22:56:35Z

core/src/main/scala/org/apache/spark/deploy/ExternalShuffleService.scala

This interface is confusing: we pass in both a SparkConf and a TransportConf, even though one is created from another one.

andrewor14 · 2015-08-02T23:12:00Z

@tnachen Thanks for addressing the comments quickly. The latest changes need a bit of clean up, however. Have you had a chance to test the latest changes on a real cluster and verify that the shuffle files did get cleaned up?

tnachen · 2015-08-02T23:21:04Z

Yes I tested with a single master and slave and it worked.
I can get to the comments after 7 today.

On Aug 2, 2015, at 4:12 PM, andrewor14 [email protected] wrote:

@tnachen Thanks for addressing the comments quickly. The latest changes need a bit of clean up, however. Have you had a chance to test the latest changes on a real cluster and verify that the shuffle files did get cleaned up?

—
Reply to this email directly or view it on GitHub.

andrewor14 · 2015-08-03T02:00:21Z

Hey @tnachen I opened #7881 after addressing the comments myself. Could you test out the latest changes in that patch instead?

andrewor14 · 2015-08-03T08:56:54Z

Can you close this one now that #7881 is already merged?

…ce is used This patch builds directly on #7820, which is largely written by tnachen. The only addition is one commit for cleaning up the code. There should be no functional differences between this and #7820. Author: Timothy Chen <[email protected]> Author: Andrew Or <[email protected]> Closes #7881 from andrewor14/tim-cleanup-mesos-shuffle and squashes the following commits: 8894f7d [Andrew Or] Clean up code 2a5fa10 [Andrew Or] Merge branch 'mesos_shuffle_clean' of github.com:tnachen/spark into tim-cleanup-mesos-shuffle fadff89 [Timothy Chen] Address comments. e4d0f1d [Timothy Chen] Clean up external shuffle data on driver exit with Mesos.

tnachen · 2015-08-05T23:53:10Z

This is already merged.

dragos · 2015-08-12T15:44:26Z

sbin/start-mesos-shuffle-service.sh

Do we still need the old one, start-shuffle-service.sh? It was only used by Mesos as far as I remember.

I think we need to keep it for backward compatibility unfortunately.

tnachen force-pushed the mesos_shuffle_clean branch 2 times, most recently from fb11a33 to 1910bd5 Compare July 31, 2015 07:18

andrewor14 reviewed Aug 1, 2015
View reviewed changes

Clean up external shuffle data on driver exit with Mesos.

e4d0f1d

tnachen force-pushed the mesos_shuffle_clean branch from 1910bd5 to e0f963b Compare August 2, 2015 08:50

tnachen force-pushed the mesos_shuffle_clean branch from e0f963b to aca93da Compare August 2, 2015 09:25

Address comments.

fadff89

tnachen force-pushed the mesos_shuffle_clean branch from aca93da to fadff89 Compare August 2, 2015 17:00

andrewor14 reviewed Aug 2, 2015
View reviewed changes

andrewor14 mentioned this pull request Aug 3, 2015

[SPARK-8873][MESOS] Clean up shuffle files if external shuffle service is used #7881

Closed

tnachen closed this Aug 5, 2015

dragos reviewed Aug 12, 2015
View reviewed changes

[MESOS][SPARK-8873] Clean up external shuffle data on driver exit with Mesos. #7820

[MESOS][SPARK-8873] Clean up external shuffle data on driver exit with Mesos. #7820

Uh oh!

Conversation

tnachen commented Jul 31, 2015

Uh oh!

SparkQA commented Jul 31, 2015

Uh oh!

SparkQA commented Jul 31, 2015

Uh oh!

andrewor14 commented Jul 31, 2015

Uh oh!

SparkQA commented Aug 1, 2015

Uh oh!

andrewor14 commented Aug 1, 2015

Uh oh!

andrewor14 Aug 1, 2015

Choose a reason for hiding this comment

Uh oh!

andrewor14 Aug 1, 2015

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Aug 1, 2015

Uh oh!

SparkQA commented Aug 1, 2015

Uh oh!

SparkQA commented Aug 2, 2015

Uh oh!

SparkQA commented Aug 2, 2015

Uh oh!

SparkQA commented Aug 2, 2015

Uh oh!

andrewor14 Aug 2, 2015

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Aug 2, 2015

Uh oh!

tnachen commented Aug 2, 2015

Uh oh!

andrewor14 commented Aug 3, 2015

Uh oh!

andrewor14 commented Aug 3, 2015

Uh oh!

tnachen commented Aug 5, 2015

Uh oh!

dragos Aug 12, 2015

Choose a reason for hiding this comment

Uh oh!

andrewor14 Aug 12, 2015

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants