Skip to content

Process shows success,when the task of the process is killed in the terminal#2635

Merged
qiaozhanwei merged 8 commits intoapache:devfrom
brave-lee:dev
May 11, 2020
Merged

Process shows success,when the task of the process is killed in the terminal#2635
qiaozhanwei merged 8 commits intoapache:devfrom
brave-lee:dev

Conversation

@brave-lee
Copy link
Copy Markdown
Contributor

@brave-lee brave-lee commented May 8, 2020

重现步骤:
1、有两个工作流AA和BB,BB依赖AA。
2、AA中有test1和test2任务,test2任务依赖test1。
3、BB中有dependent和test3任务,dependent配置AA的test2,test3依赖dependent。
4、现在手工运行AA,(test1任务可以配置shell,shell中运行一个test1.sh文件,该文件中可以写个sleep 60s),在执行worker的终端kill掉该任务(这种情况是手工kill,实际生产中服务器压力大时很有可能被其他服务kill掉)。
5、此时显示工作流AA运行成功,AA中test1任务kill状态,test2任务没有运行。
6、手工运行BB,可以看到BB中dependent和test3任务都运行成功。

正确的场景是AA中test1被kill后,AA应该是失败状态而不是成功状态,BB因dependent失败而失败

Reproduction steps:

  1. There are two workflows AA and BB, and BB depends on AA.
  2. There are test1 and test2 tasks in AA, and test2 tasks depend on test1.
  3. There are DEPENDENCT and test3 tasks in BB. DEPENDENCT configures test2 of AA, and test3 depends on the DEPENDENCT.
  4. Now run AA manually (test1 task can configure the shell, and a test1.sh file can be run in the shell, and a sleep 60s can be written in the file). Kill the task at the terminal executing the worker (in this case, kill manually. In actual production, when the server is under great pressure, it is likely to be killed by other services).
  5. At this time, the workflow AA runs successfully, the test1 task in AA is in kill status, and the test2 task is not running.
  6. Run BB manually, and you can see that the DEPENDENCT and test3 tasks in BB run successfully.

I think the correct scenario is that after test1 in AA is killed, AA should be in failure state instead of success state. BB fails due to the DEPENDENCT failure

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 8, 2020

Codecov Report

Merging #2635 into dev will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #2635      +/-   ##
============================================
- Coverage     36.16%   36.12%   -0.05%     
+ Complexity     2472     2469       -3     
============================================
  Files           431      431              
  Lines         19879    19879              
  Branches       2419     2419              
============================================
- Hits           7190     7182       -8     
- Misses        12048    12053       +5     
- Partials        641      644       +3     
Impacted Files Coverage Δ Complexity Δ
...dolphinscheduler/common/enums/ExecutionStatus.java 54.83% <0.00%> (ø) 2.00 <0.00> (ø)
...he/dolphinscheduler/api/service/LoggerService.java 82.60% <0.00%> (-8.70%) 9.00% <0.00%> (-1.00%)
...he/dolphinscheduler/common/thread/ThreadUtils.java 74.60% <0.00%> (-3.18%) 14.00% <0.00%> (ø%)
...e/dolphinscheduler/remote/NettyRemotingClient.java 51.07% <0.00%> (-2.88%) 9.00% <0.00%> (-2.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 cb52853...c62358a. Read the comment docs.

@davidzollo davidzollo requested a review from qiaozhanwei May 8, 2020 07:16
@lenboo
Copy link
Copy Markdown
Contributor

lenboo commented May 8, 2020

please add a detailed description of the problem on the title and content.

@brave-lee brave-lee changed the title fix #2634 fix #2634 Process shows success,when the task of the process is killed in the terminal May 8, 2020
@brave-lee brave-lee changed the title fix #2634 Process shows success,when the task of the process is killed in the terminal Process shows success,when the task of the process is killed in the terminal May 8, 2020
@lenboo
Copy link
Copy Markdown
Contributor

lenboo commented May 8, 2020

we have the situation:
you can kill the task by 'stop' button. then the killed task can't be judged a failure.

@brave-lee
Copy link
Copy Markdown
Contributor Author

First of all, the status of the task is different from that of the terminal kill through the stop button. Secondly, in order to simulate the situation that tasks are killed by other services in production, the terminal kill

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Copy Markdown
Contributor

@qiaozhanwei qiaozhanwei left a comment

Choose a reason for hiding this comment

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

+1

@qiaozhanwei qiaozhanwei merged commit daea87a into apache:dev May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants