You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "leletan (via GitHub)" <gi...@apache.org> on 2024/03/26 04:58:42 UTC

[PR] [SPARK-47475][CORE]: Make Jars Download to Driver Optional under K8s Cluster Mode [spark]

leletan opened a new pull request, #45715:
URL: https://github.com/apache/spark/pull/45715

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
     8. If you want to add or modify an error type or message, please read the guideline first in
        'common/utils/src/main/resources/error/README.md'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   During spark submit, for K8s cluster mode driver, instead of always downloading jars and serving it to executors, make it only happen when its url matches scheme in the configuration.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   For K8s cluster mode driver, `SparkSubmit` will download all the jars in the `spark.jars` to driver and then those jars' urls in `spark.jars` will be replaced by the driver local paths. Later when driver starts the `SparkContext`, it will copy all the `spark.jars` to `spark.app.initial.jar.urls`, start a file server and replace the jars with driver local paths in `spark.app.initial.jar.urls` with file service urls. When the executors start, they will download those driver local jars by `spark.app.initial.jar.urls`.
   When jars are big and the spark application requests a lot of executors, the executors' massive concurrent download of the jars from the driver will cause network saturation. In this case, the executors jar download will timeout, causing executors to be terminated. From user point of view, the application is trapped in the loop of massive executor loss and re-provision, but never gets enough live executors as requested, leads to SLA breach or sometimes failure. 
   So instead of letting driver to download the jars and then serve them to executors, if we just avoid driver from downloading the jars and keeping the urls in `spark.jars` as they were, the executor will try to directly download the jars from the urls provided by user. This will avoid the driver download bottleneck mentioned above, especially when jar urls are with scalable storage schemes, like s3 or hdfs.
   Meanwhile, there are cases jar urls are with schemes of less scalable than driver file server, e.g. http, ftp, etc, or when the jars are small, or executor count is small - user may still want to fall back to current solution and use driver file server to serve the jars.
   So in this case, make the driver jars downloading and serving optional by scheme (similar idea to `FORCE_DOWNLOAD_SCHEMES` in YARN) is a good approach for the solution.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   A configuration `spark.kubernetes.jars.avoidDownloadSchemes` is added
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
   -->
   - Unit tests added
   - Tested with an application running on AWS EKS submitted with a 1GB jar on s3.
     - Before the fix, the application could not scale beyond 1000 live executors. 
     - After the fix, the application had no problem to scale 12k live executors.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   <!--
   If generative AI tooling has been used in the process of authoring this patch, please include the
   phrase: 'Generated-by: ' followed by the name of the tool and its version.
   If no, write 'No'.
   Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
   -->
   No


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Jars Download to Driver to Follow `spark.kubernetes.jars.avoidDownloadSchemes` under K8s Cluster Mode [spark]

