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.

Sign up for GitHub

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

Jump to bottom

[Bug] Fix dependent task logic #15795

Merged
merged 1 commit into from
Apr 3, 2024
Merged

[Bug] Fix dependent task logic #15795

merged 1 commit into from
Apr 3, 2024

Conversation

abzymeinsjtu
Copy link
Contributor

@abzymeinsjtu abzymeinsjtu commented Apr 2, 2024 •
edited by EricGao888

For details, see - #15707

However, in #15712, only one scenario - when instance triggered manually is fixed. This PR fix the scenario when the instance is triggered by scheduler or backfill.

@abzymeinsjtu abzymeinsjtu force-pushed the fix_dependent_task_wait_for_logic branch from 7530531 to 23407ee Compare April 2, 2024 14:01
EricGao888
EricGao888 previously approved these changes Apr 2, 2024
View reviewed changes
Copy link
Member

@EricGao888 EricGao888 left a comment

Choose a reason for hiding this comment

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

LGTM, would u like to double check? @ruanwenjun @calvinjiang

SbloodyS
SbloodyS reviewed Apr 2, 2024
View reviewed changes
Copy link
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.

Please link this pr to your issue and describe it.

...uler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapperTest.java Outdated Show resolved Hide resolved
@EricGao888
Copy link
Member

Please link this pr to your issue and describe it.

@SbloodyS This is a bug fix PR following up https://github.com/apache/dolphinscheduler/pull/15712/files

For details, see - #15707

However, in #15712, only one scenario - when instance triggered manually is fixed. This PR fix the scenario when the instance is triggered by scheduler or backfill.

@EricGao888 EricGao888 changed the title fix dependent task logic [Bug] Fix dependent task logic Apr 2, 2024
@SbloodyS
Copy link
Member

SbloodyS commented Apr 2, 2024

Please link this pr to your issue and describe it.

@SbloodyS This is a bug fix PR following up https://github.com/apache/dolphinscheduler/pull/15712/files

For details, see - #15707

However, in #15712, only one scenario - when instance triggered manually is fixed. This PR fix the scenario when the instance is triggered by scheduler or backfill.

Ok. I got it.

@SbloodyS SbloodyS added the 3.2.2 label Apr 2, 2024
@SbloodyS SbloodyS added the bug Something isn't working label Apr 2, 2024
@SbloodyS SbloodyS added this to the 3.2.2 milestone Apr 2, 2024
@abzymeinsjtu abzymeinsjtu force-pushed the fix_dependent_task_wait_for_logic branch from 23407ee to d8f11cf Compare April 2, 2024 14:21
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2024 •
edited

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 39.04%. Comparing base (0419543) to head (611478f).

❗ Current head 611478f differs from pull request most recent head 5e7fe2e. Consider uploading reports for the commit 5e7fe2e to get more accurate results

Files Patch % Lines
...er/dao/repository/impl/ProcessInstanceDaoImpl.java 0.00% 1 Missing ⚠️
...cheduler/server/master/utils/DependentExecute.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##                dev   #15795      +/-   ##
============================================
- Coverage     39.11%   39.04%   -0.07%     
+ Complexity     4887     4866      -21     
============================================
  Files          1326     1326              
  Lines         45206    45206              
  Branches       4818     4818              
============================================
- Hits          17683    17652      -31     
- Misses        25635    25666      +31     
  Partials       1888     1888

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

EricGao888
EricGao888 previously approved these changes Apr 3, 2024
View reviewed changes
@abzymeinsjtu abzymeinsjtu force-pushed the fix_dependent_task_wait_for_logic branch from d8f11cf to 5e7fe2e Compare April 3, 2024 04:02
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Apr 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@EricGao888 EricGao888 requested a review from SbloodyS April 3, 2024 05:35
SbloodyS
SbloodyS approved these changes Apr 3, 2024
View reviewed changes
Copy link
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.

+1

ruanwenjun
ruanwenjun approved these changes Apr 3, 2024
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

+1

@EricGao888 EricGao888 merged commit c0435e5 into apache:dev Apr 3, 2024
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rickchengx rickchengx rickchengx left review comments

@SbloodyS SbloodyS SbloodyS approved these changes

@ruanwenjun ruanwenjun ruanwenjun approved these changes

@EricGao888 EricGao888 EricGao888 approved these changes

@caishunfeng caishunfeng Awaiting requested review from caishunfeng caishunfeng is a code owner

Assignees

@abzymeinsjtu abzymeinsjtu

Labels
3.2.2 backend bug Something isn't working ready-to-merge
Projects
None yet
Milestone
3.2.2
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@abzymeinsjtu @EricGao888 @SbloodyS @codecov-commenter @ruanwenjun @rickchengx