You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by nishkamravi2 <gi...@git.apache.org> on 2014/11/19 08:08:55 UTC
[GitHub] spark pull request: maxResultSize = 0 should be treated as unlimit...
GitHub user nishkamravi2 opened a pull request:
https://github.com/apache/spark/pull/3360
maxResultSize = 0 should be treated as unlimited
The check for maxResultSize > 0 is missing, results in failures. Also, error message needs to be improved so the developers know that there is a new parameter to be configured
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nishkamravi2/spark master_nravi
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3360.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 #3360
----
commit 681b36f5fb63e14dc89e17813894227be9e2324f
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-05-08T07:05:33Z
Fix for SPARK-1758: failing test org.apache.spark.JavaAPISuite.wholeTextFiles
The prefix "file:" is missing in the string inserted as key in HashMap
commit 5108700230fd70b995e76598f49bdf328c971e77
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-06-03T22:25:22Z
Fix in Spark for the Concurrent thread modification issue (SPARK-1097, HADOOP-10456)
commit 6b840f017870207d23e75de224710971ada0b3d0
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-06-03T22:34:02Z
Undo the fix for SPARK-1758 (the problem is fixed)
commit df2aeb179fca4fc893803c72a657317f5b5539d7
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-06-09T19:02:59Z
Improved fix for ConcurrentModificationIssue (Spark-1097, Hadoop-10456)
commit eb663ca20c73f9c467192c95fc528c6f55f202be
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-06-09T19:04:39Z
Merge branch 'master' of https://github.com/apache/spark
commit 5423a03ddf4d747db7261d08a64e32f44e8be95e
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-06-10T20:06:07Z
Merge branch 'master' of https://github.com/apache/spark
commit 3bf8fad85813037504189cf1323d381fefb6dfbe
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-06-16T05:47:00Z
Merge branch 'master' of https://github.com/apache/spark
commit 2b630f94079b82df3ebae2b26a3743112afcd526
Author: nravi <nr...@c1704.halxg.cloudera.com>
Date: 2014-06-16T06:00:31Z
Accept memory input as "30g", "512M" instead of an int value, to be consistent with rest of Spark
commit efd688a4e15b79e92d162073035b03362fcf66f0
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-07-13T00:04:17Z
Merge branch 'master' of https://github.com/apache/spark
commit 2e69f112d1be59951cd32da4127d8b51bfa03338
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-21T23:17:15Z
Merge branch 'master' of https://github.com/apache/spark into master_nravi
commit ebcde10252e6c45169ea086e8426ec9997d46490
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-22T06:44:40Z
Modify default YARN memory_overhead-- from an additive constant to a multiplier (redone to resolve merge conflicts)
commit 1cf2d1ef57ed6d783df06dad36b9505bc74329fb
Author: nishkamravi2 <ni...@gmail.com>
Date: 2014-09-22T08:54:33Z
Update YarnAllocator.scala
commit f00fa311945c1eafa8957eae5c84719521761dcd
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-22T23:06:07Z
Improving logging for AM memoryOverhead
commit c726bd9f707ce182ec8d56ffecf9da87dcdb3091
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-25T01:19:32Z
Merge branch 'master' of https://github.com/apache/spark into master_nravi
Conflicts:
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala
commit 362da5edfd04bd8bad990fb210a9e11b8494fa62
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-25T19:56:13Z
Additional changes for yarn memory overhead
commit 42c2c3d18862d3632c20931ecfe2c64883c5febf
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-25T20:02:49Z
Additional changes for yarn memory overhead issue
commit dac1047995c99f5a2670f934eb8d3a4ad9b532c8
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-25T21:20:38Z
Additional documentation for yarn memory overhead issue
commit 5ac2ec11629e19030ad5577da1eee2d135cc3d1c
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-25T21:25:44Z
Remove out
commit 35daa6498048cabb736316e2f19e565c99243b7e
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-25T21:59:22Z
Slight change in the doc for yarn memory overhead
commit 8f76c8b46379736aeb7dbe1a4d88729424a041f7
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-09-25T22:03:00Z
Doc change for yarn memory overhead
commit 636a9ffeb4a4ae0b941edd849dcbabf38821db53
Author: nishkamravi2 <ni...@gmail.com>
Date: 2014-09-30T18:33:28Z
Update YarnAllocator.scala
commit 5f8f9ede0fda5c7a4f6a411c746a3d893f550524
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-11-19T01:46:58Z
Merge branch 'master' of https://github.com/apache/spark into master_nravi
Conflicts:
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientArguments.scala
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala
commit 9f6583eb92b59e88b0994ef45dd0e1a418d349a6
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-11-19T03:42:25Z
Changes to maxResultSize code (improve error message and add condition to check if maxResultSize > 0)
commit 3e1b616a56cedec0b963b7f813a8e0195c3417e8
Author: Nishkam Ravi <nr...@cloudera.com>
Date: 2014-11-19T06:56:55Z
Modify test for maxResultSize
----
---
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-4484] Treat maxResultSize as unlimited ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63705914
> How about retaining the original message and adding a line in the end
Sounds good to me. I'd also replace "maxResultSize" in the original message with "maximum result size" if doing that.
---
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: maxResultSize = 0 should be treated as unlimit...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63600928
Can one of the admins verify this patch?
---
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-4484] Treat maxResultSize as unlimited ...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63609847
Is maxResultSize documented anywhere? I couldn't find it. If not, can we add it to the configuration page?
---
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-4484] Treat maxResultSize as unlimited ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63734728
Sorry, I wasn't being clear. The MiMa checks were failing in master, but a hot fix was merged. Once you pull in the latest changes from master the tests will pass for this PR. Does that make sense?
---
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-4484] Treat maxResultSize as unlimited ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63745463
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23641/
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-4484] Treat maxResultSize as unlimited ...
Posted by nishkamravi2 <gi...@git.apache.org>.
Github user nishkamravi2 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63724986
I'm not sure I understand.
---
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-4484] Treat maxResultSize as unlimited ...
Posted by nishkamravi2 <gi...@git.apache.org>.
Github user nishkamravi2 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63721676
Updated as per @sryza and @andrewor14 's comments
---
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-4484] Treat maxResultSize as unlimited ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63735065
@andrewor14 he shouldn't need to rebase, right? Shouldn't jenkins pick up the latest master and merge his changes on top of it?
---
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-4484] Treat maxResultSize as unlimited ...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on a diff in the pull request:
https://github.com/apache/spark/pull/3360#discussion_r20563746
--- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSetManager.scala ---
@@ -536,7 +536,7 @@ private[spark] class TaskSetManager(
calculatedTasks += 1
if (maxResultSize > 0 && totalResultSize > maxResultSize) {
val msg = s"Total size of serialized results of ${calculatedTasks} tasks " +
- s"(${Utils.bytesToString(totalResultSize)}) is bigger than maxResultSize " +
+ s"(${Utils.bytesToString(totalResultSize)}) is bigger than configurable parameter spark.driver.maxResultSize " +
--- End diff --
I think just "spark.driver.maxResultSize" would be clear.
---
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-4484] Treat maxResultSize as unlimited ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63746443
Alright merging this into master and 1.2 thanks
---
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-4484] Treat maxResultSize as unlimited ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63720998
Oh I see it's already documented that 0 means unlimited. In that case it's fine to keep it there.
---
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-4484] Treat maxResultSize as unlimited ...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63720314
I think just referencing the property by its full name and allowing the user to look it up should be sufficient. Recommending boosting the limit is not right in all situations - e.g. in some situations the user might be thinking an action was a transformation and didn't mean to pull all the data to the driver.
@andrewor14 the current documentation references 0 as unlimited so we should change that too if we switch to -1.
> On Nov 19, 2014, at 12:30 PM, andrewor14 <no...@github.com> wrote:
>
> Hey @nishkamravi2 the changes here look good. Just so I understand, the purpose of treating 0 as unlimited is to provide a way for user to raise the limit when they hit it, correct? I would even go further to tell the user to set it to -1; to me it's clearer that maxResultSize can't be negative and so it must be some magic number.
>
> —
> Reply to this email directly or view it on GitHub.
>
---
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-4484] Treat maxResultSize as unlimited ...
Posted by nishkamravi2 <gi...@git.apache.org>.
Github user nishkamravi2 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63614248
This is documented in docs/configuration.md. Yeah, 'configurable parameter' may be a bit much.
How about retaining the original message and adding a line in the end, something along the lines of "Please configure spark.driver.maxResultSize to a higher value (0 for unlimited)" ?
---
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-4484] Treat maxResultSize as unlimited ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63735786
Oh yeah you're right. Jenkins, add to whitelist.
---
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-4484] Treat maxResultSize as unlimited ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63736703
[Test build #23641 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23641/consoleFull) for PR 3360 at commit [`5c9a4cb`](https://github.com/apache/spark/commit/5c9a4cb4eb12eb6cefbf9e9dda42b722abe1d3ed).
* 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-4484] Treat maxResultSize as unlimited ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63745455
[Test build #23641 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23641/consoleFull) for PR 3360 at commit [`5c9a4cb`](https://github.com/apache/spark/commit/5c9a4cb4eb12eb6cefbf9e9dda42b722abe1d3ed).
* 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-4484] Treat maxResultSize as unlimited ...
Posted by nishkamravi2 <gi...@git.apache.org>.
Github user nishkamravi2 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63751427
Thanks @andrewor14
---
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-4484] Treat maxResultSize as unlimited ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63707945
Hey @nishkamravi2 the changes here look good. Just so I understand, the purpose of treating 0 as unlimited is to provide a way for user to raise the limit when they hit it, correct? I would even go further to tell the user to set it to -1; to me it's clearer that `maxResultSize` can't be negative and so it must be some magic number.
---
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-4484] Treat maxResultSize as unlimited ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3360
---
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-4484] Treat maxResultSize as unlimited ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3360#issuecomment-63724600
OK LGTM. Can you rebase this to master so that it passes the MiMa tests now?
---
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