Skip to content

[Bug-7788][MasterServer] fix submit duplicate tasks sometimes when retry#7808

Merged
lenboo merged 4 commits into
apache:2.0.2-releasefrom
caishunfeng:fix_retry
Jan 6, 2022
Merged

[Bug-7788][MasterServer] fix submit duplicate tasks sometimes when retry#7808
lenboo merged 4 commits into
apache:2.0.2-releasefrom
caishunfeng:fix_retry

Conversation

@caishunfeng
Copy link
Copy Markdown
Contributor

@caishunfeng caishunfeng commented Jan 5, 2022

this pr close #7788

Purpose of the pull request

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

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:

@caishunfeng caishunfeng requested a review from lenboo January 5, 2022 06:13
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 5, 2022

Codecov Report

❗ No coverage uploaded for pull request base (2.0.2-release@1d3f77b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##             2.0.2-release    #7808   +/-   ##
================================================
  Coverage                 ?   31.33%           
  Complexity               ?     1557           
================================================
  Files                    ?      433           
  Lines                    ?    14802           
  Branches                 ?     1483           
================================================
  Hits                     ?     4638           
  Misses                   ?     9705           
  Partials                 ?      459           

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 1d3f77b...8796130. Read the comment docs.

Copy link
Copy Markdown
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@zhongjiajie zhongjiajie left a comment

Choose a reason for hiding this comment

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

Add NIT suggestion.

logger.warn("task was found in ready submit queue, task code:{}", taskInstance.getTaskCode());
return;
}
logger.info("add task to stand by list, task name: {}, task id:{}, task code:{}",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
logger.info("add task to stand by list, task name: {}, task id:{}, task code:{}",
logger.info("add task to stand by list, task name:{}, task id:{}, task code:{}",

Integer taskInstanceId = entry.getKey();
if (activeTaskProcessorMaps.containsKey(taskInstanceId)) {
TaskInstance latestTaskInstance = processService.findTaskInstanceById(taskInstanceId);
if (latestTaskInstance != null && !latestTaskInstance.getState().typeIsFailure()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it necessary to query task instance from db?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it necessary to query task instance from db?

Yes, because the task instance in memory was not up to date.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jan 6, 2022

Copy link
Copy Markdown
Contributor

@lenboo lenboo left a comment

Choose a reason for hiding this comment

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

+1

@lenboo lenboo merged commit ff3f571 into apache:2.0.2-release Jan 6, 2022
lenboo added a commit that referenced this pull request Jan 7, 2022
lenboo added a commit that referenced this pull request Jan 7, 2022
caishunfeng added a commit to caishunfeng/dolphinscheduler that referenced this pull request Jan 7, 2022
…try (apache#7808)

* [Bug-7788] fix submit duplicate tasks sometimes when retry

* add exist check when add task to standby list

* update

* put queue contain judge first

Co-authored-by: caishunfeng <534328519@qq.com>
caishunfeng added a commit that referenced this pull request Jan 7, 2022
…try (#7808) (#7866)

* [Bug-7788] fix submit duplicate tasks sometimes when retry

* add exist check when add task to standby list

* update

* put queue contain judge first

Co-authored-by: caishunfeng <534328519@qq.com>

Co-authored-by: caishunfeng <534328519@qq.com>
@caishunfeng caishunfeng deleted the fix_retry branch February 17, 2022 08:03
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