import into: precheck and register to pd #44313
Conversation
|
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
bot
added
release-note-none
| } else { | ||
| // if the task is run distributively, like IMPORT INTO, we should refresh the lease ID, | ||
| // in case the owner changed during the registration, and the new owner create the key. | ||
| tr.curLeaseID = clientv3.LeaseID(resp.Kvs[0].Lease) |
There was a problem hiding this comment.
Does it want to prevent that, the new leader overwrites the key, but some peers are still keepalive for the overwritten key?
There was a problem hiding this comment.
yes, other tidb might keeps a stale lease id, although this case seems ok to report err
| type flowHandle struct { | ||
| mu sync.RWMutex | ||
| // the last time we switch TiKV into IMPORT mode, this is a global operation, do it for one task makes | ||
| // no difference to do it for all tasks. So we do not need to record the switch time for each task. | ||
| lastSwitchTime atomic.Time | ||
|
|
||
| // taskInfoMap is a map from taskID to taskInfo | ||
| taskInfoMap sync.Map | ||
| } | ||
|
|
||
| var _ dispatcher.TaskFlowHandle = (*flowHandle)(nil) | ||
|
|
||
| func (h *flowHandle) OnTicker(ctx context.Context, task *proto.Task) { |
There was a problem hiding this comment.
Will this callback function be called concurrently by framework 🤔 Maybe no need to use sync.Map
There was a problem hiding this comment.
yes, there's one detect routine for each task in framework(detectTask
There was a problem hiding this comment.
i am considering to remove all sync, since we only allow 1 load task right now.
| info.register(ctx) | ||
| } | ||
|
|
||
| func (h *flowHandle) unregisterTask(ctx context.Context, task *proto.Task) { |
There was a problem hiding this comment.
will the framework make sure no registerTask is called after unregisterTask is returned? And will they be concurrently called?
There was a problem hiding this comment.
yes, unregisterTask is called when task finished or met error, registerTask when not finished and no error
same task no, different task yes.
ti-chi-bot
bot
added
the
needs-rebase
ti-chi-bot
bot
removed
the
needs-rebase
|
/run-integration-br-test |
ti-chi-bot
bot
added
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GMHDBJD, lance6716 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
lgtm
and removed
needs-1-more-lgtm
|
/hold |
ti-chi-bot
bot
added
the
do-not-merge/hold
|
/run-integration-br-test |
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
|
|
What problem does this PR solve?
Issue Number: ref #42930
Problem Summary:
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.