pkg/dxf: avoid misleading error log on context cancel by D3Hunter · Pull Request #67612 · pingcap/tidb

D3Hunter · 2026-04-08T08:35:32Z

What problem does this PR solve?

Issue Number: ref #67631

Problem Summary:

When a distributed task subtask exits because the context is canceled, the executor currently logs it as a failure. This produces misleading error logs for expected cancellation paths.

What changed and how does it work?

In BaseTaskExecutor.Run, handle runSubtask errors by type:

If the error is a context-canceled error (llog.IsContextCanceledError(err)), log an info message.
Otherwise keep logging the error as before.

This preserves error visibility for real failures while reducing noisy/misleading logs for expected cancellations.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test. just changing log printing
- 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

None

Summary by CodeRabbit

Bug Fixes
- Reduced log noise for task execution: subtasks terminated due to context cancellation are now logged as informational messages rather than errors, making logs clearer and avoiding misleading error entries.

pantheon-ai · 2026-04-08T08:35:40Z

@D3Hunter I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details.

⏳ This process typically takes 10-30 minutes depending on the complexity of the changes.

_{ℹ️ Learn more details on Pantheon AI.}

tiprow · 2026-04-08T08:35:52Z

Hi @D3Hunter. 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.

Details

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.

coderabbitai · 2026-04-08T08:35:58Z

📝 Walkthrough

Walkthrough

BaseTaskExecutor.Run now logs errors from runSubtask differently: context-cancellation errors (detected via llog.IsContextCanceledError) are logged at Info using llog.ShortError(err); all other errors continue to be logged as Error with zap.Error(err).

Changes

Cohort / File(s)	Summary
Context-Cancellation Error Handling `pkg/dxf/framework/taskexecutor/task_executor.go`	In `BaseTaskExecutor.Run`, distinguish `runSubtask` failures: log context-cancellation errors at Info using `llog.ShortError(err)`; preserve Error logging (`zap.Error(err)`) for non-cancellation errors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

pkg/dxf: sample high-frequency logs and summarize kv-range split #67513 — Modifies logging for runSubtask failures in the same file; routes similar error path through a sampled logger.

Suggested labels

size/L

Suggested reviewers

hawkingrei
joechenrh
GMHDBJD

Poem

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the package (pkg/dxf) and the specific change (avoiding misleading error logs on context cancellation), directly summarizing the main changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The pull request description follows the required template with all key sections completed: problem statement with issue reference, detailed explanation of changes, and appropriate test/documentation checkboxes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

pkg/dxf/framework/taskexecutor/task_executor.go (1)

394-399: Include cancellation details in the Info log.

At Line 396, the cancellation branch drops the error payload. Keep it at Info level, but include context (error and subtask ID) so cancellations remain diagnosable.

♻️ Suggested refinement

 		if err != nil {
 			// task executor keeps running its subtasks even though some subtask
 			// might have failed, we rely on scheduler to detect the error, and
 			// notify task executor or manager to cancel.
 			if llog.IsContextCanceledError(err) {
 				// to avoid the log being misleading when context is canceled.
-				e.logger.Info("meet context cancel when run subtask")
+				e.logger.Info("context canceled when running subtask",
+					zap.Int64("subtask-id", subtask.ID),
+					zap.Error(err))
 			} else {
 				e.logger.Error("run subtask failed", zap.Error(err))
 			}
 		} else {

As per coding guidelines, "Keep error handling actionable and contextual; avoid silently swallowing errors."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/dxf/framework/taskexecutor/task_executor.go` around lines 394 - 399, The
cancellation branch drops the error payload; update the
llog.IsContextCanceledError branch in the run-subtask handling (the code around
llog.IsContextCanceledError and e.logger.Info("meet context cancel when run
subtask")) to log the cancellation at Info level but include the error and
subtask identifier (e.g., subtask.ID or other subtask identifier variable) so
cancellations are diagnosable—replace the current e.logger.Info call with an
Info log that adds zap.Error(err) and a field for the subtask ID (or use
e.logger.InfoContext / With to attach context) while leaving non-cancellation
errors unchanged in e.logger.Error.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/dxf/framework/taskexecutor/task_executor.go`:
- Around line 394-399: The cancellation branch drops the error payload; update
the llog.IsContextCanceledError branch in the run-subtask handling (the code
around llog.IsContextCanceledError and e.logger.Info("meet context cancel when
run subtask")) to log the cancellation at Info level but include the error and
subtask identifier (e.g., subtask.ID or other subtask identifier variable) so
cancellations are diagnosable—replace the current e.logger.Info call with an
Info log that adds zap.Error(err) and a field for the subtask ID (or use
e.logger.InfoContext / With to attach context) while leaving non-cancellation
errors unchanged in e.logger.Error.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0ef1f38c-8a62-4ee9-b472-a1b1f2a13a00

📥 Commits

Reviewing files that changed from the base of the PR and between a69ecb8 and 6b875ac.

📒 Files selected for processing (1)

pkg/dxf/framework/taskexecutor/task_executor.go

codecov · 2026-04-08T08:53:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.6573%. Comparing base (a69ecb8) to head (4884fa5).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #67612        +/-   ##
================================================
+ Coverage   77.5859%   77.6573%   +0.0714%     
================================================
  Files          1980       1965        -15     
  Lines        547705     550852      +3147     
================================================
+ Hits         424942     427777      +2835     
- Misses       121953     123064      +1111     
+ Partials        810         11       -799

Flag	Coverage Δ
integration	`41.3247% <0.0000%> (+6.9850%)`	⬆️
unit	`76.9214% <100.0000%> (+0.5857%)`	⬆️

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

Components	Coverage Δ
dumpling	`61.5065% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`49.9700% <ø> (-10.5211%)`	⬇️

🚀 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.

D3Hunter · 2026-04-08T09:22:15Z

/retest

tiprow · 2026-04-08T09:22:39Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-08T10:00:40Z

/retest

tiprow · 2026-04-08T10:01:04Z

@D3Hunter: 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.

Details

In response to this:

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.

ingress-bot · 2026-04-08T11:11:00Z

🔍 Starting code review for this PR...

ingress-bot

Summary

Total findings: 3
Inline comments: 3
Summary-only findings (no inline anchor): 0

Findings (highest risk first)

ℹ️ [Info] (1)

Workaround comment explains logging effect but not cancellation contract (pkg/dxf/framework/taskexecutor/task_executor.go:395)

🧹 [Nit] (2)

Cancellation log wording diverges from existing domain term (pkg/dxf/framework/taskexecutor/task_executor.go:396, pkg/dxf/framework/taskexecutor/task_executor.go:766)
New cancellation log text introduces vocabulary drift (pkg/dxf/framework/taskexecutor/task_executor.go:396, pkg/dxf/framework/taskexecutor/task_executor.go:766)

D3Hunter · 2026-04-08T11:46:19Z

/retest

tiprow · 2026-04-08T11:46:44Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-08T13:07:44Z

/retest

tiprow · 2026-04-08T13:08:12Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-08T14:56:29Z

/retest

tiprow · 2026-04-08T14:56:58Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-09T03:52:58Z

/retest

tiprow · 2026-04-09T03:53:20Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-09T08:49:22Z

/retest

tiprow · 2026-04-09T08:49:46Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-09T08:49:57Z

/approve

ti-chi-bot · 2026-04-09T08:50:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, joechenrh, lance6716

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/dxf/OWNERS~~ [D3Hunter]

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

D3Hunter · 2026-04-09T10:32:57Z

/retest

tiprow · 2026-04-09T10:33:23Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-09T11:07:40Z

/retest

tiprow · 2026-04-09T11:08:04Z

@D3Hunter: 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.

Details

In response to this:

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.

hawkingrei · 2026-04-09T11:26:52Z

/retest

D3Hunter · 2026-04-09T12:10:33Z

/retest

tiprow · 2026-04-09T12:10:57Z

@D3Hunter: 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.

Details

In response to this:

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.

hawkingrei · 2026-04-09T12:31:45Z

/retest

D3Hunter · 2026-04-09T14:56:48Z

/retest

tiprow · 2026-04-09T14:57:15Z

@D3Hunter: 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.

Details

In response to this:

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.

hawkingrei · 2026-04-09T16:05:45Z

/retest

D3Hunter · 2026-04-09T17:27:50Z

/retest

tiprow · 2026-04-09T17:28:16Z

@D3Hunter: 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.

Details

In response to this:

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.

D3Hunter · 2026-04-10T02:57:07Z

/cherry-pick release-nextgen-202603

D3Hunter · 2026-04-10T02:57:11Z

/cherry-pick release-nextgen-20251011

ti-chi-bot · 2026-04-10T02:57:56Z

@D3Hunter: new pull request created to branch release-nextgen-202603: #67673.

Details

In response to this:

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 ti-community-infra/tichi repository.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2026-04-10T02:58:02Z

@D3Hunter: new pull request created to branch release-nextgen-20251011: #67674.
But this PR has conflicts, please resolve them!

Details

In response to this:

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 ti-community-infra/tichi repository.

ref #67631

ref pingcap#67631

change

6b875ac

ti-chi-bot Bot added the release-note-none Denotes a PR that doesn't merit a release note. label Apr 8, 2026

ti-chi-bot Bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 8, 2026

coderabbitai Bot reviewed Apr 8, 2026

View reviewed changes

change

2966b45

joechenrh approved these changes Apr 8, 2026

View reviewed changes

ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 8, 2026

print short err

50d82c4

ingress-bot reviewed Apr 8, 2026

View reviewed changes

Comment thread pkg/dxf/framework/taskexecutor/task_executor.go Outdated

Comment thread pkg/dxf/framework/taskexecutor/task_executor.go Outdated

Comment thread pkg/dxf/framework/taskexecutor/task_executor.go Outdated

pkg/dxf: clarify context-canceled subtask log contract

4884fa5

D3Hunter mentioned this pull request Apr 9, 2026

pkg/dxf: reduce noisy logs for hot loops and expected cancellation paths #67631

Closed

lance6716 approved these changes Apr 9, 2026

View reviewed changes

ti-chi-bot Bot added the lgtm label Apr 9, 2026

ti-chi-bot Bot added the approved label Apr 9, 2026

ti-chi-bot Bot merged commit 7968513 into pingcap:master Apr 9, 2026
35 checks passed

D3Hunter deleted the fix-log branch April 10, 2026 01:01

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

pkg/dxf: avoid misleading error log on context cancel (#67612) #67673

Merged

13 tasks

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

This is an automated cherry-pick of pingcap#67612

ade13f7

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

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

pkg/dxf: avoid misleading error log on context cancel (#67612) #67674

Merged

13 tasks

ti-chi-bot Bot pushed a commit that referenced this pull request Apr 10, 2026

pkg/dxf: avoid misleading error log on context cancel (#67612) (#67673)

744884d

ref #67631

ti-chi-bot Bot pushed a commit that referenced this pull request Apr 10, 2026

pkg/dxf: avoid misleading error log on context cancel (#67612) (#67674)

a31d609

ref #67631

premal pushed a commit to premal/tidb that referenced this pull request Apr 30, 2026

pkg/dxf: avoid misleading error log on context cancel (pingcap#67612)

c11dbbd

ref pingcap#67631

Conversation

D3Hunter commented Apr 8, 2026 • 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

Summary by CodeRabbit

Uh oh!

pantheon-ai Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tiprow Bot commented Apr 8, 2026

Uh oh!

coderabbitai Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

D3Hunter commented Apr 8, 2026

Uh oh!

tiprow Bot commented Apr 8, 2026

Uh oh!

D3Hunter commented Apr 8, 2026

Uh oh!

tiprow Bot commented Apr 8, 2026

Uh oh!

ingress-bot commented Apr 8, 2026

Uh oh!

ingress-bot left a comment

Choose a reason for hiding this comment

Summary

ℹ️ [Info] (1)

🧹 [Nit] (2)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

D3Hunter commented Apr 8, 2026

Uh oh!

tiprow Bot commented Apr 8, 2026

Uh oh!

D3Hunter commented Apr 8, 2026

Uh oh!

tiprow Bot commented Apr 8, 2026

Uh oh!

D3Hunter commented Apr 8, 2026

Uh oh!

tiprow Bot commented Apr 8, 2026

Uh oh!

D3Hunter commented Apr 9, 2026

Uh oh!

tiprow Bot commented Apr 9, 2026

Uh oh!

D3Hunter commented Apr 9, 2026

Uh oh!

tiprow Bot commented Apr 9, 2026

Uh oh!

D3Hunter commented Apr 9, 2026

Uh oh!

ti-chi-bot Bot commented Apr 9, 2026

Uh oh!

D3Hunter commented Apr 9, 2026

Uh oh!

tiprow Bot commented Apr 9, 2026

Uh oh!

D3Hunter commented Apr 9, 2026

Uh oh!

D3Hunter commented Apr 8, 2026 •

edited

Loading

pantheon-ai Bot commented Apr 8, 2026 •

edited

Loading

coderabbitai Bot commented Apr 8, 2026 •

edited

Loading

codecov Bot commented Apr 8, 2026 •

edited

Loading