[SPARK-29202][DEPLOY] Driver java options are not passed to driver process in Yarn client mode #25889

sandeep-katta · 2019-09-22T06:10:46Z

What changes were proposed in this pull request?

--driver-java-options is not passed to driver process if the user runs the application in Yarn client mode

Run the below command

./bin/spark-sql --master yarn \
--driver-java-options="-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5555"

In Spark 2.4.4

java ... -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5555
org.apache.spark.deploy.SparkSubmit --master yarn --conf spark.driver.extraJavaOptions=-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5555 ...

In Spark 3.0

java ...
org.apache.spark.deploy.SparkSubmit --master yarn --conf spark.driver.extraJavaOptions=-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5556 ...

This issue is caused by SPARK-28980

Why are the changes needed?

Corrected the isClientMode API implementation

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manually,

dongjoon-hyun · 2019-09-22T23:56:15Z

ok to test

dongjoon-hyun · 2019-09-22T23:56:33Z

cc @srowen

dongjoon-hyun · 2019-09-22T23:57:26Z

BTW, @sandeep-katta .
Please use a live version example in the PR description. For example, 2.4.4 is the best. 2.3.x became EOL already.

launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java

srowen · 2019-09-23T01:03:50Z

I'm not clear this is a bug. At least, this previously only applied to YARN cluster mode. See 6378d4b#diff-75e0f814aa3717db995fa701883dc4e1 . How would driver options be valid for YARN client mode?

sandeep-katta · 2019-09-23T01:24:34Z

earlier the conditions is if it is not cluster-mode

!userMaster.equals("yarn-cluster") && userDeployMode == null

earlier user can run application with by following ways
-- master yarn
--master yarn --deploy-mode cluster
--master yarn --deploy-mode client
--master yarn-cluster
--master yarn-client

Above condition is for case 4, now from Spark3 user can submit application using 1st 3 cases

srowen · 2019-09-23T01:57:11Z

OK, so the point is that an unspecified deploy mode should be considered client for all RMs? I'd agree with that given other code. It's not specific to YARN, right?

SparkQA · 2019-09-23T02:07:25Z

Test build #111175 has finished for PR 25889 at commit e4ef428.

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

sandeep-katta · 2019-09-23T02:10:34Z

OK, so the point is that an unspecified deploy mode should be considered client for all RMs? I'd agree with that given other code. It's not specific to YARN, right?

Yes it is applicable to all RM, and by default it is client mode

SparkQA · 2019-09-23T04:39:12Z

Test build #111181 has finished for PR 25889 at commit 32760a7.

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

srowen

OK by me

srowen · 2019-09-23T13:26:20Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

  }

+  @Test
+  public void testisClientMode() {


Very minor: testIsClientMode

SparkQA · 2019-09-24T07:05:02Z

Test build #111257 has finished for PR 25889 at commit ac8c36d.

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

sandeep-katta · 2019-09-24T07:09:09Z

retest this please

dongjoon-hyun · 2019-09-24T07:43:23Z

Retest this please.

dongjoon-hyun · 2019-09-24T07:53:56Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

+  @Test
+  public void testIsClientMode() {
+    // Default master is "local[*]"
+    SparkSubmitCommandBuilder launcher =


launcher -> builder.

dongjoon-hyun · 2019-09-24T07:54:12Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

+  public void testIsClientMode() {
+    // Default master is "local[*]"
+    SparkSubmitCommandBuilder launcher =
+            newCommandBuilder(Collections.emptyList());


Shall we make line 256 ~ 257 into one line?

dongjoon-hyun · 2019-09-24T07:54:38Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

+    SparkSubmitCommandBuilder launcher =
+            newCommandBuilder(Collections.emptyList());
+    assertTrue("By default application run in local mode",
+            launcher.isClientMode(Collections.EMPTY_MAP));


Indentation? We use 2-space indentation in Java file. Please see the other test case.

dongjoon-hyun · 2019-09-24T07:55:07Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

+    // --master yarn or it can be any RM
+    List<String> sparkSubmitArgs = Arrays.asList(
+            parser.MASTER,
+            "yarn");


List<String> sparkSubmitArgs = Arrays.asList(parser.MASTER, "yarn");

dongjoon-hyun · 2019-09-24T07:55:25Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

+            "yarn");
+    launcher = newCommandBuilder(sparkSubmitArgs);
+    assertTrue("By default deploy mode is client",
+            launcher.isClientMode(Collections.EMPTY_MAP));


assertTrue("By default deploy mode is client", builder.isClientMode(Collections.EMPTY_MAP));

dongjoon-hyun · 2019-09-24T07:56:23Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

+            parser.MASTER,
+            "mesos",
+            parser.DEPLOY_MODE,
+            "cluster");


sparkSubmitArgs = Arrays.asList(parser.MASTER, "mesos", parser.DEPLOY_MODE, "cluster");

dongjoon-hyun · 2019-09-24T07:56:44Z

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

+            parser.DEPLOY_MODE,
+            "cluster");
+    launcher = newCommandBuilder(sparkSubmitArgs);
+    assertTrue(!launcher.isClientMode(Collections.EMPTY_MAP));


