[FIX-3900][server] kill multi yarn app in one job by Eights-Li · Pull Request #4042 · apache/dolphinscheduler

Eights-Li · 2020-11-09T14:18:20Z

What is the purpose of the pull request

fix #3900 kill multi yarn app in one job

Brief change log

modify server/utils/ProcessUtils.java cancelApplication method

Verify this pull request

server/utils/ProcessUtilsTest to verify this pr

…duler into dev

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java

lenboo

some code smell can be wiped out.

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java

lenboo

two magic number.

…duler into dev-kill-yarn-job

codecov-io · 2020-11-16T12:43:00Z

Codecov Report

Merging #4042 (802de19) into dev (f152cae) will increase coverage by 0.10%.
The diff coverage is 51.28%.

@@             Coverage Diff              @@
##                dev    #4042      +/-   ##
============================================
+ Coverage     39.39%   39.49%   +0.10%     
- Complexity     2968     2981      +13     
============================================
  Files           467      467              
  Lines         22104    22113       +9     
  Branches       2712     2714       +2     
============================================
+ Hits           8708     8734      +26     
+ Misses        12589    12570      -19     
- Partials        807      809       +2

Impacted Files	Coverage Δ	Complexity Δ
...he/dolphinscheduler/server/utils/ProcessUtils.java	`48.34% <48.29%> (+29.33%)`	`25.00 <25.00> (+17.00)`
.../org/apache/dolphinscheduler/common/Constants.java	`85.00% <100.00%> (ø)`	`1.00 <0.00> (ø)`
...che/dolphinscheduler/common/utils/LoggerUtils.java	`62.50% <0.00%> (-8.34%)`	`5.00% <0.00%> (-1.00%)`
...e/dolphinscheduler/common/shell/AbstractShell.java	`44.53% <0.00%> (-5.89%)`	`5.00% <0.00%> (-1.00%)`
...er/master/dispatch/host/assign/RandomSelector.java	`77.77% <0.00%> (-5.56%)`	`3.00% <0.00%> (-1.00%)`
...inscheduler/service/zk/CuratorZookeeperClient.java	`56.09% <0.00%> (-4.88%)`	`6.00% <0.00%> (-1.00%)`
...pache/dolphinscheduler/common/utils/FileUtils.java	`28.92% <0.00%> (-3.31%)`	`9.00% <0.00%> (ø%)`
.../apache/dolphinscheduler/common/utils/OSUtils.java	`44.57% <0.00%> (-2.41%)`	`23.00% <0.00%> (ø%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f152cae...802de19. Read the comment docs.

lenboo · 2020-11-17T07:58:21Z

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java

+                FileUtils.writeStringToFile(new File(commandFile), sb.toString(), StandardCharsets.UTF_8);
+            }
+
+            String runCmd = "sh " + commandFile;


is this "sh " can be instead?
I can find it elsewhere.

lenboo · 2020-11-17T08:01:57Z

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java

+                mat = MACPATTERN.matcher(pids);
+            }
+        } else {
+            String pids = OSUtils.exeCmd("pstree -p " + processId);


i think this "pstree " can be extracted to one variable, then I can easily replace this variable.

lenboo

other magic numbers exists.

…duler into dev-kill-yarn-job

lenboo

+1

lgcareer

+1

davidzollo · 2020-11-19T00:58:18Z

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java

+            }
+
+            logger.info("kill cmd:{}", runCmd);
+            Runtime.getRuntime().exec(runCmd);


please use OSUtils.exeCmd method instead，thx

davidzollo

don't use Runtime.getRuntime().exec directly, replace with OSUtils.exeCmd, thx

…duler into dev-kill-yarn-job

sonarqubecloud · 2020-11-20T13:56:23Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

62.7% Coverage
0.0% Duplication

Eights-Li · 2020-11-22T03:46:29Z

don't use Runtime.getRuntime().exec directly, replace with OSUtils.exeCmd, thx
please review this pr~

davidzollo

+1
good job

Eights-Li added 4 commits November 9, 2020 22:02

fix #3900 kill multi yarn app in one job

8fdd6f5

add ProcessUtils UT

2cad271

Merge branch 'dev' of https://github.com/apache/incubator-dolphinsche…

134cef3

…duler into dev

add ProcessUtils UT

1280155

lenboo reviewed Nov 12, 2020

View reviewed changes

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java Show resolved Hide resolved

lenboo reviewed Nov 12, 2020

View reviewed changes

modify ProcessUtils & ProcessUtilsTest

aac762a

Eights-Li requested review from lenboo and lgcareer November 13, 2020 09:12

lenboo reviewed Nov 16, 2020

View reviewed changes

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java Outdated Show resolved Hide resolved

lenboo reviewed Nov 16, 2020

View reviewed changes

...hinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java Show resolved Hide resolved

lenboo reviewed Nov 16, 2020

View reviewed changes

Eights-Li added 2 commits November 16, 2020 20:25

Merge branch 'dev' of https://github.com/apache/incubator-dolphinsche…

7844508

…duler into dev-kill-yarn-job

deal with magic value

cb669b5

Eights-Li requested a review from lenboo November 16, 2020 13:08

lenboo reviewed Nov 17, 2020

View reviewed changes

lenboo requested changes Nov 17, 2020

View reviewed changes

Eights-Li added 2 commits November 17, 2020 21:39

Merge branch 'dev' of https://github.com/apache/incubator-dolphinsche…

d3397b8

…duler into dev-kill-yarn-job

deal with magic value

c24dbc4

Eights-Li requested a review from lenboo November 17, 2020 14:26

lenboo approved these changes Nov 18, 2020

View reviewed changes

lgcareer approved these changes Nov 18, 2020

View reviewed changes

davidzollo reviewed Nov 19, 2020

View reviewed changes

davidzollo requested changes Nov 19, 2020

View reviewed changes

Eights-Li added 2 commits November 20, 2020 21:35

Merge branch 'dev' of https://github.com/apache/incubator-dolphinsche…

6672ac8

…duler into dev-kill-yarn-job

using OSUtils.execCmd when kill yarn app

802de19

Eights-Li requested a review from davidzollo November 20, 2020 13:43

davidzollo approved these changes Nov 22, 2020

View reviewed changes

davidzollo added this to the 1.3.4-release milestone Nov 22, 2020

davidzollo merged commit 368d3e8 into apache:dev Nov 22, 2020

Eights-Li deleted the dev-kill-yarn-app branch November 25, 2020 15:14

Conversation

Eights-Li commented Nov 9, 2020

What is the purpose of the pull request

Brief change log

Verify this pull request

Uh oh!

Uh oh!

lenboo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lenboo left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lenboo Nov 17, 2020

Choose a reason for hiding this comment

Uh oh!

lenboo Nov 17, 2020

Choose a reason for hiding this comment

Uh oh!

lenboo left a comment

Choose a reason for hiding this comment

Uh oh!

lenboo left a comment

Choose a reason for hiding this comment

Uh oh!

lgcareer left a comment

Choose a reason for hiding this comment

Uh oh!

davidzollo Nov 19, 2020

Choose a reason for hiding this comment

Uh oh!

Eights-Li Nov 20, 2020

Choose a reason for hiding this comment

Uh oh!

davidzollo left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 20, 2020

Uh oh!

Eights-Li commented Nov 22, 2020

Uh oh!

davidzollo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-io commented Nov 16, 2020 •

edited

Loading