-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-2290] Worker should directly use its own sparkHome instead of appDesc.sparkHome when LaunchExecutor #1244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…appDesc.sparkHome when LaunchExecutor
|
Can one of the admins verify this patch? |
|
If we are going to remove this feature, we should just take the sparkHome field out of |
…ir is a list of multiple paths and one of them has error
…appDesc.sparkHome when LaunchExecutor
|
The sarkHome field is taken out of ApplicationDescription entirely. Please review again. Thanks. |
|
LGTM pending tests, this is something that has confused people before, so I think it's best to just leave it out. |
|
Jenkins, test this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16401/ |
|
Jenkins, retest this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16440/ |
|
Jenkins, test this please |
|
Changes look reasonable to me. There were a few questions from the mailing list about this, so it'll be good to get this in. |
|
Merged build triggered. |
|
Merged build started. |
|
QA tests have started for PR 1244. This patch merges cleanly. |
|
QA results for PR 1244: |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16518/ |
|
@YanTangZhai This doesn't compile... could you fix it? |
|
I've fixed the compile problem. Please review and test again. Thanks very much. |
|
instead of discarding sparkHome parameter entirely, shall we just prioritizing local SPARK_HOME env and pass the applicationDesc.sparkHome only if SPARK_HOME is not set in worker side? |
|
@CodingCat We only want to do this if the driver shares the same directory structure as the executors. This is an assumption that is incorrect in many deployment settings. Really, we should have something like I am not 100% sure if we can just rip this functionality out actually. I am under the impression that Mesos still depends on something like this, so we should double check before we remove it. |
|
@andrewor14 yeah, I agree with you, I just thought in somewhere (document in the earlier versions? I cannot find it now), the user has to set this env variable? so I said prioritizing worker side SPARK_HOME, if this is not set, Spark will try to read application setup about SPARK_HOME (which may generates error if the directory structure is not the same).... I also noticed this JIRA https://issues.apache.org/jira/browse/SPARK-2454 (left some comments there) |
Create the log directory if it does not already exist. This is required when running in a multitenancy-enabled cluster, since Spark applications in each namespace will be writing logs in a namespace-specific folder. Signed-off-by: Konstantinos Lolos <[email protected]> Co-authored-by: Konstantinos Lolos <[email protected]>
Worker should directly use its own sparkHome instead of appDesc.sparkHome when LaunchExecutor