assertTrue(! -> assertFalse(.

Finally, use Collections.emptyMap() instead of Collections.EMPTY_MAP. Collections.EMPTY_MAP gives us Unchecked assignment warning due to type mismatch. There are multiple instances.

SparkQA · 2019-09-24T10:18:26Z

Test build #111272 has finished for PR 25889 at commit ac8c36d.

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

SparkQA · 2019-09-24T11:09:30Z

Test build #111278 has finished for PR 25889 at commit 7b2497a.

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

dongjoon-hyun · 2019-09-24T15:52:44Z

Retest this please.

SparkQA · 2019-09-24T15:55:13Z

Test build #111296 has started for PR 25889 at commit 7b2497a.

sandeep-katta · 2019-09-24T16:48:43Z

retest this please

SparkQA · 2019-09-25T15:41:19Z

Test build #4884 has finished for PR 25889 at commit 7b2497a.

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

srowen

Yep, looks nicer.

dongjoon-hyun · 2019-09-25T15:50:15Z

launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java

    return userMaster == null ||
      "client".equals(userDeployMode) ||
-      (!userMaster.equals("yarn") && userDeployMode == null);
+      userDeployMode == null;


Sorry for the last minute request, but shall we change three lines into one-liner?

- return userMaster == null || - "client".equals(userDeployMode) || - (!userMaster.equals("yarn") && userDeployMode == null); + return userMaster == null || userDeployMode == null || "client".equals(userDeployMode);

SparkQA · 2019-09-26T07:05:02Z

Test build #111394 has finished for PR 25889 at commit 6f1ba43.

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

vanzin · 2019-09-26T18:10:58Z

retest this please

SparkQA · 2019-09-26T20:54:52Z

Test build #111441 has finished for PR 25889 at commit 6f1ba43.

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

dongjoon-hyun

+1, LGTM. Merged to master.

dongjoon-hyun · 2019-09-26T22:51:05Z

Thank you, @srowen and @vanzin , too!

driver java options are not passed to driver process in Yarn client mode

e4ef428

dongjoon-hyun added the DEPLOY label Sep 22, 2019

dongjoon-hyun reviewed Sep 22, 2019

View reviewed changes

launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java Outdated Show resolved Hide resolved

added test case

32760a7

srowen reviewed Sep 23, 2019

View reviewed changes

update review comments

ac8c36d

dongjoon-hyun changed the title ~~[SPARK-29202][Deploy]driver java options are not passed to driver process in Yarn client mode~~ [SPARK-29202][Deploy] Driver java options are not passed to driver process in Yarn client mode Sep 24, 2019

dongjoon-hyun reviewed Sep 24, 2019

View reviewed changes

review comments fix

7b2497a

srowen approved these changes Sep 25, 2019

View reviewed changes

dongjoon-hyun reviewed Sep 25, 2019

View reviewed changes

changed the condition to 1 liner

6f1ba43

dongjoon-hyun changed the title ~~[SPARK-29202][Deploy] Driver java options are not passed to driver process in Yarn client mode~~ [SPARK-29202][CORE] Driver java options are not passed to driver process in Yarn client mode Sep 26, 2019

dongjoon-hyun changed the title ~~[SPARK-29202][CORE] Driver java options are not passed to driver process in Yarn client mode~~ [SPARK-29202][DEPLOY] Driver java options are not passed to driver process in Yarn client mode Sep 26, 2019

dongjoon-hyun approved these changes Sep 26, 2019

View reviewed changes

dongjoon-hyun closed this in 103de96 Sep 26, 2019

[SPARK-29202][DEPLOY] Driver java options are not passed to driver process in Yarn client mode #25889

[SPARK-29202][DEPLOY] Driver java options are not passed to driver process in Yarn client mode #25889

Uh oh!

Conversation

sandeep-katta commented Sep 22, 2019 • edited by dongjoon-hyun Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

dongjoon-hyun commented Sep 22, 2019

Uh oh!

dongjoon-hyun commented Sep 22, 2019

Uh oh!

dongjoon-hyun commented Sep 22, 2019

Uh oh!

Uh oh!

srowen commented Sep 23, 2019

Uh oh!

sandeep-katta commented Sep 23, 2019

Uh oh!

srowen commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

sandeep-katta commented Sep 23, 2019

Uh oh!

SparkQA commented Sep 23, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 24, 2019

Uh oh!

sandeep-katta commented Sep 24, 2019

Uh oh!

dongjoon-hyun commented Sep 24, 2019

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!

dongjoon-hyun Sep 24, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Sep 24, 2019 • 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 Sep 24, 2019

Uh oh!

SparkQA commented Sep 24, 2019

Uh oh!

dongjoon-hyun commented Sep 24, 2019

Uh oh!

SparkQA commented Sep 24, 2019

Uh oh!

sandeep-katta commented Sep 24, 2019

Uh oh!

SparkQA commented Sep 25, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 26, 2019

Uh oh!

sandeep-katta commented Sep 22, 2019 •

edited by dongjoon-hyun

Loading

dongjoon-hyun Sep 24, 2019 •

edited

Loading

dongjoon-hyun Sep 24, 2019 •

edited

Loading