You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by guoxiaolongzte <gi...@git.apache.org> on 2017/08/02 01:48:51 UTC
[GitHub] spark pull request #18806: [SPARK-21600] The description of "this requires s...
GitHub user guoxiaolongzte opened a pull request:
https://github.com/apache/spark/pull/18806
[SPARK-21600] The description of "this requires spark.shuffle.service.enabled to be set" for the spark.dynamicAllocation.enabled configuration item is not clear
## What changes were proposed in this pull request?
The description of "this requires spark.shuffle.service.enabled to be set" for the spark.dynamicAllocation.enabled configuration item is not clear. I am not sure how to set spark.shuffle.service.enabled is true or false, so that the user to guess, resulting in doubts. All i have changed here, stressed that must spark.shuffle.service.enabled to be set true.
When I set spark.dynamicAllocation.enabled=true, but set spark.shuffle.service.enabled false, When I submit the spark-submit --master yarn job, the program throws the exception.
## How was this patch tested?
manual tests
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/guoxiaolongzte/spark SPARK-21600
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18806.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 #18806
----
commit 4c8de9a46ea4a09334eaa31328cb9eea1c2eb6b1
Author: guoxiaolong <gu...@zte.com.cn>
Date: 2017-08-02T01:34:27Z
[SPARK-21600] The description of "this requires spark.shuffle.service.enabled to be set" for the spark.dynamicAllocation.enabled configuration item is not clear. I am not sure how to set spark.shuffle.service.enabled is true or false, so that the user to guess, resulting in doubts. All i have changed here, stressed that must spark.shuffle.service.enabled to be set true.
----
---
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 #18806: [SPARK-21600] The description of "this requires s...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18806#discussion_r130777347
--- Diff: docs/configuration.md ---
@@ -1638,7 +1638,7 @@ Apart from these, the following properties are also available, and may be useful
For more detail, see the description
<a href="job-scheduling.html#dynamic-resource-allocation">here</a>.
<br><br>
- This requires <code>spark.shuffle.service.enabled</code> to be set.
+ This requires <code>spark.shuffle.service.enabled</code> to be set true.
--- End diff --
I think there's no ambiguity here. Usually configuration with name "xxx.enabled" can only have two values "true" or "false". So "to be set" usually means to enable it (to set it to true).
---
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 #18806: [SPARK-21600] The description of "this requires s...
Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte commented on a diff in the pull request:
https://github.com/apache/spark/pull/18806#discussion_r130784995
--- Diff: docs/configuration.md ---
@@ -1638,7 +1638,7 @@ Apart from these, the following properties are also available, and may be useful
For more detail, see the description
<a href="job-scheduling.html#dynamic-resource-allocation">here</a>.
<br><br>
- This requires <code>spark.shuffle.service.enabled</code> to be set.
+ This requires <code>spark.shuffle.service.enabled</code> to be set true.
--- End diff --
You are right, but usually is not very sure. Can not let the user to guess, the document needs to be accurately described. In addition, the spark project, several other are clearly described, spark.shuffle.service.enabled set true.
---
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 issue #18806: [SPARK-21600][docs] The description of "this requires sp...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/18806
"set" is pretty synonymous with "true" for boolean properties. Its name includes 'enabled'. I think this is too trivial.
---
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 issue #18806: [SPARK-21600][docs] The description of "this requires sp...
Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte commented on the issue:
https://github.com/apache/spark/pull/18806
@srowen Help review the code,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 issue #18806: [SPARK-21600] The description of "this requires spark.sh...
Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte commented on the issue:
https://github.com/apache/spark/pull/18806
@srowen Help review the code,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 issue #18806: [SPARK-21600] The description of "this requires spark.sh...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18806
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 #18806: [SPARK-21600] The description of "this requires s...
Posted by guoxiaolongzte <gi...@git.apache.org>.
Github user guoxiaolongzte commented on a diff in the pull request:
https://github.com/apache/spark/pull/18806#discussion_r130788660
--- Diff: docs/configuration.md ---
@@ -1638,7 +1638,7 @@ Apart from these, the following properties are also available, and may be useful
For more detail, see the description
<a href="job-scheduling.html#dynamic-resource-allocation">here</a>.
<br><br>
- This requires <code>spark.shuffle.service.enabled</code> to be set.
+ This requires <code>spark.shuffle.service.enabled</code> to be set true.
--- End diff --
Thank you for your comments.
This requires spark.shuffle.service.enabled to be set true. It is very clearly. Only such an accurate description，there be no ambiguity.
---
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 #18806: [SPARK-21600][docs] The description of "this requ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/18806
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18806: [SPARK-21600] The description of "this requires s...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18806#discussion_r130787079
--- Diff: docs/configuration.md ---
@@ -1638,7 +1638,7 @@ Apart from these, the following properties are also available, and may be useful
For more detail, see the description
<a href="job-scheduling.html#dynamic-resource-allocation">here</a>.
<br><br>
- This requires <code>spark.shuffle.service.enabled</code> to be set.
+ This requires <code>spark.shuffle.service.enabled</code> to be set true.
--- End diff --
Since other places are clearly defined the property, so there should be no ambiguity. Personally I'm not fond of this super nit fix...
---
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