Skip to content
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-32119][CORE] ExecutorPlugin doesn't work with Standalone Cluster and Kubernetes with --jars #28939

Closed
wants to merge 11 commits into from

Conversation

sarutak
Copy link
Member

@sarutak sarutak commented Jun 28, 2020

What changes were proposed in this pull request?

This PR changes Executor to load jars and files added by --jars and --files on Executor initialization.
To avoid downloading those jars/files twice, they are assosiated with startTime as their uploaded timestamp.

Why are the changes needed?

ExecutorPlugin can't work with Standalone Cluster and Kubernetes
when a jar which contains plugins and files used by the plugins are added by --jars and --files option with spark-submit.

This is because jars and files added by --jars and --files are not loaded on Executor initialization.
I confirmed it works with YARN because jars/files are distributed as distributed cache.

Does this PR introduce any user-facing change?

Yes. jars/files added by --jars and --files are downloaded on each executor on initialization.

How was this patch tested?

Added a new testcase.

@dongjoon-hyun
Copy link
Member

Hi, @sarutak . Do you think we can consider this as a Bug for 3.0.1 and 2.4.7? Currently, it's an improvement JIRA for 3.1.0.

@sarutak
Copy link
Member Author

sarutak commented Jun 28, 2020

Marked as improvement is just a mistake. I've modified it to bug.
The current plugin feature is introduced in 3.0.0 so we can consider for 3.0.1.
For 2.4, this bug may exist but the implementation of plugin feature is different from the one of 3.0 so it might be difficult to backport this change to 2.4 straightforward.

@dongjoon-hyun
Copy link
Member

Thanks, @sarutak . That's enough~

@SparkQA
Copy link

SparkQA commented Jun 28, 2020

Test build #124605 has finished for PR 28939 at commit 69a9be1.

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

@dongjoon-hyun
Copy link
Member

The failure seems relevant.

org.apache.spark.deploy.SparkSubmitSuite.SPARK-32119:
Jars and files should be loaded when Executors launch for plugins

@HyukjinKwon
Copy link
Member

cc @vanzin FYI

