Skip to content

[Improvement-17843][Master] Add IT case for task timeout alert#18001

Merged
SbloodyS merged 41 commits intoapache:devfrom
shrihari7396:feature/17843-task-timeout-alert-it
Apr 19, 2026
Merged

[Improvement-17843][Master] Add IT case for task timeout alert#18001
SbloodyS merged 41 commits intoapache:devfrom
shrihari7396:feature/17843-task-timeout-alert-it

Conversation

@shrihari7396
Copy link
Copy Markdown
Contributor

@shrihari7396 shrihari7396 commented Feb 25, 2026 •
edited
Loading

Purpose

Closes #17843

Adds integration test cases to verify task timeout alert behavior during workflow execution in the master module.

Changes

  • Added integration test cases:

    • testStartWorkflow_withTimeoutWarnTask
    • testStartWorkflow_withTimeoutWarnFailedTask

  • Added YAML workflow definitions for timeout scenarios

  • Added repository method to query alerts from the database

  • Updated master module to support timeout alert verification in tests

Behavior

Before:

  • No integration tests to verify timeout alert behavior
  • Timeout alerts were not automatically validated

After:

  • Integration tests validate timeout alert generation
  • Alerts are verified to be correctly generated and persisted in the database

Testing

  • Ran integration tests:

    • WorkflowStartTestCase#testStartWorkflow_withTimeoutWarnTask
    • WorkflowStartTestCase#testStartWorkflow_withTimeoutWarnFailedTask

  • Verified:

    • Workflow execution with timeout configurations
    • Task and workflow states
    • Alert generation and persistence in the database

@boring-cyborg
Copy link
Copy Markdown

boring-cyborg Bot commented Feb 25, 2026

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

SbloodyS
SbloodyS requested changes Feb 26, 2026
View reviewed changes
Comment thread .../java/org/apache/dolphinscheduler/server/master/integration/cases/WorkflowStartTestCase.java Outdated
@shrihari7396 shrihari7396 force-pushed the feature/17843-task-timeout-alert-it branch from 2361ef0 to c6f6ed8 Compare February 26, 2026 04:44
@shrihari7396 shrihari7396 changed the title [Improvement][Master] Add IT case for task timeout alert [Improvement-17843][Master] Add IT case for task timeout alert Feb 26, 2026
@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Feb 26, 2026

Hi @SbloodyS,

Thank you for the review. This is my first contribution to this project, so I really appreciate your guidance.
I will fix the requested changes and update the PR shortly.

Thanks!

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Feb 26, 2026

Hi @SbloodyS,

The milestone-label-check is currently failing because no milestone and valid label are assigned to this PR.

Could you please help assign the appropriate milestone and label so the check can pass?

Thank you!

@SbloodyS
Copy link
Copy Markdown
Member

SbloodyS commented Feb 26, 2026

Hi @SbloodyS,

The milestone-label-check is currently failing because no milestone and valid label are assigned to this PR.

Could you please help assign the appropriate milestone and label so the check can pass?

Thank you!

Milestones are required before the merger, and contributors do not need to pay attention to them. You should check failed IT. It did not passed.

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Feb 26, 2026 •
edited
Loading

Hi @SbloodyS,

I would like to work on adding the integration test case for task timeout alert in the master module.

While preparing the environment, my IDE formatter modified some files unintentionally. I will revert any unrelated formatting changes and ensure only the relevant test case changes are included in the PR.

I will first analyze the current timeout handling and alert triggering flow to ensure the IT covers the correct execution path (timeout detection → task state update → alert creation).

Please let me know if there are any specific scenarios or timeout strategies (e.g., WARN vs FAILED) that you would like the test to cover.

Thanks!

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Feb 27, 2026

Hi @SbloodyS,

I have applied spotless formatting and updated the PR accordingly.

Thanks.

shrihari7396
shrihari7396 commented Feb 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@shrihari7396 shrihari7396 left a comment

Choose a reason for hiding this comment

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

Thanks for the review. I have reverted the unnecessary changes and updated the PR accordingly.

I also ensured the formatting issues are fixed.

Please let me know if any further modifications are needed.

Comment thread .../java/org/apache/dolphinscheduler/server/master/integration/cases/WorkflowStartTestCase.java Outdated
@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Feb 27, 2026

Hi @SbloodyS,

Just a gentle follow-up on this PR. I have addressed the requested changes.

Could you please take another look when you get time?

Thank you!

ruanwenjun
ruanwenjun requested changes Mar 1, 2026
View reviewed changes
Copy link
Copy Markdown
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.

You should do assertion of the alert event in DB.

@shrihari7396 shrihari7396 requested a review from ruanwenjun March 1, 2026 07:32
@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Mar 19, 2026

Hi @ruanwenjun @SbloodyS,

I’ve addressed the requested changes and updated the PR.
Could you please take another look when you have time?

Thanks!

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Mar 31, 2026

Hi @ruanwenjun, @SbloodyS,
I have updated the branch. Please let me know if any changes are needed.

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

Please resolve conflicts. @shrihari7396

@shrihari7396 shrihari7396 force-pushed the feature/17843-task-timeout-alert-it branch 3 times, most recently from 1cc9c99 to 9a8541c Compare April 6, 2026 13:08
@shrihari7396 shrihari7396 requested a review from SbloodyS April 6, 2026 17:38
@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Apr 6, 2026

Hi @ruanwenjun, @SbloodyS, I have resolved the conflicts during the branch merge. Please review and let me know if any changes are needed.

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Apr 7, 2026

Hi @ruanwenjun, @SbloodyS,
I have updated the branch. Please let me know if any changes are needed.

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Apr 7, 2026 •
edited
Loading

@ruanwenjun @SbloodyS
All PR checks have completed successfully. The only remaining one is the milestone-labeled check. Could you please take a look or let me know if anything is required from my side?

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Apr 9, 2026

Hi @ruanwenjun, @SbloodyS,
I have updated the branch. Please let me know if any changes are needed.

@shrihari7396
Copy link
Copy Markdown
Contributor Author

shrihari7396 commented Apr 18, 2026

Hi @ruanwenjun, @SbloodyS,
I have updated the branch. Please let me know if any changes are needed.

ruanwenjun
ruanwenjun approved these changes Apr 19, 2026
View reviewed changes
Copy link
Copy Markdown
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.

LGTM

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 19, 2026

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 SonarQube Cloud

@SbloodyS SbloodyS added this to the 3.4.2 milestone Apr 19, 2026
@SbloodyS SbloodyS added the improvement make more easy to user or prompt friendly label Apr 19, 2026
SbloodyS
SbloodyS approved these changes Apr 19, 2026
View reviewed changes
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.

+1

@SbloodyS SbloodyS merged commit 91b75ee into apache:dev Apr 19, 2026
124 of 125 checks passed
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg Bot commented Apr 19, 2026

Awesome work, congrats on your first merged pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SbloodyS SbloodyS SbloodyS approved these changes

@ruanwenjun ruanwenjun ruanwenjun approved these changes

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

Assignees

@shrihari7396 shrihari7396

Labels

backend improvement make more easy to user or prompt friendly test

Projects

None yet

Milestone

3.4.2

Development

Successfully merging this pull request may close these issues.

[Improvement][Master] Add IT case for task timeout alert

3 participants

@shrihari7396 @SbloodyS @ruanwenjun