[SPARK-10643] [Core] Make spark-submit download remote files to local in client mode #18078

loneknightpy · 2017-05-24T00:00:01Z

What changes were proposed in this pull request?

This PR makes spark-submit script download remote files to local file system for local/standalone client mode.

How was this patch tested?

Unit tests
Manual tests by adding s3a jar and testing against file on s3.

Please review http://spark.apache.org/contributing.html before opening a pull request.

…l/standalone client mode

vanzin · 2017-05-24T00:09:37Z

Please reference the existing bug (SPARK-10643) instead.

SparkQA · 2017-05-24T02:47:05Z

Test build #77268 has finished for PR 18078 at commit 4cdeeed.

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

SparkQA · 2017-05-24T03:14:12Z

Test build #77271 has finished for PR 18078 at commit 6e86290.

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

jiangxb1987

Thank you for working on this! The PR looks good overall, it would be great if more negative test cases could be added such as invalid file paths.

jiangxb1987 · 2017-05-25T20:42:02Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

      RPackageUtils.checkAndBuildRPackage(args.jars, printStream, args.verbose)
    }

+    // In client mode, download remotes files.


nit: "remotes" -> "remote"

jiangxb1987 · 2017-05-25T20:44:53Z

core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala

  test("resolves command line argument paths correctly") {
    val jars = "/jar1,/jar2"                 // --jars
-    val files = "hdfs:/file1,file2"          // --files
+    val files = "local:/file1,file2"          // --files


Could you expand on why we are changing this?

To make it not try to download file from hdfs

It is kinda difficult to test download file from hdfs now, but we should cover this scene in the future.

jiangxb1987 · 2017-05-25T20:51:41Z

Could you also add "[Core]" tag in the title? @loneknightpy
Also cc @cloud-fan @gatorsmile

SparkQA · 2017-05-25T21:27:35Z

Test build #77388 has started for PR 18078 at commit e5171ca.

jiangxb1987 · 2017-05-25T21:47:03Z

LGTM

SparkQA · 2017-05-25T22:02:36Z

Test build #77389 has started for PR 18078 at commit 62e57df.

cloud-fan · 2017-05-26T03:01:45Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

    }