@@ -1254,7 +1255,7 @@ class SparkSubmitSuite
|public void init(PluginContext ctx, Map<String, String> extraConf) {
| String str = null;
| try (BufferedReader reader =
| new BufferedReader(new InputStreamReader(new FileInputStream($tempFileName)))) {
| new BufferedReader(new InputStreamReader(new FileInputStream("$tempFileName")))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting. Previously, this test case succeeds with "?

Copy link
Member Author

@sarutak sarutak Jun 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I replaced "test.txt" with $tempFileName just before the first push so, it's a miss-replacement.

@SparkQA
Copy link

SparkQA commented Jun 29, 2020

Test build #124622 has finished for PR 28939 at commit b571f4c.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sarutak
Copy link
Member Author

sarutak commented Jun 29, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Jun 29, 2020

Test build #124628 has finished for PR 28939 at commit b571f4c.

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

@sarutak
Copy link
Member Author

sarutak commented Jun 30, 2020

Those failed tests passed on my laptop so I'll extend the timeout of the newly added testcase to 30 secs just in case.

@SparkQA
Copy link

SparkQA commented Jun 30, 2020

Test build #124682 has finished for PR 28939 at commit 2a47155.

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

@sarutak
Copy link
Member Author

sarutak commented Jul 2, 2020

cc: @LucaCanali

@LucaCanali
Copy link
Contributor

This appears to have issues with Kubernetes in my tests, when adding --jars it throws "Unable to create executor due to ".

@sarutak
Copy link
Member Author

sarutak commented Jul 2, 2020

@LucaCanali Thank you for the feedback. I'll look into the cause.

@SparkQA
Copy link

SparkQA commented Jul 5, 2020

Test build #124941 has started for PR 28939 at commit 21720ac.

@sarutak
Copy link
Member Author

sarutak commented Jul 6, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Jul 6, 2020

Test build #125113 has started for PR 28939 at commit 21720ac.

@sarutak
Copy link
Member Author

sarutak commented Jul 7, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Jul 7, 2020

Test build #125260 has started for PR 28939 at commit 21720ac.

@sarutak
Copy link
Member Author

sarutak commented Jul 9, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Jul 9, 2020

Test build #125458 has started for PR 28939 at commit 21720ac.

@sarutak
Copy link
Member Author

sarutak commented Jul 9, 2020

Hmm, how flaky it is...

@sarutak
Copy link
Member Author

sarutak commented Jul 9, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32046/

@mridulm
Copy link
Contributor

mridulm commented Aug 14, 2020

This looks good to me - once Tom's comment is addressed.

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Test build #127425 has finished for PR 28939 at commit 5d65caf.

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

@sarutak
Copy link
Member Author

sarutak commented Aug 14, 2020

This looks good to me - once Tom's comment is addressed.

Oops, I've forgotten to remove the unused code. Thanks.

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32059/

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Test build #127438 has finished for PR 28939 at commit e57d053.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32059/

@sarutak
Copy link
Member Author

sarutak commented Aug 14, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32066/

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32066/

@sarutak
Copy link
Member Author

sarutak commented Aug 14, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32070/

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Test build #127445 has finished for PR 28939 at commit e57d053.

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

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32070/

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Test build #127449 has finished for PR 28939 at commit e57d053.

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

@sarutak
Copy link
Member Author

sarutak commented Aug 14, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32078/

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32078/

@tgravescs
Copy link
Contributor

test this please

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32080/

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Test build #127457 has finished for PR 28939 at commit e57d053.

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

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/32080/

@SparkQA
Copy link

SparkQA commented Aug 14, 2020

Test build #127459 has finished for PR 28939 at commit e57d053.

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

@asfgit asfgit closed this in 1a4c8f7 Aug 14, 2020
@mridulm
Copy link
Contributor

mridulm commented Aug 14, 2020

Thanks @sarutak !

sarutak added a commit to sarutak/spark that referenced this pull request Sep 1, 2020
…er and Kubernetes with --jars

### What changes were proposed in this pull request?

This PR changes Executor to load jars and files added by --jars and --files on Executor initialization.
To avoid downloading those jars/files twice, they are assosiated with `startTime` as their uploaded timestamp.

### Why are the changes needed?

ExecutorPlugin can't work with Standalone Cluster and Kubernetes
when a jar which contains plugins and files used by the plugins are added by --jars and --files option with spark-submit.

This is because jars and files added by --jars and --files are not loaded on Executor initialization.
I confirmed it works with YARN because jars/files are distributed as distributed cache.

### Does this PR introduce _any_ user-facing change?

Yes. jars/files added by --jars and --files are downloaded on each executor on initialization.

### How was this patch tested?

Added a new testcase.

Closes apache#28939 from sarutak/fix-plugin-issue.

Authored-by: Kousuke Saruta <sarutak@oss.nttdata.com>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
holdenk pushed a commit to holdenk/spark that referenced this pull request Oct 27, 2020
…er and Kubernetes with --jars

This PR changes Executor to load jars and files added by --jars and --files on Executor initialization.
To avoid downloading those jars/files twice, they are assosiated with `startTime` as their uploaded timestamp.

ExecutorPlugin can't work with Standalone Cluster and Kubernetes
when a jar which contains plugins and files used by the plugins are added by --jars and --files option with spark-submit.

This is because jars and files added by --jars and --files are not loaded on Executor initialization.
I confirmed it works with YARN because jars/files are distributed as distributed cache.

Yes. jars/files added by --jars and --files are downloaded on each executor on initialization.

Added a new testcase.

Closes apache#28939 from sarutak/fix-plugin-issue.

Authored-by: Kousuke Saruta <sarutak@oss.nttdata.com>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants