Skip to content

Conversation

@ruanwenjun
Copy link
Member

Purpose of the pull request

close #15750

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:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@ruanwenjun ruanwenjun self-assigned this Mar 21, 2024
@ruanwenjun ruanwenjun added this to the 3.2.2 milestone Mar 21, 2024
@ruanwenjun ruanwenjun added the bug Something isn't working label Mar 21, 2024
@ruanwenjun ruanwenjun changed the title Fix delay execution task will never be dispatched [Bug] Fix delay execution task will never be dispatched Mar 21, 2024
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_fixDelayExecution branch from 8354eea to c654b00 Compare March 21, 2024 09:07
Copy link
Contributor

@caishunfeng caishunfeng left a comment

Choose a reason for hiding this comment

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

LGTM

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_fixDelayExecution branch from c654b00 to 0a56145 Compare March 22, 2024 02:04
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2024

Codecov Report

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

Project coverage is 39.08%. Comparing base (99a2461) to head (0178cc2).

❗ Current head 0178cc2 differs from pull request most recent head 261751f. Consider uploading reports for the commit 261751f to get more accurate results

Files Patch % Lines
...rator/BaseTaskExecuteRunnableDispatchOperator.java 0.00% 3 Missing ⚠️
...r/runner/GlobalTaskDispatchWaitingQueueLooper.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #15751      +/-   ##
============================================
- Coverage     39.08%   39.08%   -0.01%     
  Complexity     4851     4851              
============================================
  Files          1316     1316              
  Lines         44964    44965       +1     
  Branches       4809     4809              
============================================
- Hits          17574    17573       -1     
- Misses        25491    25492       +1     
- Partials       1899     1900       +1     

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

taskInstance.setState(TaskExecutionStatus.DELAY_EXECUTION);
taskInstanceDao.updateById(taskInstance);
log.info("Current taskInstance: {} is choose delay execution, delay time: {}/s, remainTime: {}/s",
log.info("Current taskInstance: {} is choose delay execution, delay time: {}/min, remainTime: {}/s",
Copy link
Member

@EricGao888 EricGao888 Mar 22, 2024

Choose a reason for hiding this comment

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

This looks a bit weird. Could we make the unit for delay time and remain time consistent? (all in second / all in minute)

Copy link
Member Author

Choose a reason for hiding this comment

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

it's OK to log min here, since the input is min.

@sonarqubecloud
Copy link

@ruanwenjun ruanwenjun merged commit 2e16907 into apache:dev Mar 22, 2024
@ruanwenjun ruanwenjun deleted the dev_wenjun_fixDelayExecution branch March 22, 2024 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Delay execution task will never be dispatched

4 participants