+    // In client mode, download remote files.
+    if (deployMode == CLIENT) {


sorry I may not have enough background knowledge, why we only do this for client mode?

It seems it can handle remote files in Yarn/Mesos cluster mode. I haven't tested it, because we are using client mode.

gatorsmile · 2017-05-26T05:23:52Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

+  /**
+   * Download a list of remote files to temp local files. If the file is local, the original file
+   * will be returned.
+   * @param fileList A comma separated file list, it cannot be null.


Nit: no need to add the comment it cannot be null. Just add an assert at the beginning of the function to ensure it.

gatorsmile · 2017-05-26T05:30:41Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

+  }
+
+  /**
+   * Download remote file to a temporary local file. If the file is local, the original file


How about?

Downloads a file from the remote to a local temporary directory. If the input path points to a local path, returns it with no operation

gatorsmile · 2017-05-26T06:11:11Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

+          .fromPath(tmpFile.getAbsolutePath)
+          .scheme("file")
+          .build()
+          .toString


val localPath = new Path(tmpFile.getAbsolutePath) fs.copyToLocalFile(new Path(uri), localPath) val localFS: FileSystem = localPath.getFileSystem(hadoopConf) localFS.makeQualified(localPath).toString

Does this work?

It sounds like our code base never calls UriBuilder directly.

If UriBuilder is a concern, we can just use "file:${tmpFile.getAbsolutePath}"

SparkQA · 2017-05-26T06:12:38Z

Test build #77412 has started for PR 18078 at commit 7eb5d1a.

SparkQA · 2017-05-26T06:37:37Z

Test build #77413 has started for PR 18078 at commit 3f18b81.

gatorsmile · 2017-05-26T06:56:47Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

+        printStream.println(s"Downloading ${uri.toString} to ${tmpFile.getAbsolutePath}.")
+        // scalastyle:on println
+        fs.copyToLocalFile(new Path(uri), new Path(tmpFile.getAbsolutePath))
+        s"file:${tmpFile.getAbsolutePath}"


Or calling Utils.resolveURI(tmpFile.getAbsolutePath).toString?

It sounds Utils.resolveURI is commonly used for this purpose?

cloud-fan · 2017-05-26T07:10:29Z

retest this please

SparkQA · 2017-05-26T09:46:03Z

Test build #77418 has finished for PR 18078 at commit 3f18b81.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2017-05-26T16:46:49Z

LGTM pending Jenkins

SparkQA · 2017-05-26T20:32:36Z

Test build #77427 has finished for PR 18078 at commit 2d6f2cd.

This patch fails from timeout after a configured wait of `250m`.
This patch merges cleanly.
This patch adds no public classes.

loneknightpy · 2017-05-26T20:37:15Z

retest this please

SparkQA · 2017-05-26T23:24:22Z

Test build #77434 has finished for PR 18078 at commit 2d6f2cd.

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

SparkQA · 2017-05-26T23:26:17Z

Test build #77435 has finished for PR 18078 at commit 2d6f2cd.

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

…in client mode ## What changes were proposed in this pull request? This PR makes spark-submit script download remote files to local file system for local/standalone client mode. ## How was this patch tested? - Unit tests - Manual tests by adding s3a jar and testing against file on s3. Please review http://spark.apache.org/contributing.html before opening a pull request. Author: Yu Peng <loneknightpy@gmail.com> Closes #18078 from loneknightpy/download-jar-in-spark-submit. (cherry picked from commit 4af3781) Signed-off-by: Xiao Li <gatorsmile@gmail.com>

gatorsmile · 2017-05-26T23:32:05Z

Thanks! Merging to master/2.2

Make spark-submit download remote files to local file system for loca…

4cdeeed

…l/standalone client mode

loneknightpy changed the title ~~[SPARK-20860] Make spark-submit download remote files to local in client mode~~ [SPARK-10643] Make spark-submit download remote files to local in client mode May 24, 2017

enable download for yarn and mesos as well

6e86290

jiangxb1987 reviewed May 25, 2017

View reviewed changes

loneknightpy changed the title ~~[SPARK-10643] Make spark-submit download remote files to local in client mode~~ [SPARK-10643] [Core] Make spark-submit download remote files to local in client mode May 25, 2017

address

e5171ca

add more tests

62e57df

cloud-fan reviewed May 26, 2017

View reviewed changes

gatorsmile reviewed May 26, 2017

View reviewed changes

comments

7eb5d1a

gatorsmile reviewed May 26, 2017

View reviewed changes

remove url builder

3f18b81

gatorsmile reviewed May 26, 2017

View reviewed changes

resolveURI

2d6f2cd

asfgit closed this in 4af3781 May 26, 2017

jerryshao mentioned this pull request Jul 6, 2017

[SPARK-21012][Submit] Add glob support for resources adding to Spark #18235

Closed

jerryshao mentioned this pull request Sep 21, 2017

[SPARK-21714][CORE][BACKPORT-2.2] Avoiding re-uploading remote resources in yarn client mode #19074

Closed

[SPARK-10643] [Core] Make spark-submit download remote files to local in client mode #18078

[SPARK-10643] [Core] Make spark-submit download remote files to local in client mode #18078

Uh oh!

Conversation

loneknightpy commented May 24, 2017

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

vanzin commented May 24, 2017

Uh oh!

SparkQA commented May 24, 2017

Uh oh!

SparkQA commented May 24, 2017

Uh oh!

jiangxb1987 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiangxb1987 commented May 25, 2017

Uh oh!

SparkQA commented May 25, 2017

Uh oh!

jiangxb1987 commented May 25, 2017

Uh oh!

SparkQA commented May 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loneknightpy May 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 26, 2017

Uh oh!

SparkQA commented May 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented May 26, 2017

Uh oh!

SparkQA commented May 26, 2017

Uh oh!

gatorsmile commented May 26, 2017

Uh oh!

SparkQA commented May 26, 2017

Uh oh!

loneknightpy commented May 26, 2017

Uh oh!

SparkQA commented May 26, 2017

Uh oh!

SparkQA commented May 26, 2017

Uh oh!

gatorsmile commented May 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

loneknightpy May 26, 2017 •

edited

Loading

gatorsmile commented May 26, 2017 •

edited

Loading