-
Notifications
You must be signed in to change notification settings - Fork 28.4k
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
[SPARK-12699][SPARKR] R driver process should start in a clean state #10652
Conversation
jenkins, retest this please |
@@ -36,7 +36,8 @@ private[deploy] object RPackageUtils extends Logging { | |||
private final val hasRPackage = "Spark-HasRPackage" | |||
|
|||
/** Base of the shell command used in order to install R packages. */ | |||
private final val baseInstallCmd = Seq("R", "CMD", "INSTALL", "-l") | |||
private final val baseInstallCmd = Seq("R", "--no-save", "--no-site-file", "--no-environ", | |||
"--no-restore", "CMD", "INSTALL", "-l") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these options do not make sense for R package installation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually think it does - it's easier to try to install package in a clean state than trying to debug when the job failed because the package failed to install.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just installation that will not start R session, so these options won't be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually would load the same site file, saved session etc when launching R with R CMD
- look for R CMD
in https://stat.ethz.ch/R-manual/R-devel/library/base/html/Startup.html
So I'm not completely sure this is a good idea. Users might have their own R environment setup scripts in their home directory (site-file or init-file as in the R docs you linked to) that they expect to work on the driver side. On the executor side it is much more limited in terms of what code runs (i.e. invisible to the user) so I don't think the same expectations can be matched with respect to that ? |
Driver could also be running in YARN cluster mode in which a clean state might make sense? |
Yeah doing it just for the cluster mode driver seems fine to me. |
Test build #49735 has finished for PR 10652 at commit
|
RRunner is not only for running driver on cluster, but also for running an R script locally in client mode. |
@sun-rui is it |
@felixcheung, yes, something like that |
I don't know if there is a way to distinguish that. I guess we could explicitly bypass this if the cluster manager is |
It is possible to get deploy mode from "spark.submit.deployMode", and check if it is "client". You can take a look at https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/api/r/RUtils.scala#L49 |
I realize that, my point is even in client mode the driver could be running on a worker machine, as in the case Spark job is submitted from another YARN app. To elaborate more, one of the possible source of issue is running Spark job in YARN client mode from a workflow engine (eg. Oozie). In such a case, the driver/client is actually being run on a random worker node of the cluster. If we think picking up random profile that way is ok, then I guess I could change it to add the flags only when deployMode is cluster. |
Currently we have R worker process launched with the --vanilla option that brings it up in a clean state (without init profile or workspace data, https://stat.ethz.ch/R-manual/R-devel/library/base/html/Startup.html). However, the R process for the Spark driver is not.
We should do that because
Here are the changes proposed:
sparkR
shell (except: allow save/restore workspace, since the driver/shell is local)This is discussed in PR #10171
@shivaram @sun-rui