[SPARK-3319] [SPARK-3338] Resolve Spark submit config paths

[andrewor14]

The bulk of this PR is comprised of tests. All changes in functionality are made in SparkSubmit.scala (~20 lines).

SPARK-3319. There is currently a divergence in behavior when the user passes in additional jars through --jars and through setting spark.jars in the default properties file. The former will happily resolve the paths (e.g. convert my.jar to file:/absolute/path/to/my.jar), while the latter does not. We should resolve paths consistently in both cases. This also applies to the following pairs of command line arguments and Spark configs:

• --jars ~ spark.jars
• --files ~ spark.files / spark.yarn.dist.files
• --archives ~ spark.yarn.dist.archives
• --py-files ~ spark.submit.pyFiles

SPARK-3338. This PR also fixes the following bug: if the user sets spark.submit.pyFiles in his/her properties file, it does not actually get picked up even if --py-files is not set. This is simply because the config is overridden by an empty string.

[SparkQA]

QA tests have started for PR 2232 at commit 05e03d6.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2232 at commit 05e03d6.

• This patch passes unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[andrewor14]

@sryza can you look at this?

[SparkQA]

QA tests have started for PR 2232 at commit f0fae64.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2232 at commit f0fae64.

• This patch passes unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[sryza]

Could this change behavior in cases where the spark.yarn.dist.files is configured with no scheme? Without this change, it would interpret no scheme to mean that it's on HDFS, and with it, it would interpret it to mean that it's on the local FS?

[andrewor14]

Yes, this changes the behavior if the user sets spark.yarn.dist.*. Note that the whole point of this PR is to fix the divergence in path resolution behavior in the following two cases: (1) if the user specifies --files, and (2) if the user specifies the corresponding config, which in the case of yarn-client is spark.yarn.dist.files. Currently, we resolve the paths only for (1) but not for (2).

However I believe these configs are actually introduced before Spark submit. Do you think we should leave the yarn ones out in this PR to keep backward compatibility?

[sryza]

Hmm. My feeling is that it's better to be consistent here and consider the old behavior a bug than to maintain compatibility than to support a cornerish case for a property that's rarely used. Just wanted to bring it up. @pwendell do you have an opinion on this?

Either way, for each of these properties, we should document whether they can take HDFS paths and the default behavior when no scheme is given. Also, are there any strange corner cases we can run into if a file exists at the same path on the local FS and HDFS?

Last, do we need to also resolve URIs for spark.yarn.jar?

[andrewor14]

Yes, probably. This list is by no means complete. For spark.yarn.dist.* however, it seems that there is some prior discussion on keeping it unresolved (i.e. relative paths become HDFS paths) here: #969. @tgravescs and @vanzin may have objections on this.

[tgravescs]

From the other pr we need to keep backwards compatibility for the env variables, the configs should be fine. I believe this pr matches the behavior described below. Please correct me if I'm wrong as I haven't looked at the code in detail.

The behavior we want is and should have been the behavior as of pr969:

• --archives/--files via sparksubmit defaults to use file:// if not specified, for both yarn-client and yarn-cluster
• spark.yarn.dist.archives/spark.yarn.dist.files defaults to use file:// if not specified, for both yarn-client and yarn-cluster
• env variable SPARK_YARN_DIST_ARCHIVES/SPARK_YARN_DIST_FILES set in yarn-client then it should default to hdfs://
• --files/--archives specified from spark-class in yarn-cluster mode then it should default to hdfs://

[andrewor14]

Hi @tgravescs, I believe every point of the behavior you listed is correct and preserved in this PR, since it only affects spark.yarn.dist.* and these are resolved to file:// if relative paths are provided. Then it seems OK to keep the spark.yarn.dist.* in the list of things to resolve in SparkSubmit here.

[andrewor14]

Alright, I just resolved the merge conflict and added spark.yarn.jar. @sryza @tgravescs I believe I addressed all your comments. Anything else?

[SparkQA]

Test build #22549 has started for PR 2232 at commit fff2869.

• This patch merges cleanly.

[SparkQA]

Test build #22549 has finished for PR 2232 at commit fff2869.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22549/
Test PASSed.

[sryza]

This looks good to me.

[andrewor14]

Alright merging this.