[Improvement-11857][Spark] Remove the spark version of spark task #11860

rickchengx · 2022-09-08T09:10:56Z

Purpose of the pull request

Currently, the spark version is misleading.

This is not the spark version that DS currently supports. E.g., DS can also run the spark task of SPARK-3.x.x.

The spark version selected by the user only determines the environment variables used and the final command executed as below:

And also there is a bug that spark-sql can only be executed by SPARK2, see #11721

So why not just remove the spark version, like other task
And use {SPARK_HOME}/bin/spark_submit and {SPARK_HOME}/bin/spark-sql

Brief change log

use {SPARK_HOME}/bin/spark_submit and {SPARK_HOME}/bin/spark-sql

Verify this pull request

manually tested

rickchengx · 2022-09-08T09:12:48Z

Related Doc will be modified after this PR is reviewed

SbloodyS · 2022-09-08T09:17:29Z

Related Doc will be modified after this PR is reviewed

Please add the docs in this PR. We suggest that the documents involved in the modification should be submitted together.

rickchengx · 2022-09-08T09:19:30Z

Related Doc will be modified after this PR is reviewed

Please add the docs in this PR. We suggest that the documents involved in the modification should be submitted together.

Sure, but there are many documents that need to be modified, maybe we can first review whether this PR is appropriate

SbloodyS · 2022-09-08T09:21:17Z

Related Doc will be modified after this PR is reviewed

Please add the docs in this PR. We suggest that the documents involved in the modification should be submitted together.

Sure, but there are many documents that need to be modified, maybe we can first review whether this PR is appropriate

That sound good to me.

fuchanghai · 2022-09-08T12:42:34Z

If there is only one version of spark on a machine, there is no problem keeping only one. Problems can arise if a machine has two versions. Of course, I don't know about spark, I just analyze this problem from a business perspective

fuchanghai · 2022-09-08T12:54:40Z

@SbloodyS may be this PR is conflict with #11720 ,According to the actual application scenario, it seems that the current PR should be supported. if ds want to support two different versions ,i support another

rickchengx · 2022-09-09T02:05:48Z

Hi, @fuchanghai, Thanks for the reply. I agree with your point.

I think that the current spark version on ui is kind of misleading. Users may be concerned that DS does not support spark 3.x.x. I have seen people asking whether DS supports spark3 , even if there is a description for this problem in the DS documentation (but users will not find this description easily) as below:

Even if DS needs to support multiple sparks, I don't think setting SPARK_HOME1 and SPARK_HOME2 is an elegant way. What if the user needs to support 3 or more sparks?
I think that using SPARK_HOME is an easy and efficient way without misunderstandings and some potential problems.

SbloodyS · 2022-09-09T02:09:50Z

@SbloodyS may be this PR is conflict with #11720 ,According to the actual application scenario, it seems that the current PR should be supported. if ds want to support two different versions ,i support another

I think it's better to use current environment management to manage different version of spark. This is the meaning of environmental management.

rickchengx · 2022-09-09T02:15:20Z

@SbloodyS may be this PR is conflict with #11720 ,According to the actual application scenario, it seems that the current PR should be supported. if ds want to support two different versions ,i support another

I think it's better to use current environment management to manage different version of spark. This is the meaning of environmental management.

@SbloodyS Thanks for your suggestion. I totally agree with you.

So DS uses SPARK_HOME/bin/spark-submit to build the command, and user can set the SPARK_HOME he wants in the environment.

SbloodyS · 2022-09-09T02:29:43Z

@SbloodyS may be this PR is conflict with #11720 ,According to the actual application scenario, it seems that the current PR should be supported. if ds want to support two different versions ,i support another

I think it's better to use current environment management to manage different version of spark. This is the meaning of environmental management.

@SbloodyS Thanks for your suggestion. I totally agree with you.

So DS uses SPARK_HOME/bin/spark-submit to build the command, and user can set the SPARK_HOME he wants in the environment.

Yes. That's what I mean.

rickchengx · 2022-09-13T06:39:07Z

Yes. That's what I mean.

Hi, @SbloodyS , @fuchanghai

Since #11721 is merged, I've rebased my PR and also modified the related docs.

