disttask: cancel subtask context if scheduled away #58615

tangenta · 2024-12-30T09:13:56Z

What problem does this PR solve?

Issue Number: close #58450

Problem Summary: See #58450 (comment)

What changed and how does it work?

Cancel subtask context if scheduled away.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

tiprow · 2024-12-30T09:14:13Z

Hi @tangenta. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

codecov · 2024-12-30T09:32:56Z

Codecov Report

Attention: Patch coverage is 57.89474% with 8 lines in your changes missing coverage. Please review.

Project coverage is 73.5652%. Comparing base (5934515) to head (6976bf3).
Report is 514 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #58615        +/-   ##
================================================
+ Coverage   73.0744%   73.5652%   +0.4908%     
================================================
  Files          1676       1677         +1     
  Lines        463356     471804      +8448     
================================================
+ Hits         338595     347084      +8489     
+ Misses       103913     103619       -294     
- Partials      20848      21101       +253

Flag	Coverage Δ
integration	`42.8587% <0.0000%> (?)`
unit	`72.5633% <57.8947%> (+0.0469%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`∅ <ø> (∅)`
parser	`∅ <ø> (∅)`
br	`49.1215% <ø> (+2.2114%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pkg/disttask/framework/taskexecutor/task_executor.go

D3Hunter

rest lgtm

pkg/disttask/framework/taskexecutor/task_executor.go

ti-chi-bot · 2025-01-06T08:29:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, wjhuang2016

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [D3Hunter,wjhuang2016]
~~pkg/disttask/OWNERS~~ [D3Hunter,wjhuang2016]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-01-06T08:29:47Z

[LGTM Timeline notifier]

Timeline:

2025-01-06 07:35:17.931307047 +0000 UTC m=+166261.220138752: ☑️ agreed by D3Hunter.
2025-01-06 08:29:46.383273544 +0000 UTC m=+169529.672105247: ☑️ agreed by wjhuang2016.

tiprow · 2025-01-06T08:48:14Z

@tangenta: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
fast_test_tiprow	`8681e00`	link	true	`/test fast_test_tiprow`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

…long-vector * commit '510d0037b18f258f505abc6cf13a8128563e9359': *: upgrade pd client to make sure tso client initiate successfully (#58752) ttl, test: scale TTL workers during the fault tests (#58750) planner: improve warning messages for unsupported HASH_JOIN hints (#58646) planner: prealloc the slices in the SplitCorColAccessCondFromFilters (#58785) ddl: supports non-unique global index (#58678) util/stmtsummary: add the network traffic related fields (#58101) var: enable `pd_enable_follower_handle_region` as default (#58385) statistics: refactor stats meta handling to use DeltaUpdate for multi-table support (#58657) parser: move 'model' to 'ast' pkg (#58704) statistics: add recover to protect background task (#58739) disttask: cancel subtask context if scheduled away (#58615) *: don't handle live updates of column size (#58596) *: fix a bug for default_authentication_plugin (2) (#58723) dupdetect: gRPC cancel should trigger retry (#58542) *: fix a bug for default_authentication_plugin (#57391) distsql: Fix backoff execution info inaccurate issue (#58707)

ti-chi-bot · 2025-04-10T08:21:34Z

In response to a cherrypick label: new pull request created to branch release-8.5: #60481.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <[email protected]>

close pingcap#58450

disttask: recreate step executor if step context is done

24015ed

ti-chi-bot bot added do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 30, 2024

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Dec 30, 2024

D3Hunter reviewed Dec 30, 2024

View reviewed changes

pkg/disttask/framework/taskexecutor/task_executor.go Outdated Show resolved Hide resolved

tangenta added 5 commits December 31, 2024 12:28

address comment

4a2dba9

fix TestCheckBalanceSubtask

6937ceb

fix TestTaskExecutorRun

b146bac

introduce subtask context and cancel it when subtask is scheduled away

d09240a

don't store subtask context cancel function

83c7c6a

tangenta changed the title ~~disttask: recreate step executor if step context is done~~ disttask: cancel subtask context if scheduled away Jan 6, 2025

D3Hunter approved these changes Jan 6, 2025

View reviewed changes

pkg/disttask/framework/taskexecutor/task_executor.go Outdated Show resolved Hide resolved

pkg/disttask/framework/taskexecutor/task_executor.go Outdated Show resolved Hide resolved

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 6, 2025

cancel contexts in order

8681e00

wjhuang2016 approved these changes Jan 6, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 6, 2025

fix TestCheckBalanceSubtask

6976bf3

ti-chi-bot bot merged commit c199ddf into pingcap:master Jan 7, 2025
22 checks passed

D3Hunter mentioned this pull request Jan 10, 2025

import into failed after injection ddl owner network partition #58827

Closed

Benjamin2037 added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Apr 10, 2025

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Apr 10, 2025

This is an automated cherry-pick of pingcap#58615

6d7f7c9

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Apr 10, 2025

disttask: cancel subtask context if scheduled away (#58615) #60481

Closed

13 tasks

D3Hunter pushed a commit to D3Hunter/tidb that referenced this pull request Jun 30, 2025

disttask: cancel subtask context if scheduled away (pingcap#58615)

b2d122c

close pingcap#58450

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

disttask: cancel subtask context if scheduled away #58615

disttask: cancel subtask context if scheduled away #58615

Uh oh!

tangenta commented Dec 30, 2024 •

edited

Loading

Uh oh!

tiprow bot commented Dec 30, 2024

Uh oh!

codecov bot commented Dec 30, 2024 •

edited

Loading

Uh oh!

Uh oh!

D3Hunter left a comment

Uh oh!

Uh oh!

Uh oh!

ti-chi-bot bot commented Jan 6, 2025

Uh oh!

ti-chi-bot bot commented Jan 6, 2025

Uh oh!

tiprow bot commented Jan 6, 2025

Uh oh!

Uh oh!

ti-chi-bot commented Apr 10, 2025

Uh oh!

Uh oh!

disttask: cancel subtask context if scheduled away #58615

disttask: cancel subtask context if scheduled away #58615

Uh oh!

Conversation

tangenta commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow bot commented Dec 30, 2024

Uh oh!

codecov bot commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

D3Hunter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ti-chi-bot bot commented Jan 6, 2025

Uh oh!

ti-chi-bot bot commented Jan 6, 2025

[LGTM Timeline notifier]

Uh oh!

tiprow bot commented Jan 6, 2025

Uh oh!

Uh oh!

ti-chi-bot commented Apr 10, 2025

Uh oh!

Uh oh!

tangenta commented Dec 30, 2024 •

edited

Loading

codecov bot commented Dec 30, 2024 •

edited

Loading