master(dm): support create a task in `stopped` state #4510

Ehco1996 · 2022-02-07T05:52:53Z

What problem does this PR solve?

Issue Number: ref #4484

What is changed and how it works?

for master

separate openapi.go into openapi_view.go and openapi_controaller.go, more inner logic such as filter,validate, and organze reponse will be add to openapi_controaller
scheduler support create a task in stopped stage
add new task OP op_delete in proto, when worker see this op, it will delete this subtask
dmctl stop-task now use op_delete to update task stage

for worker

support start a task in stopped stage when subtask not started
update operateSubTaskStage logic to match new stage

Check List

Tests

Unit test
Integration test

Code changes

Has interface methods change

Side effects

Increased code complexity

Related changes

Release note

support create a task in `stopped` state.

ti-chi-bot · 2022-02-07T05:52:54Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

D3Hunter
lance6716

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Ehco1996 · 2022-02-07T05:53:21Z

/hold

Ehco1996 · 2022-02-07T05:57:57Z

/run-dm-integration-tests

Ehco1996 · 2022-02-13T06:12:05Z

/run-dm-integration-tests

dm/dm/master/server.go

D3Hunter

rest lgtm

dm/dm/master/server.go

Ehco1996 · 2022-03-02T07:09:09Z

/run-dm-integration-tests

D3Hunter

LGTM

lance6716 · 2022-03-03T03:49:58Z

dm/dm/worker/source_worker.go

-		} else if stage.Expect == pb.Stage_Paused {
-			op = pb.TaskOp_Pause
+			return opErrTypeBeforeOp, w.StartSubTask(&subTaskCfg, stage.Expect, expectValidatorStage, true)
+		default:


st is nil, so OperateSubTask will fail. In old logic when st is nil we always call StartSubTask

i left a comment here, this err from operateSubTask is expected, caller should not operate a task that not in subTaskHolder

Oh I see the old logic only handles Stage_Running and Stage_Paused. But I think the logic of deciding if the expected stage is valid should be put inside StartSubTask and OperateSubTask, not in operateSubTaskStage. In other words, if the subtask exists, we simply forward expecte stage to OperateSubTask, otherwise to StartSubTask.

It's OK we change it in future.

dm/dm/worker/source_worker.go

lance6716 · 2022-03-03T04:00:59Z

dm/dm/worker/source_worker_test.go

+	}, pb.Stage_Stopped, pb.Stage_Stopped, true), IsNil)
+	task = w.subTaskHolder.findSubTask("testStartTask-in-stopped")
+	c.Assert(task, NotNil)
+	c.Assert(task.Result().String(), Matches, ".*worker already closed.*")


may need discuss with PM, if user creates a stopped task, DM-worker should
reserve some structure in memory and paused them, or do nothing?

For example, can a stopped incremental task that will read binlog.000001 forbid relay log purging binlog.000001

I think the stopped task still have to hold the necessary binlog.

An binlog may relates with multiple tasks. Whatever it's stopped or running , the binlog it needs should not be purged.

Maybe we can offer a feature in future like support query how many tasks relate with this binlog and users can purge it forced.

maybe later, current just do nothing

dm/dm/worker/subtask.go

Co-authored-by: lance6716 <lance6716@gmail.com>

lance6716 · 2022-03-03T06:49:19Z

dm/dm/worker/source_worker.go

