*: implement a global task manager and a sub task manager to handle the read/write of global task table and sub-task table #41979

wjhuang2016 · 2023-03-07T07:56:31Z

What problem does this PR solve?

Issue Number: close #41984

Problem Summary:

This PR does the following:
1: create a table and modify a table(global task table and sub-task table) when upgrading.
2: Implement a global task manager and a sub-task manager to handle the read/write of these two tables.
3: Add a kv option to represent the distTask type.
4: Fix a tidy bug when handling the parse error, which is annoying if there is any parse error of the internal SQL.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

ti-chi-bot · 2023-03-07T07:56:33Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

XuHuaiyu
zimulala

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.

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

purelind · 2023-03-07T15:37:01Z

/retest

disttask/framework/storage/table_test.go

zimulala · 2023-03-08T01:28:11Z

disttask/framework/storage/task_table.go

+	return task, nil
+}
+
+// GetTaskByID gets the task by the global task id.


zimulala · 2023-03-08T01:38:31Z

session/bootstrap.go

+	);`
+
+	// CreateSubTask is a table about sub-task.
+	CreateSubTask = `CREATE TABLE IF NOT EXISTS mysql.tidb_sub_task (


Feeling can use BackgroundSubtaskHistoryTable directly, because now the table structure in the late may also want to change, that is to change the original directly.

disttask/framework/storage/BUILD.bazel

disttask/framework/storage/task_table.go

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

zimulala · 2023-03-09T03:51:28Z

disttask/framework/storage/task_table.go

+}
+
+// GetNewTask get a new task from global task table, it's used by dispatcher only.
+func (stm *GlobalTaskManager) GetNewTask() (task *proto.Task, err error) {


Is it better to provide a function to get unfinished tasks? Otherwise, when TiDB downs, the task being processed will not get.

It can be added later when it's needed.

GMHDBJD

LGTM

zimulala · 2023-03-09T05:54:56Z

session/session.go

@@ -1719,7 +1719,7 @@ func (s *session) ParseWithParams(ctx context.Context, sql string, args ...inter
 	} else {
 		stmts, warns, err = s.ParseSQL(ctx, sql, s.sessionVars.GetParseParams()...)
 	}
-	if len(stmts) != 1 {
+	if len(stmts) != 1 && err == nil {


Do we have a test to test this？

session/bootstrap_test.go

disttask/framework/storage/table_test.go

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 · 2023-03-09T07:27:34Z

/retest

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

zimulala

LGTM

zimulala · 2023-03-09T17:16:23Z

disttask/framework/storage/task_table.go

+	stm.mu.Lock()
+	defer stm.mu.Unlock()
+
+	_, err := execSQL(stm.ctx, stm.se, "update mysql.tidb_global_task set state = %?, dispatcher_id = %?, step = %?, state_update_time = %? where id = %?", task.State, task.DispatcherID, task.Step, task.StateUpdateTime.UTC().String(), task.ID)


This function forgets to update Concurrency and please add a test.

zimulala · 2023-03-09T17:29:32Z

session/bootstrap.go

+		type VARCHAR(256) NOT NULL,
+		dispatcher_id VARCHAR(256),
+		state VARCHAR(64) NOT NULL,
+		start_time TIMESTAMP DEFAULT CURRENT_TIMESTAMP,


It is not felt necessary to set the default value to "CURRENT_TIMESTAMP"？Now the dispatcher will set the value when it is processed. If it is processed by PR, the default value will be set when it is processed by the client. Is it not as expected？

zimulala · 2023-03-10T04:09:49Z

PTAL @wjhuang2016

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 · 2023-03-13T03:16:16Z

/merge

ti-chi-bot · 2023-03-13T03:16:20Z

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

Commit hash: 81b824e

wjhuang2016 · 2023-03-13T03:30:41Z

/retest

wjhuang2016 requested a review from a team as a code owner March 7, 2023 07:56

ti-chi-bot added do-not-merge/invalid-title do-not-merge/needs-linked-issue release-note-none size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 7, 2023

init

93b922b

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 force-pushed the distTask_storage branch from f705872 to 93b922b Compare March 7, 2023 07:58

wjhuang2016 added 2 commits March 7, 2023 16:31

save

d0a5796

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

add more test

806376c

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-linked-issue labels Mar 7, 2023

wjhuang2016 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 7, 2023

ti-chi-bot removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/needs-triage-completed labels Mar 7, 2023

wjhuang2016 added 3 commits March 7, 2023 17:06

finish test

ba3f451

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

Merge branch 'master' of github.com:pingcap/tidb into distTask_storage

f564b53

done

744b867

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 changed the title ~~WIP~~ *: implement global task manager and sub task manager Mar 7, 2023

ti-chi-bot removed the do-not-merge/invalid-title label Mar 7, 2023

wjhuang2016 changed the title *: implement global task manager and sub task manager *: implement a global task manager and a sub task manager to handle the read/write of global task table and sub-task table Mar 7, 2023

wjhuang2016 added 2 commits March 7, 2023 21:43

add comment

d6ab3aa

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

fix lint

3570b13

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

zimulala reviewed Mar 8, 2023

View reviewed changes

hawkingrei reviewed Mar 8, 2023

View reviewed changes

disttask/framework/storage/BUILD.bazel Show resolved Hide resolved

hawkingrei reviewed Mar 8, 2023

View reviewed changes

disttask/framework/storage/task_table.go Outdated Show resolved Hide resolved

wjhuang2016 added 3 commits March 8, 2023 16:33

address comment

d50c901

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

refine

77bad5e

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

fmt

b69a4ab

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

add one index

b711fbf

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

zimulala reviewed Mar 9, 2023

View reviewed changes

GMHDBJD approved these changes Mar 9, 2023

View reviewed changes

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

zimulala reviewed Mar 9, 2023

View reviewed changes

address comment

1201d91

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

refine test

fceb63f

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

zimulala approved these changes Mar 9, 2023

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 9, 2023

zimulala requested changes Mar 9, 2023

View reviewed changes

ti-chi-bot removed the status/LGT2 Indicates that a PR has LGTM 2. label Mar 9, 2023

zimulala reviewed Mar 9, 2023

View reviewed changes

address comment

546286f

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 10, 2023

fix conflict

386658c

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 10, 2023

zimulala approved these changes Mar 10, 2023

View reviewed changes

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

fix test

81b824e

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

XuHuaiyu approved these changes Mar 13, 2023

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 13, 2023

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

ti-chi-bot merged commit 6c03f5e into pingcap:master Mar 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: implement a global task manager and a sub task manager to handle the read/write of global task table and sub-task table #41979

*: implement a global task manager and a sub task manager to handle the read/write of global task table and sub-task table #41979

wjhuang2016 commented Mar 7, 2023 •

edited

ti-chi-bot commented Mar 7, 2023 •

edited

purelind commented Mar 7, 2023

zimulala Mar 8, 2023

zimulala Mar 8, 2023

zimulala Mar 9, 2023

wjhuang2016 Mar 9, 2023

GMHDBJD left a comment

zimulala Mar 9, 2023

wjhuang2016 Mar 9, 2023

wjhuang2016 commented Mar 9, 2023

zimulala left a comment

zimulala Mar 9, 2023

zimulala Mar 9, 2023

zimulala commented Mar 10, 2023

wjhuang2016 commented Mar 13, 2023

ti-chi-bot commented Mar 13, 2023

wjhuang2016 commented Mar 13, 2023

*: implement a global task manager and a sub task manager to handle the read/write of global task table and sub-task table #41979

*: implement a global task manager and a sub task manager to handle the read/write of global task table and sub-task table #41979

Conversation

wjhuang2016 commented Mar 7, 2023 • edited

What problem does this PR solve?

Check List

Release note

ti-chi-bot commented Mar 7, 2023 • edited

purelind commented Mar 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GMHDBJD left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjhuang2016 commented Mar 9, 2023

zimulala left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zimulala commented Mar 10, 2023

wjhuang2016 commented Mar 13, 2023

ti-chi-bot commented Mar 13, 2023

wjhuang2016 commented Mar 13, 2023

wjhuang2016 commented Mar 7, 2023 •

edited

ti-chi-bot commented Mar 7, 2023 •

edited