Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature-3575][*] Force Task Success #4267

Merged
merged 66 commits into from
Jan 10, 2021
Merged

[Feature-3575][*] Force Task Success #4267

merged 66 commits into from
Jan 10, 2021

Conversation

chengshiwen
Copy link
Member

@chengshiwen chengshiwen commented Dec 20, 2020

Tips

What is the purpose of the pull request

New Feature: Force Task Success #3726 #3575

  • Only add the ExecutionStatus FORCED_SUCCESS
  • Remove the operation ExecuteType resume_from_forced_success

this closes #3726, closes #3575, closes #196

Brief change log

Verify this pull request

(Please pick either of the following options)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added dolphinscheduler-api tests in TaskInstanceControllerTest.java and TaskInstanceServiceTest.java for end-to-end.
  • Added dolphinscheduler-dao tests in TaskInstanceMapperTest.java for end-to-end.
  • Manually verified the change by testing locally for frontend to backend.

Gif Verify:
image

Video Verify: See Video Verify Link

legen618 and others added 30 commits July 22, 2020 10:08
1. 新添加executeType
2. 新添加对应的commandType
3. 修改controller对应的service部分
4. 补充processService中的verifyIsNeedCreateCommand的判断逻辑
1. 添加两个查询(mapper.java和mapper.xml)
2. 添加ProcessService中constructProcessInstance中对应的部分
1. 这样的话可以完整的构建整个dag,恢复之前的上下文;也没有任务可能重复执行的担忧
2. 不需要额外去处理process执行完之后的状态(主要是部分与整体的原因)
3. RecoverNodeIdList也不会重复
1. 失败任务强制成功后会继续submit后续结点
2. 失败重试的节点强制成功后会继续submit后面结点,并且停止retry
3. 对于失败的sub-process如果其中的结点强制成功了,在parent-process中不会有任何影响,只能等到结束后“从强制成功处继续执行”。
1. dataAnalysis
2. alertManager
3. businessTimeUtils的schedule选择
1. taskStateCount中添加了强制成功的状态
2. processStateCount中剔除强制成功的状态
regular update local dev
防止logger注入的问题
@codecov-io
Copy link

codecov-io commented Dec 20, 2020

Codecov Report

Merging #4267 (e3547a3) into dev (e2e6a77) will increase coverage by 0.02%.
The diff coverage is 55.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #4267      +/-   ##
============================================
+ Coverage     43.23%   43.26%   +0.02%     
- Complexity     3185     3193       +8     
============================================
  Files           471      471              
  Lines         21728    21796      +68     
  Branches       2626     2639      +13     
============================================
+ Hits           9395     9430      +35     
- Misses        11438    11469      +31     
- Partials        895      897       +2     
Impacted Files Coverage Δ Complexity Δ
...heduler/api/controller/TaskInstanceController.java 82.35% <0.00%> (-17.65%) 3.00 <0.00> (ø)
...he/dolphinscheduler/common/utils/VarPoolUtils.java 82.05% <0.00%> (ø) 7.00 <2.00> (ø)
...heduler/server/master/runner/MasterExecThread.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...dolphinscheduler/common/enums/ExecutionStatus.java 62.16% <66.66%> (+1.05%) 6.00 <0.00> (ø)
...uler/api/service/impl/DataAnalysisServiceImpl.java 98.91% <75.00%> (-1.09%) 30.00 <2.00> (+1.00) ⬇️
.../apache/dolphinscheduler/api/dto/TaskCountDto.java 79.16% <83.33%> (+1.38%) 7.00 <2.00> (+2.00)
...apache/dolphinscheduler/api/enums/ExecuteType.java 33.33% <100.00%> (ø) 1.00 <0.00> (ø)
.../org/apache/dolphinscheduler/api/enums/Status.java 100.00% <100.00%> (ø) 5.00 <0.00> (ø)
...phinscheduler/api/service/TaskInstanceService.java 98.38% <100.00%> (+0.71%) 15.00 <5.00> (+5.00)
...inscheduler/service/zk/CuratorZookeeperClient.java 60.97% <0.00%> (-4.88%) 7.00% <0.00%> (-1.00%)
... and 7 more

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 e2e6a77...e3547a3. Read the comment docs.

Copy link
Contributor

@break60 break60 left a comment

Choose a reason for hiding this comment

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

front end +1

Copy link
Member

@CalvinKirs CalvinKirs left a comment

Choose a reason for hiding this comment

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

+1

@@ -414,7 +414,7 @@ private void initTaskQueue() {
if (task.isConditionsTask() || DagHelper.haveConditionsAfterNode(task.getName(), dag)) {
continue;
}
if (task.getState().typeIsFailure() && !task.taskCanRetry()) {
if (task.getState().typeIsFailure() && !task.taskCanRetry() && !task.isConditionsTask()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

already judged by the above if.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I will revise it

@chengshiwen
Copy link
Member Author

I will remove the operation resume_from_forced_success and revise all code logic @lenboo @lgcareer @CalvinKirs @break60

@davidzollo
Copy link
Contributor

I will remove the operation resume_from_forced_success and revise all code logic @lenboo @lgcareer @CalvinKirs @break60

any progress?

@chengshiwen
Copy link
Member Author

I will remove the operation resume_from_forced_success and revise all code logic @lenboo @lgcareer @CalvinKirs @break60

any progress?

Not yet since preparing marriage propose in the past few days. I will finish it today or tomorrow.

@chengshiwen
Copy link
Member Author

chengshiwen commented Jan 7, 2021

The operation resume_from_forced_success has been removed. Please review @dailidong @CalvinKirs @lenboo @lgcareer @break60

The configuration of force-task-B is as follow:
image

Gif Verify:
image

Video Verify: See Video Verify Link

@sonarcloud
Copy link

sonarcloud bot commented Jan 7, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

52.1% 52.1% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1
great job, and the video gif is very clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants