Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

dmctl, scheduler: support manually transfer-source #1492

Merged
merged 10 commits into from
Mar 12, 2021
Merged

dmctl, scheduler: support manually transfer-source #1492

merged 10 commits into from
Mar 12, 2021

Conversation

lance6716
Copy link
Collaborator

@lance6716 lance6716 commented Mar 9, 2021 •
edited
Loading

What problem does this PR solve?

close #1075

What is changed and how it works?

support transfer a source to a free worker by transfer-source -s <sourc-id> -w <worker-id>

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@lance6716 lance6716 added needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Mar 9, 2021
@lance6716 lance6716 added this to the v2.0.2 milestone Mar 9, 2021
@GMHDBJD
Copy link
Collaborator

GMHDBJD commented Mar 9, 2021

Maybe operate-source transfer ?

@lance6716
Copy link
Collaborator Author

Maybe operate-source transfer ?

OK, I haven't add dmctl part

@lance6716
Copy link
Collaborator Author

Maybe operate-source transfer ?

operate-source will be more difficult to describe in one line as

operate-source <operate-type> [config-file ...] [--print-sample-config]

so I prefer transfer-source. And to implement #398, I hope someday we will support subcommand like source transfer 😂

@lance6716 lance6716 changed the title (WIP) dmctl, scheduler: support manually transfer-source dmctl, scheduler: support manually transfer-source Mar 10, 2021
lichunzhu
lichunzhu reviewed Mar 12, 2021
View reviewed changes
dm/master/scheduler/scheduler.go
failpoint.Inject("failToReplaceSourceBound", func(_ failpoint.Value) {
failpoint.Return(errors.New("failToPutSourceBound"))
})
_, err := ha.ReplaceSourceBound(s.etcdCli, source, oldWorker.BaseInfo().Name, worker)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any problem when the last worker doesn't stop its own source timely?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete source bound can't handle it? If so, we may have the same problem when there is a network isolation between worker and master.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

30f87fb

@lichunzhu
Copy link
Contributor

/lgtm

@ti-srebot ti-srebot added the status/LGT1 One reviewer already commented LGTM label Mar 12, 2021
GMHDBJD
GMHDBJD previously approved these changes Mar 12, 2021
View reviewed changes
Copy link
Collaborator

@GMHDBJD GMHDBJD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@lance6716
Copy link
Collaborator Author

going to stop transfer when there's running subtask on it, to avoid new and old worker both online

@lichunzhu lichunzhu added the status/DNM Do not merge, test is failing or blocked by another PR label Mar 12, 2021
@lance6716 lance6716 removed the status/DNM Do not merge, test is failing or blocked by another PR label Mar 12, 2021
@lance6716
Copy link
Collaborator Author

/run-all-tests

@lichunzhu
Copy link
Contributor

/lgtm

lichunzhu
lichunzhu approved these changes Mar 12, 2021
View reviewed changes
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lance6716 lance6716 merged commit 8157e33 into pingcap:master Mar 12, 2021
ti-srebot pushed a commit to ti-srebot/dm that referenced this pull request Mar 12, 2021
@lance6716 @ti-srebot
cherry pick pingcap#1492 to release-2.0
f3ce41f 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Mar 12, 2021
Merged
@ti-srebot
Copy link

cherry pick to release-2.0 in PR #1509

@ti-srebot ti-srebot added already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked and removed needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 labels Mar 12, 2021
lance6716 pushed a commit that referenced this pull request Mar 12, 2021
@ti-srebot
cherry pick #1492 to release-2.0 (#1509)
d4607cb
@lance6716 lance6716 mentioned this pull request Apr 1, 2021
Merged
7 tasks
@ti-srebot ti-srebot mentioned this pull request Apr 9, 2021
Merged
7 tasks
@lance6716 lance6716 removed the needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated label May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@GMHDBJD GMHDBJD GMHDBJD left review comments

@lichunzhu lichunzhu lichunzhu approved these changes

Assignees
No one assigned
Labels
already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated status/LGT1 One reviewer already commented LGTM
Projects
None yet
Milestone
v2.0.2
Development

Successfully merging this pull request may close these issues.

transfer source support for DM-worker
4 participants
@lance6716 @GMHDBJD @lichunzhu @ti-srebot