[SPARK-31235][YARN] Separates different categories of applications by wang-zhun · Pull Request #28009 · apache/spark

wang-zhun · 2020-03-25T01:40:57Z

What changes were proposed in this pull request?

This PR adds spark.yarn.applicationType to identify the application type

Why are the changes needed?

The current application defaults to the SPARK type.
In fact, different types of applications have different characteristics and are suitable for different scenarios.For example: SPAKR-SQL, SPARK-STREAMING.
I recommend distinguishing them by the parameter spark.yarn.applicationType so that we can more easily manage and maintain different types of applications.

How was this patch tested?

1.add UT
2.Tested by verifying Yarn-UI ApplicationType in the following cases:

client and cluster mode

Need additional explanation:
limit cannot exceed 20 characters, can be empty or space
The reasons are as follows:

// org.apache.hadoop.yarn.server.resourcemanager.submitApplication.
 if (submissionContext.getApplicationType() == null) {
      submissionContext
        .setApplicationType(YarnConfiguration.DEFAULT_APPLICATION_TYPE);
} else {
      // APPLICATION_TYPE_LENGTH = 20
      if (submissionContext.getApplicationType().length() > YarnConfiguration.APPLICATION_TYPE_LENGTH) {
        submissionContext.setApplicationType(submissionContext
          .getApplicationType().substring(0,
            YarnConfiguration.APPLICATION_TYPE_LENGTH));
      }
    }

jiangxb1987 · 2020-03-25T18:36:43Z

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

+      "--archives", "archive1.txt,archive2.txt",
+      "--num-executors", "6",
+      "--name", "trill",
+      "--conf", "spark.yarn.applicationType=SPARK-SQL",


should you also test the case when spark.yarn.applicationType is not set?

I agree, what happens if empty string, any characters not allowed in it? Add tests and documentation to cover.

you didn't add tests for other like one empty string?
Also I believe the limit on that string is 20 characters, we should test and document that. Can you also test a space in the type?

tgravescs

You also need to update the docs/running-on-yarn.md document for the config

resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/config.scala

tgravescs · 2020-03-25T19:53:32Z

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

+      "--archives", "archive1.txt,archive2.txt",
+      "--num-executors", "6",
+      "--name", "trill",
+      "--conf", "spark.yarn.applicationType=SPARK-SQL",


I agree, what happens if empty string, any characters not allowed in it? Add tests and documentation to cover.

tgravescs · 2020-03-25T19:54:13Z

ok to test

SparkQA · 2020-03-25T22:17:17Z

Test build #120371 has finished for PR 28009 at commit ef705dc.

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

tgravescs · 2020-03-25T22:24:05Z

test this please

SparkQA · 2020-03-26T00:48:08Z

Test build #120377 has finished for PR 28009 at commit ef705dc.

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

wang-zhun · 2020-03-26T04:15:23Z

@jiangxb1987 @tgravescs Thanks for your responses and suggestions

SparkQA · 2020-03-26T04:16:15Z

Test build #120390 has finished for PR 28009 at commit 2064ce1.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

wang-zhun · 2020-03-26T04:23:33Z

test this please

SparkQA · 2020-03-26T10:03:19Z

Test build #120413 has finished for PR 28009 at commit c3aa425.

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

wang-zhun · 2020-03-27T05:27:09Z

Hi @jiangxb1987 @tgravescs , could you help to review this?

jiangxb1987 · 2020-03-27T05:29:32Z

Please add test case as Thomas suggested, thanks!

wang-zhun · 2020-03-27T05:54:16Z

Please add test case as Thomas suggested, thanks!
Ok sorry i didn't notice

SparkQA · 2020-03-29T07:05:02Z

Test build #120548 has finished for PR 28009 at commit 1744c48.

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

tgravescs · 2020-03-30T16:51:13Z

My comment was to add both a unit test case and a description. Please add the test cases - one with an empty string, 1 with a space in the string, 1 exceeding the 20 character limit.

SparkQA · 2020-04-07T17:38:28Z

Test build #120926 has finished for PR 28009 at commit 76e9a2a.

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

wang-zhun · 2020-04-28T09:25:37Z

@tgravescs help look at this PR.

tgravescs · 2020-04-28T13:43:34Z

test this please

SparkQA · 2020-04-28T14:09:13Z

Test build #122000 has finished for PR 28009 at commit 76e9a2a.

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

tgravescs · 2020-04-29T20:26:24Z

thanks for pinging me, I'll try to look by the end of the week

tgravescs · 2020-05-04T14:56:21Z

this is weird, I can't rerun the checks @dongjoon-hyun do you have permissions to tell this to rerun checks?

docs/running-on-yarn.md

resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/config.scala

resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala

SparkQA · 2020-05-05T04:03:04Z

Test build #122298 has finished for PR 28009 at commit 4599e18.

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

SparkQA · 2020-05-05T04:16:35Z

Test build #122299 has finished for PR 28009 at commit b762753.

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

tgravescs

changes look good, thanks @wang-zhun

tgravescs · 2020-05-05T13:41:50Z

merged to master

viirya · 2020-05-05T21:30:43Z

resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala

    appContext.getPriority.getPriority should be (1)
  }

+  test("specify a more specific type for the application") {


Is this test flaky? It failed twice in #26624.

tgravescs · 2020-05-05T21:43:02Z

yeah looks like its having issues with the yarn authorization provider for some reason

@wang-zhun can we change the test to just create the RMAppManager once and then reuse?

wang-zhun · 2020-05-06T02:34:35Z

@tgravescs ok, #28456

dongjoon-hyun · 2020-05-16T04:26:58Z

Hi, All. I made a follow-up PR. Until now, it looks promising.

[SPARK-31235][TESTS][FOLLOWUP][test-maven][test-hadoop3.2] Fix Hadoop-3.2 profile UT failure #28550

jiangxb1987 reviewed Mar 25, 2020

View reviewed changes

tgravescs reviewed Mar 25, 2020

View reviewed changes

wang-zhun requested a review from tgravescs March 27, 2020 04:56

tgravescs reviewed May 4, 2020

View reviewed changes

docs/running-on-yarn.md Show resolved Hide resolved

tgravescs reviewed May 4, 2020

View reviewed changes

resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/config.scala Show resolved Hide resolved

tgravescs reviewed May 4, 2020

View reviewed changes

resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala Outdated Show resolved Hide resolved

tgravescs reviewed May 4, 2020

View reviewed changes

resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala Outdated Show resolved Hide resolved

wang-zhun added 4 commits May 5, 2020 11:17

[SPARK-31235] Separates different categories of applications

17e89e2

Add tests and documentation to cover

14a61e3

fix

f8e411d

fix

b2127dd

wang-zhun added 3 commits May 5, 2020 11:17

fix

b496e82

Add test description

074917f

add the test cases

53de9a8

probot-autolabeler bot added INFRA ML R SQL labels May 5, 2020

fix

b762753

wang-zhun force-pushed the SPARK-31235 branch from 4599e18 to b762753 Compare May 5, 2020 03:52

probot-autolabeler bot added DOCS YARN labels May 5, 2020

tgravescs approved these changes May 5, 2020

View reviewed changes

asfgit closed this in f3891e3 May 5, 2020

viirya reviewed May 5, 2020

View reviewed changes

Conversation

wang-zhun commented Mar 25, 2020 • edited 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?

How was this patch tested?

Uh oh!

jiangxb1987 Mar 25, 2020

Choose a reason for hiding this comment

Uh oh!

tgravescs Mar 25, 2020

Choose a reason for hiding this comment

Uh oh!

tgravescs Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

tgravescs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tgravescs Mar 25, 2020

Choose a reason for hiding this comment

Uh oh!

tgravescs commented Mar 25, 2020

Uh oh!

SparkQA commented Mar 25, 2020

Uh oh!

tgravescs commented Mar 25, 2020

Uh oh!

SparkQA commented Mar 26, 2020

Uh oh!

wang-zhun commented Mar 26, 2020

Uh oh!

SparkQA commented Mar 26, 2020

Uh oh!

wang-zhun commented Mar 26, 2020

Uh oh!

SparkQA commented Mar 26, 2020

Uh oh!

wang-zhun commented Mar 27, 2020

Uh oh!

jiangxb1987 commented Mar 27, 2020

Uh oh!

wang-zhun commented Mar 27, 2020

Uh oh!

SparkQA commented Mar 29, 2020

Uh oh!

tgravescs commented Mar 30, 2020

Uh oh!

SparkQA commented Apr 7, 2020

Uh oh!

wang-zhun commented Apr 28, 2020

Uh oh!

tgravescs commented Apr 28, 2020

Uh oh!

SparkQA commented Apr 28, 2020

Uh oh!

tgravescs commented Apr 29, 2020

Uh oh!

tgravescs commented May 4, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented May 5, 2020

Uh oh!

SparkQA commented May 5, 2020

Uh oh!

tgravescs left a comment

Choose a reason for hiding this comment

Uh oh!

tgravescs commented May 5, 2020

Uh oh!

viirya May 5, 2020

Choose a reason for hiding this comment

Uh oh!

tgravescs commented May 5, 2020

Uh oh!

wang-zhun commented May 6, 2020

wang-zhun commented Mar 25, 2020 •

edited

Loading