pkg/dxf: avoid misleading error log on context cancel (#67612)#67674
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
@D3Hunter This PR has conflicts, I have hold it. |
|
@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide. DetailsInstructions 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChanged logging in BaseTaskExecutor.Run so that when runSubtask returns a context-canceled error it is logged at Info with a short message; other runSubtask errors continue to be logged as Error via sampleLogger. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/disttask/framework/taskexecutor/task_executor.go`:
- Around line 387-393: The code incorrectly references e.sampleLogger which
doesn't exist on BaseTaskExecutor; replace both uses of e.sampleLogger in the
context-canceled and error branches with the existing logger field e.logger so
logging remains consistent with the struct; update the two calls that currently
call e.sampleLogger.Info(...) and e.sampleLogger.Error(...) to call
e.logger.Info(...) and e.logger.Error(...) respectively in the Run/execute path
where llog.IsContextCanceledError(err) is checked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1103c7e0-c678-4439-b28a-ad002a344b3f
📒 Files selected for processing (1)
pkg/disttask/framework/taskexecutor/task_executor.go
… cherry-pick-67612-to-release-nextgen-20251011 # Conflicts: # pkg/disttask/framework/taskexecutor/task_executor.go
|
/unhold |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Benjamin2037, D3Hunter The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-nextgen-20251011 #67674 +/- ##
=============================================================
Coverage ? 71.8532%
=============================================================
Files ? 1833
Lines ? 493072
Branches ? 0
=============================================================
Hits ? 354288
Misses ? 115460
Partials ? 23324
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
a31d609
into
pingcap:release-nextgen-20251011
This is an automated cherry-pick of #67612
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, handlerunSubtaskerrors by type:llog.IsContextCanceledError(err)), log an info message.This preserves error visibility for real failures while reducing noisy/misleading logs for expected cancellations.
Check List
Tests
Side effects
Documentation
Release note
Summary by CodeRabbit