You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zuotingbing <gi...@git.apache.org> on 2017/01/06 07:19:01 UTC
[GitHub] spark pull request #16484: Sh
GitHub user zuotingbing opened a pull request:
https://github.com/apache/spark/pull/16484
Sh
JIRA Issue: https://issues.apache.org/jira/browse/SPARK-19083#
sbin/start-history-server.sh script use of $@ without quotes, this will affect the length of args which used in HistoryServerArguments::parse(args: List[String])
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zuotingbing/spark sh
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16484.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 #16484
----
commit e4fe18e2fb894c6e41d49e620d342d6be475b5ed
Author: zuotingbing <zu...@zte.com.cn>
Date: 2016-12-30T02:53:13Z
local directories cannot be cleanuped when create directory of executor-*** throws IOException
commit 61cf467d6da6618bff9f2a001adc56b26c1a3421
Author: zuotingbing <zu...@zte.com.cn>
Date: 2017-01-03T01:58:11Z
code review
commit 755a23988f29473d48efb81090a8ef41c6a1e9f0
Author: zuotingbing <zu...@zte.com.cn>
Date: 2017-01-06T07:14:46Z
Update sbin/start-history-server.sh
[SPARK-19083]sbin/start-history-server.sh script use of $@ without quotes, this will affect the length of args which used in HistoryServerArguments::parse(args: List[String])
----
---
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 #16484: [SPARK-19083]sbin/start-history-server.sh script ...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on a diff in the pull request:
https://github.com/apache/spark/pull/16484#discussion_r94911586
--- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala ---
@@ -445,12 +445,24 @@ private[deploy] class Worker(
// Create local dirs for the executor. These are passed to the executor via the
// SPARK_EXECUTOR_DIRS environment variable, and deleted by the Worker when the
// application finishes.
- val appLocalDirs = appDirectories.getOrElse(appId,
- Utils.getOrCreateLocalRootDirs(conf).map { dir =>
- val appDir = Utils.createDirectory(dir, namePrefix = "executor")
- Utils.chmod700(appDir)
- appDir.getAbsolutePath()
- }.toSeq)
+ val appLocalDirs = appDirectories.getOrElse(appId, {
--- End diff --
Sorry, this change is for another issue. Should i close this PR and checkout a new branch for this issue ?
---
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 #16484: [SPARK-19083]sbin/start-history-server.sh script use of ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/16484
Would you mind checking all the scripts under `bin` and `sbin` to see if this issue also existed in other scripts.
---
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 #16484: [SPARK-19083]sbin/start-history-server.sh script use of ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/16484
> Would it cause each individual script arg to act as if quoted?
Yes.
LGTM. Merging to master / 2.1.
---
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 #16484: [SPARK-19083]sbin/start-history-server.sh script use of ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16484
This is the only instance I see, yes. Other instances are in dev scripts which don't matter.
Could you explain what difference it makes? I read the bash reference and don't fully understand. Would it cause each individual script arg to act as if quoted?
---
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 #16484: Sh
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16484
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 #16484: [SPARK-19083]sbin/start-history-server.sh script ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/16484#discussion_r94911716
--- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala ---
@@ -445,12 +445,24 @@ private[deploy] class Worker(
// Create local dirs for the executor. These are passed to the executor via the
// SPARK_EXECUTOR_DIRS environment variable, and deleted by the Worker when the
// application finishes.
- val appLocalDirs = appDirectories.getOrElse(appId,
- Utils.getOrCreateLocalRootDirs(conf).map { dir =>
- val appDir = Utils.createDirectory(dir, namePrefix = "executor")
- Utils.chmod700(appDir)
- appDir.getAbsolutePath()
- }.toSeq)
+ val appLocalDirs = appDirectories.getOrElse(appId, {
--- End diff --
you could push another commit to this branch to revert this change.
---
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 #16484: [SPARK-19083]sbin/start-history-server.sh script ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/16484
---
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 #16484: [SPARK-19083]sbin/start-history-server.sh script use of ...
Posted by zuotingbing <gi...@git.apache.org>.
Github user zuotingbing commented on the issue:
https://github.com/apache/spark/pull/16484
@jerryshao Thank you for your advice. I checked off all the scripts under bin and sbin. It seems ok.
---
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 #16484: [SPARK-19083]sbin/start-history-server.sh script ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/16484#discussion_r94910539
--- Diff: core/src/main/scala/org/apache/spark/deploy/worker/Worker.scala ---
@@ -445,12 +445,24 @@ private[deploy] class Worker(
// Create local dirs for the executor. These are passed to the executor via the
// SPARK_EXECUTOR_DIRS environment variable, and deleted by the Worker when the
// application finishes.
- val appLocalDirs = appDirectories.getOrElse(appId,
- Utils.getOrCreateLocalRootDirs(conf).map { dir =>
- val appDir = Utils.createDirectory(dir, namePrefix = "executor")
- Utils.chmod700(appDir)
- appDir.getAbsolutePath()
- }.toSeq)
+ val appLocalDirs = appDirectories.getOrElse(appId, {
--- End diff --
What is the purpose of change here, from my understanding it is not related to the issue you mentioned in PR description.
---
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