Posted by "leletan (via GitHub)" <gi...@apache.org>.
leletan commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1539855896


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Updated.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Jars Download to Driver to Follow `spark.kubernetes.jars.avoidDownloadSchemes` under K8s Cluster Mode [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538662339


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Ya, `scalafmt` is only recommended for newly added code. Please don't touch the existing code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Jars Download to Driver to Follow `spark.kubernetes.jars.avoidDownloadSchemes` under K8s Cluster Mode [spark]

Posted by "leletan (via GitHub)" <gi...@apache.org>.
leletan commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538667385


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Thanks for the tips @dongjoon-hyun! 
   I tried running `./dev/scalafmt`, somehow a lot of existing code was refactored for style. Is this expected?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Make Jars Download to Driver Optional under K8s Cluster Mode [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538627313


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Could you take a look at `Apache Spark Coding Guideline` at this time?
   - https://spark.apache.org/contributing.html
   > For Scala code, Apache Spark follows the official [Scala style guide](http://docs.scala-lang.org/style/) and [Databricks Scala guide](https://github.com/databricks/scala-style-guide). The latter is preferred. To format Scala code, run ./dev/scalafmt prior to submitting a PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE][K8S] Support `spark.kubernetes.jars.avoidDownloadSchemes` for K8s Cluster Mode [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #45715: [SPARK-47475][CORE][K8S] Support `spark.kubernetes.jars.avoidDownloadSchemes` for K8s Cluster Mode
URL: https://github.com/apache/spark/pull/45715


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Make Jars Download to Driver Optional under K8s Cluster Mode [spark]

Posted by "leletan (via GitHub)" <gi...@apache.org>.
leletan commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538651802


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Ran ` ./dev/scalafmt` as mentioned, it actually modified a lot of existing code style as well - is this expected? Or something I missed here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Make Jars Download to Driver Optional under K8s Cluster Mode [spark]

Posted by "leletan (via GitHub)" <gi...@apache.org>.
leletan commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538651802


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Thanks for the tips, @dongjoon-hyun. Ran ` ./dev/scalafmt` as mentioned, it actually modified a lot of existing code style as well - is this expected? Or something I missed here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Jars Download to Driver to Follow `spark.kubernetes.jars.avoidDownloadSchemes` under K8s Cluster Mode [spark]

Posted by "leletan (via GitHub)" <gi...@apache.org>.
leletan commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538667385


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Thanks for the tips @dongjoon-hyun! 
   I tried running `./dev/scalafmt`, somehow a lot of existing code was refactored for style. Is this expected? Is there something I missed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Make Jars Download to Driver Optional under K8s Cluster Mode [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538627597


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   For this case, the following is the rule.
   > For method declarations, use 4 space indentation for their parameters 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Make Jars Download to Driver Optional under K8s Cluster Mode [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538631146


##########
core/src/main/scala/org/apache/spark/internal/config/package.scala:
##########
@@ -1458,6 +1458,18 @@ package object config {
       .doubleConf
       .createWithDefault(1.5)
 
+  private[spark] val KUBERNETES_AVOID_JAR_DOWNLOAD_SCHEMES =
+    ConfigBuilder("spark.kubernetes.jars.avoidDownloadSchemes")

Review Comment:
   BTW, the AS-IS PR title match with this?
   > Make Jars Download to Driver Optional under K8s Cluster Mode



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Jars Download to Driver to Follow `spark.kubernetes.jars.avoidDownloadSchemes` under K8s Cluster Mode [spark]

Posted by "leletan (via GitHub)" <gi...@apache.org>.
leletan commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538651802


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Thanks for the tips, @dongjoon-hyun. Ran ` ./dev/scalafmt` as mentioned, it actually modified a lot of existing code style as well (not relevant to this PR) - is this expected? Or there is something I missed here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Make Jars Download to Driver Optional under K8s Cluster Mode [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538626148


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+

Review Comment:
   We don't need to add a new empty line here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47475][CORE]: Jars Download to Driver to Follow `spark.kubernetes.jars.avoidDownloadSchemes` under K8s Cluster Mode [spark]

Posted by "leletan (via GitHub)" <gi...@apache.org>.
leletan commented on code in PR #45715:
URL: https://github.com/apache/spark/pull/45715#discussion_r1538651802


##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala:
##########
@@ -401,16 +401,24 @@ private[spark] class SparkSubmit extends Logging {
         // SPARK-33782 : This downloads all the files , jars , archiveFiles and pyfiles to current
         // working directory
         // SPARK-43540: add current working directory into driver classpath
+        // SPARK-47475: make download to driver optional so executors may fetch resource from remote
+        // url directly to avoid overwhelming driver network when resource is big and executor count
+        // is high
         val workingDirectory = "."
         childClasspath += workingDirectory
-        def downloadResourcesToCurrentDirectory(uris: String, isArchive: Boolean = false):
+
+        def downloadResourcesToCurrentDirectory(uris: String,
+                                                isArchive: Boolean = false,

Review Comment:
   Thanks for the tips, @dongjoon-hyun. Ran ` ./dev/scalafmt` as mentioned, it actually modified a lot of existing code style as well (not relevant to this PR) - is this expected? Or there is something I missed here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org