-		} else if stage.Expect == pb.Stage_Paused {
-			op = pb.TaskOp_Pause
+			return opErrTypeBeforeOp, w.StartSubTask(&subTaskCfg, stage.Expect, expectValidatorStage, true)
+		default:


Oh I see the old logic only handles Stage_Running and Stage_Paused. But I think the logic of deciding if the expected stage is valid should be put inside StartSubTask and OperateSubTask, not in operateSubTaskStage. In other words, if the subtask exists, we simply forward expecte stage to OperateSubTask, otherwise to StartSubTask.

It's OK we change it in future.

Ehco1996 · 2022-03-03T06:57:19Z

/merge

ti-chi-bot · 2022-03-03T06:57:21Z

This pull request has been accepted and is ready to merge.

Commit hash: 5594251

Ehco1996 · 2022-03-03T07:28:53Z

/merge

ti-chi-bot · 2022-03-03T07:28:55Z

This pull request has been accepted and is ready to merge.

Commit hash: 7b21bf7

ti-chi-bot · 2022-03-03T07:37:56Z

@Ehco1996: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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.

savework enable test

d631fdd

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 7, 2022

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 7, 2022

Ehco1996 added the area/dm Issues or PRs related to DM. label Feb 7, 2022

Ehco1996 changed the title ~~savework enable test~~ master(dm): add task manger to manger task lifecycle Feb 7, 2022

Ehco1996 changed the title ~~master(dm): add task manger to manger task lifecycle~~ master(dm): add task manager Feb 7, 2022

Ehco1996 added 2 commits February 7, 2022 14:42

add UT

3e1ccc4

refine ut

873b5e1

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 7, 2022

Ehco1996 mentioned this pull request Feb 8, 2022

Tracking issues for Development of enhancing DM task manageability #4484

Open

7 tasks

Merge branch 'master' into feature-#4484-new-task-state

b8b7712

ti-chi-bot removed the do-not-merge/needs-triage-completed label Feb 13, 2022

add source manager and repalce api

b4d1e66

Ehco1996 changed the title ~~master(dm): add task manager~~ master(dm): add task/source manager Feb 13, 2022

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 13, 2022

Ehco1996 added 2 commits February 13, 2022 16:11

fix disableSource

f37b4f9

save work

30dbbbe

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 14, 2022

merge to one file

95ee828

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 14, 2022

Ehco1996 added 3 commits March 1, 2022 19:37

add status len check

a6080c9

use stage null when op_delete for better reading

256ef41

Merge branch 'master' into feature-#4484-new-task-state

f0e2406

D3Hunter reviewed Mar 2, 2022

View reviewed changes

dm/dm/master/server.go Outdated Show resolved Hide resolved

D3Hunter reviewed Mar 2, 2022

View reviewed changes

dm/dm/master/server.go Outdated Show resolved Hide resolved

use op to check result

fee001c

D3Hunter reviewed Mar 2, 2022

View reviewed changes

dm/dm/master/server.go Outdated Show resolved Hide resolved

address comment

c608c0b

D3Hunter approved these changes Mar 2, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 2, 2022

lance6716 reviewed Mar 3, 2022

View reviewed changes

Ehco1996 and others added 2 commits March 3, 2022 14:01

Update dm/dm/worker/source_worker.go

74ea013

Co-authored-by: lance6716 <lance6716@gmail.com>

address comment

8955c53

lance6716 approved these changes Mar 3, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 3, 2022

Ehco1996 added 2 commits March 3, 2022 14:56

left todo

78f098c

Merge branch 'master' into feature-#4484-new-task-state

5594251

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 3, 2022

fix conficts after merge master

7b21bf7

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 3, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 3, 2022

Merge branch 'master' into feature-#4484-new-task-state

5719c90

ti-chi-bot merged commit 3cbac21 into pingcap:master Mar 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

master(dm): support create a task in `stopped` state #4510

master(dm): support create a task in `stopped` state #4510

Ehco1996 commented Feb 7, 2022 •

edited

Loading

ti-chi-bot commented Feb 7, 2022 •

edited

Loading

Ehco1996 commented Feb 7, 2022

Ehco1996 commented Feb 7, 2022

Ehco1996 commented Feb 13, 2022

D3Hunter left a comment

Ehco1996 commented Mar 2, 2022

D3Hunter left a comment

lance6716 Mar 3, 2022

Ehco1996 Mar 3, 2022 •

edited

Loading

lance6716 Mar 3, 2022

lance6716 Mar 3, 2022

sunzhaoyang Mar 3, 2022

Ehco1996 Mar 3, 2022

lance6716 Mar 3, 2022

Ehco1996 commented Mar 3, 2022

ti-chi-bot commented Mar 3, 2022

Ehco1996 commented Mar 3, 2022

ti-chi-bot commented Mar 3, 2022

ti-chi-bot commented Mar 3, 2022

master(dm): support create a task in stopped state #4510

master(dm): support create a task in stopped state #4510

Conversation

Ehco1996 commented Feb 7, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

for master

for worker

Check List

Release note

ti-chi-bot commented Feb 7, 2022 • edited Loading

Ehco1996 commented Feb 7, 2022

Ehco1996 commented Feb 7, 2022

Ehco1996 commented Feb 13, 2022

D3Hunter left a comment

Choose a reason for hiding this comment

Ehco1996 commented Mar 2, 2022

D3Hunter left a comment

Choose a reason for hiding this comment

lance6716 Mar 3, 2022

Choose a reason for hiding this comment

Ehco1996 Mar 3, 2022 • edited Loading

Choose a reason for hiding this comment

lance6716 Mar 3, 2022

Choose a reason for hiding this comment

lance6716 Mar 3, 2022

Choose a reason for hiding this comment

sunzhaoyang Mar 3, 2022

Choose a reason for hiding this comment

Ehco1996 Mar 3, 2022

Choose a reason for hiding this comment

lance6716 Mar 3, 2022

Choose a reason for hiding this comment

Ehco1996 commented Mar 3, 2022

ti-chi-bot commented Mar 3, 2022

Ehco1996 commented Mar 3, 2022

ti-chi-bot commented Mar 3, 2022

ti-chi-bot commented Mar 3, 2022

master(dm): support create a task in `stopped` state #4510

master(dm): support create a task in `stopped` state #4510

Ehco1996 commented Feb 7, 2022 •

edited

Loading

ti-chi-bot commented Feb 7, 2022 •

edited

Loading

Ehco1996 Mar 3, 2022 •

edited

Loading