You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ilganeli <gi...@git.apache.org> on 2015/03/10 21:29:56 UTC
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
GitHub user ilganeli opened a pull request:
https://github.com/apache/spark/pull/4965
[SPARK-5845] Time to cleanup spilled shuffle files not included in shuffle write time
I've added a timer in the right place to fix this inaccuracy.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ilganeli/spark SPARK-5845
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4965.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4965
----
commit db8647e268097358c5bf184a31e35e3404751f8d
Author: Ilya Ganelin <il...@capitalone.com>
Date: 2015-02-25T01:11:59Z
Added update for shuffleWriteTime around spilled file cleanup in ExternalSorter
commit 9434b503835b6431105d82caeda1444f2c14b473
Author: Ilya Ganelin <il...@capitalone.com>
Date: 2015-03-10T20:29:22Z
Merge remote-tracking branch 'upstream/master' into SPARK-5845
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78305952
[Test build #28475 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28475/consoleFull) for PR 4965 at commit [`3e059b0`](https://github.com/apache/spark/commit/3e059b002e18da3241a8358ee5ccda723e25b1cc).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845][Shuffle] Time to cleanup spilled ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4965#discussion_r26351183
--- Diff: core/src/main/scala/org/apache/spark/shuffle/sort/SortShuffleWriter.scala ---
@@ -88,7 +88,12 @@ private[spark] class SortShuffleWriter[K, V, C](
} finally {
// Clean up our sorter, which may have its own intermediate files
if (sorter != null) {
+ val startTime = System.nanoTime()
sorter.stop()
+ context.taskMetrics().shuffleWriteMetrics.getOrElse({
+ metrics : ShuffleWriteMetrics =>
+ metrics.incShuffleWriteTime(System.nanoTime()-startTime)
+ },Nil)
--- End diff --
OK. As a matter of style I think it would be better to...
```
context.taskMetrics.shuffleWriteMetrics.foreach(
_.incShuffleWriteTime(System.nanoTime - startTime))
```
Which is what `ExternalSorter` does. This looks like the correct bit to time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845][Shuffle] Time to cleanup spilled ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78711225
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28538/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78178449
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28449/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78141857
[Test build #28438 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28438/consoleFull) for PR 4965 at commit [`9434b50`](https://github.com/apache/spark/commit/9434b503835b6431105d82caeda1444f2c14b473).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78334698
[Test build #28475 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28475/consoleFull) for PR 4965 at commit [`3e059b0`](https://github.com/apache/spark/commit/3e059b002e18da3241a8358ee5ccda723e25b1cc).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on a diff in the pull request:
https://github.com/apache/spark/pull/4965#discussion_r26230494
--- Diff: core/src/main/scala/org/apache/spark/shuffle/sort/SortShuffleWriter.scala ---
@@ -88,7 +88,13 @@ private[spark] class SortShuffleWriter[K, V, C](
} finally {
// Clean up our sorter, which may have its own intermediate files
if (sorter != null) {
+ val startTime = System.nanoTime()
--- End diff --
Hi Sean - other usages of writeMetrics.incShuffleWriteTime also use nanoTime(). Please see BlockObjectWriter::callWithTiming() and ExternalSorter::writePartitionedFile.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78177198
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845][Shuffle] Time to cleanup spilled ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78711189
[Test build #28538 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28538/consoleFull) for PR 4965 at commit [`bfabf88`](https://github.com/apache/spark/commit/bfabf88935658246cd2bb30eb317c6992282098d).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78178445
[Test build #28449 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28449/consoleFull) for PR 4965 at commit [`b946d08`](https://github.com/apache/spark/commit/b946d085c50e5e5072d7429fe8a634e5b401986c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78142236
[Test build #28438 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28438/consoleFull) for PR 4965 at commit [`9434b50`](https://github.com/apache/spark/commit/9434b503835b6431105d82caeda1444f2c14b473).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4965#discussion_r26215999
--- Diff: core/src/main/scala/org/apache/spark/shuffle/sort/SortShuffleWriter.scala ---
@@ -88,7 +88,13 @@ private[spark] class SortShuffleWriter[K, V, C](
} finally {
// Clean up our sorter, which may have its own intermediate files
if (sorter != null) {
+ val startTime = System.nanoTime()
--- End diff --
CC @kayousterhout
Just checking, this is meant to be in nanos and not milliseconds?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78335321
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28475/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845][Shuffle] Time to cleanup spilled ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4965
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78142240
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28438/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78176938
[Test build #28449 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28449/consoleFull) for PR 4965 at commit [`b946d08`](https://github.com/apache/spark/commit/b946d085c50e5e5072d7429fe8a634e5b401986c).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845][Shuffle] Time to cleanup spilled ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78678027
[Test build #28538 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28538/consoleFull) for PR 4965 at commit [`bfabf88`](https://github.com/apache/spark/commit/bfabf88935658246cd2bb30eb317c6992282098d).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5845] Time to cleanup spilled shuffle f...
Posted by ilganeli <gi...@git.apache.org>.
Github user ilganeli commented on the pull request:
https://github.com/apache/spark/pull/4965#issuecomment-78305887
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org