So now this PR uses SPARK_HOME/bin/spark-submit or SPARK_HOME/bin/spark-sql to build the command, and user can set the SPARK_HOME he wants in the environment as below.

So the misleading spark version is removed, and different tasks can still use different spark versions through the environment.

codecov-commenter · 2022-09-13T06:51:09Z

Codecov Report

Merging #11860 (ad683c3) into dev (ad683c3) will not change coverage.
The diff coverage is n/a.

❗ Current head ad683c3 differs from pull request most recent head 64f3d2f. Consider uploading reports for the commit 64f3d2f to get more accurate results

@@            Coverage Diff            @@
##                dev   #11860   +/-   ##
=========================================
  Coverage     38.68%   38.68%           
  Complexity     4006     4006           
=========================================
  Files          1002     1002           
  Lines         37213    37213           
  Branches       4249     4249           
=========================================
  Hits          14394    14394           
  Misses        21186    21186           
  Partials       1633     1633

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

zhongjiajie

please rebase the latest code, our docs make some change
and can you also add some discription in both https://github.com/apache/dolphinscheduler/blob/4dca488cd50b4392d222167c01ae2a79fd295e77/dolphinscheduler-python/pydolphinscheduler/UPDATING.mdand https://github.com/apache/dolphinscheduler/blob/3aa9f2ea25ca42112141aad85140a72b0963e2c3/docs/docs/en/guide/upgrade/incompatible.md becaus this is incompatible change for users

zhongjiajie · 2022-09-16T06:41:57Z

docs/docs/en/guide/task/spark.md

+| Node Name                  | Set the name of the task. Node names within a workflow definition are unique.                                                                                                                                                 |
+| Run flag                   | Indicates whether the node can be scheduled normally. If it is not necessary to execute, you can turn on the prohibiting execution switch.                                                                                    |
+| Description                | Describes the function of this node.                                                                                                                                                                                          |
+| Task priority              | When the number of worker threads is insufficient, they are executed in order from high to low according to the priority, and they are executed according to the first-in, first-out principle when the priority is the same. |
+| Worker group               | The task is assigned to the machines in the worker group for execution. If Default is selected, a worker machine will be randomly selected for execution.                                                                     |
+| Task group name            | The group in Resources, if not configured, it will not be used.                                                                                                                                                               |
+| Environment Name           | Configure the environment in which to run the script.                                                                                                                                                                         |
+| Number of failed retries   | The number of times the task is resubmitted after failure. It supports drop-down and manual filling.                                                                                                                          |
+| Failure Retry Interval     | The time interval for resubmitting the task if the task fails. It supports drop-down and manual filling.                                                                                                                      |
+| Timeout alarm              | Check Timeout Alarm and Timeout Failure. When the task exceeds the "timeout duration", an alarm email will be sent and the task execution will fail.                                                                          |


It is seems we already change the general parameter into separate parameter files, can you rebase to latest and take a look?

rickchengx · 2022-09-19T03:17:52Z

please rebase the latest code, our docs make some change and can you also add some discription in both https://github.com/apache/dolphinscheduler/blob/4dca488cd50b4392d222167c01ae2a79fd295e77/dolphinscheduler-python/pydolphinscheduler/UPDATING.mdand https://github.com/apache/dolphinscheduler/blob/3aa9f2ea25ca42112141aad85140a72b0963e2c3/docs/docs/en/guide/upgrade/incompatible.md becaus this is incompatible change for users

Hi, @zhongjiajie , thanks for your comment. I've rebased my PR and add some descriptions in UPDATING.md and incompatible.md

sonarcloud · 2022-09-19T03:44:07Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

44.8% Coverage
0.0% Duplication

zhongjiajie

LGTM, thanks

zhongjiajie · 2022-09-21T12:28:23Z

@caishunfeng should we release this PR in version 3.1.0?

rickchengx requested review from songjianet, Amy0104, caishunfeng, SbloodyS, zhongjiajie, EricGao888, zhuangchong and kezhenxu94 as code owners September 8, 2022 09:10

SbloodyS assigned rickchengx Sep 8, 2022

SbloodyS added the improvement make more easy to user or prompt friendly label Sep 8, 2022

SbloodyS added this to the 3.1.0 milestone Sep 8, 2022

SbloodyS added the miss:docs missing documents in PR label Sep 8, 2022

github-actions bot added backend CI&CD UI ui and front end related labels Sep 8, 2022

rickchengx force-pushed the Improvement-11857 branch from 4fd4a6a to 40db3a0 Compare September 13, 2022 06:26

rickchengx requested a review from Tianqi-Dotes as a code owner September 13, 2022 06:26

github-actions bot added document Python labels Sep 13, 2022

rickchengx force-pushed the Improvement-11857 branch from 40db3a0 to 38338ba Compare September 14, 2022 03:20

zhongjiajie reviewed Sep 16, 2022

View reviewed changes

zhongjiajie added the incompatible label Sep 16, 2022

rickchengx added 2 commits September 19, 2022 10:41

[Improvement-11857][Spark] Remove the spark version of spark task

2380c40

update the docs related to spark version

64f3d2f

rickchengx force-pushed the Improvement-11857 branch from 38338ba to 64f3d2f Compare September 19, 2022 03:13

zhongjiajie approved these changes Sep 21, 2022

View reviewed changes

zhongjiajie merged commit 08a4c79 into apache:dev Sep 21, 2022

caishunfeng modified the milestones: 3.1.0, 3.2.0 Sep 21, 2022

xdu-chenrj pushed a commit to xdu-chenrj/dolphinscheduler that referenced this pull request Oct 13, 2022

[improve] Remove the spark version of spark task (apache#11860)

59bc3df

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improvement-11857][Spark] Remove the spark version of spark task #11860

[Improvement-11857][Spark] Remove the spark version of spark task #11860

rickchengx commented Sep 8, 2022 •

edited

Loading

rickchengx commented Sep 8, 2022

SbloodyS commented Sep 8, 2022

rickchengx commented Sep 8, 2022

SbloodyS commented Sep 8, 2022

fuchanghai commented Sep 8, 2022

fuchanghai commented Sep 8, 2022

rickchengx commented Sep 9, 2022 •

edited

Loading

SbloodyS commented Sep 9, 2022

rickchengx commented Sep 9, 2022

SbloodyS commented Sep 9, 2022

rickchengx commented Sep 13, 2022 •

edited

Loading

codecov-commenter commented Sep 13, 2022 •

edited

Loading

zhongjiajie left a comment

zhongjiajie Sep 16, 2022

rickchengx Sep 19, 2022

rickchengx commented Sep 19, 2022

sonarcloud bot commented Sep 19, 2022

zhongjiajie left a comment

zhongjiajie commented Sep 21, 2022

[Improvement-11857][Spark] Remove the spark version of spark task #11860

[Improvement-11857][Spark] Remove the spark version of spark task #11860

Conversation

rickchengx commented Sep 8, 2022 • edited Loading

Purpose of the pull request

Brief change log

Verify this pull request

rickchengx commented Sep 8, 2022

SbloodyS commented Sep 8, 2022

rickchengx commented Sep 8, 2022

SbloodyS commented Sep 8, 2022

fuchanghai commented Sep 8, 2022

fuchanghai commented Sep 8, 2022

rickchengx commented Sep 9, 2022 • edited Loading

SbloodyS commented Sep 9, 2022

rickchengx commented Sep 9, 2022

SbloodyS commented Sep 9, 2022

rickchengx commented Sep 13, 2022 • edited Loading

codecov-commenter commented Sep 13, 2022 • edited Loading

Codecov Report

zhongjiajie left a comment

Choose a reason for hiding this comment

zhongjiajie Sep 16, 2022

Choose a reason for hiding this comment

rickchengx Sep 19, 2022

Choose a reason for hiding this comment

rickchengx commented Sep 19, 2022

sonarcloud bot commented Sep 19, 2022

zhongjiajie left a comment

Choose a reason for hiding this comment

zhongjiajie commented Sep 21, 2022

rickchengx commented Sep 8, 2022 •

edited

Loading

rickchengx commented Sep 9, 2022 •

edited

Loading

rickchengx commented Sep 13, 2022 •

edited

Loading

codecov-commenter commented Sep 13, 2022 •

edited

Loading