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

fix MasterBaseTaskExecThread submit method bug #1532

Merged
merged 8 commits into from
Dec 24, 2019
Merged

fix MasterBaseTaskExecThread submit method bug #1532

merged 8 commits into from
Dec 24, 2019

Conversation

Technoboy-
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Dec 20, 2019

Codecov Report

Merging #1532 into dev will increase coverage by 0.38%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##             dev   #1532      +/-   ##
========================================
+ Coverage    7.4%   7.78%   +0.38%     
========================================
  Files        272     272              
  Lines      13643   13646       +3     
  Branches    2170    2168       -2     
========================================
+ Hits        1010    1063      +53     
+ Misses     12566   12511      -55     
- Partials      67      72       +5
Impacted Files Coverage Δ
...ler/server/master/runner/MasterTaskExecThread.java 0% <0%> (ø) ⬆️
...server/master/runner/MasterBaseTaskExecThread.java 0% <0%> (ø) ⬆️
...pache/dolphinscheduler/common/utils/FileUtils.java 18.47% <0%> (+1.64%) ⬆️
...apache/dolphinscheduler/api/enums/ExecuteType.java 66.66% <0%> (+66.66%) ⬆️
.../dolphinscheduler/server/utils/SparkArgsUtils.java 92.15% <0%> (+92.15%) ⬆️
...ache/dolphinscheduler/common/enums/ZKNodeType.java 100% <0%> (+100%) ⬆️

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 99fc87f...16c29a0. Read the comment docs.

@@ -74,6 +74,9 @@ public TaskInstance getTaskInstance(){
public Boolean submitWaitComplete() {
Boolean result = false;
this.taskInstance = submit();
if(this.taskInstance == null){
return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

should this return be checked whether the process instance is can be continue normally?
i am not sure about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qiaozhanwei how about your idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we send alert here if submit failed?

Copy link
Contributor

Choose a reason for hiding this comment

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

1,Uncertain return, it is recommended to debug
2,submit failed should be logs and mail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I debug for submit fail test, process will end and be a failure state.
it is satisfied the result.

Copy link
Contributor

@lgcareer lgcareer left a comment

Choose a reason for hiding this comment

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

+1

Copy link
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

@@ -74,6 +74,9 @@ public TaskInstance getTaskInstance(){
public Boolean submitWaitComplete() {
Boolean result = false;
this.taskInstance = submit();
if(this.taskInstance == null){
return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

1,Uncertain return, it is recommended to debug
2,submit failed should be logs and mail

@qiaozhanwei qiaozhanwei merged commit 26d9893 into apache:dev Dec 24, 2019
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.

None yet

5 participants