ddl: only unregister backendCtx after deliver job worker for local mode #54177
Conversation
ti-chi-bot
bot
added
release-note-none
|
Hi @tangenta. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #54177 +/- ##
=================================================
- Coverage 72.8465% 55.9390% -16.9075%
=================================================
Files 1520 1644 +124
Lines 435101 618593 +183492
=================================================
+ Hits 316956 346035 +29079
- Misses 98547 248476 +149929
- Partials 19598 24082 +4484
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/retest |
|
@tangenta: Cannot trigger testing until a trusted user reviews the PR and leaves an 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. |
|
/retest |
|
@tangenta: Cannot trigger testing until a trusted user reviews the PR and leaves an 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. |
|
@tangenta: The following tests failed, say
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/test-infra repository. I understand the commands that are listed here. |
| @@ -537,7 +537,9 @@ func (s *jobScheduler) delivery2Worker(wk *worker, pool *workerPool, job *model. | |||
| s.runningJobs.removeRunning(jobID, involvedSchemaInfos) | |||
| asyncNotify(s.ddlJobNotifyCh) | |||
| metrics.DDLRunningJobCount.WithLabelValues(pool.tp().String()).Dec() | |||
| if wk.ctx.Err() != nil && ingest.LitBackCtxMgr != nil { | |||
| if wk.ctx.Err() != nil && ingest.LitBackCtxMgr != nil && | |||
There was a problem hiding this comment.
Can we add some test cases?
|
[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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
approved
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
Closed by #54149 |
What problem does this PR solve?
Issue Number: close #53910
Problem Summary:
The
unregister()afterdelivery2Worker()may conflict with the executor of the DXF.When DXF executor starts to import:
tidb/pkg/lightning/backend/local/local.go
Line 1296 in fd2b5e9
And tidb receives
SigKill, ddl workers exit first, callingunregister(), which removes all engines fromengineManager.As a result, import executor actually does nothing, but mistakenly thinks it is successful. And the state is saved to subtask table.
What changed and how does it work?
When we use DXF, backend ctx should be owned by DXF. Other goroutines should not modify backend ctx.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.