ddl: add a channel to limit multiple DDL jobs writing at the same time #14342

zimulala · 2020-01-03T11:24:04Z

What problem does this PR solve?

When performing DDL operations on multiple connections at the same time, you may encounter a "Write conflict" error.

What is changed and how it works?

Adding a channel on a single TiDB to limit multiple DDL jobs writing to the DDL job list at the same time results in "Write conflict".

Check List

Tests

Benchmark test

func addCreateTableJob(ctx sessionctx.Context, d *ddl, dbInfo *model.DBInfo, tblInfo *model.TableInfo) *model.Job {
	job := &model.Job{...}
	task := &limitJobTask{job, make(chan error)}
	d.limitJobCh <- task
	err := <-task.err
...
	return job
}

func BenchmarkAddDDLs(b *testing.B) {
...
	taskCnt := 1
	count := 200
	tblInfos := make([]*model.TableInfo, 0, count*taskCnt)
	for i := 0; i < count*taskCnt; i++ {
		tblInfo := testTableInfo(d, fmt.Sprintf("t_%d", i), 2)
		tblInfos = append(tblInfos, tblInfo)
	}
	wg := sync.WaitGroup{}
	runDDLsFunc := func(start, end int) {
		defer wg.Done()
		for i := start; i < end; i++ {
			addCreateTableJob(ctx, d, dbInfo, tblInfos[i])
		}
	}
	
	b.ResetTimer()
	wg.Add(taskCnt)
	for i := 0; i < taskCnt; i++ {
		go runDDLsFunc(i*count, (i+1)*count)
	}
	wg.Wait()
	b.StopTimer()
}

WC is the error of write confilct.

taskCnt	count	spend time(after)	WC count(after)	spend time(before)	WC count(before)
1	200	0.117ns	7	0115ns	6
3	200	0.254ns	18	1.092	147
5	200	0.419s	41	10.387	1210
5	400	1.894s	148	68.745	3509

Related changes

Need to cherry-pick to the release branch

Release note

Adding a channel on a single TiDB to limit multiple DDL jobs writing to the DDL job list at the same time results in "Write conflict".

ddl/ddl_test.go

bb7133 · 2020-01-04T07:42:35Z

Please explain the meaning of ”taskCnt" and "Count" in the table in your PR Description.

zimulala · 2020-01-05T14:15:13Z

@bb7133 ”taskCnt" and "Count" are the variables in the benchmark.

zimulala · 2020-01-07T03:46:59Z

PTAL @bb7133 @crazycs520 @tiancaiamao

ddl/ddl_worker.go

tiancaiamao · 2020-01-10T04:00:30Z

ddl/ddl_worker.go

+	for {
+		tasks := make([]*limitJobTask, 0, batchAddingJobs)
+		select {
+		case task := <-d.limitJobCh:


I do not quite understand how this limits the task rate.
By let multiple worker conflict on the same channel and increase the runtime lock spin?

Write conflicts occur when multiple DDL jobs are written to the DDL job list at the same time.
We use this channel to reduce such conflicts.

crazycs520

LGTM

tiancaiamao · 2020-01-10T05:50:50Z

LGTM

ddl/ddl_worker.go

djshow832 · 2020-01-10T06:53:48Z

ddl/ddl_worker.go

+			job.StartTS = txn.StartTS()
+			job.ID = ids[i]
+			if err = buildJobDependence(t, job); err != nil {
+				return errors.Trace(err)


If one task fails, following tasks are discarded?

Yes, but we get jobs failed only this function will fail to get jobs from TiKV. Therefore, if this job gets jobs failed, the other job should also fail.

PTAL @djshow832

zimulala · 2020-01-20T02:39:24Z

PTAL @coocood

zimulala · 2020-02-21T02:51:10Z

PTAL @coocood @AilinKid

AilinKid

Rest LGTM

AilinKid · 2020-02-21T03:02:33Z

ddl/ddl_worker.go

+		for i, task := range tasks {
+			job := task.job
+			job.Version = currentVersion
+			job.StartTS = txn.StartTS()


batch DDL jobs share the same txn.StartTS()? unless these ddls is in same statement.

Yes. But not necessarily the same statement.
And this field has no special use in TiFilash and Tools.

AilinKid

LGTM

sre-bot · 2020-03-04T02:38:38Z

/run-all-tests

sre-bot · 2020-03-04T02:41:38Z

@zimulala merge failed.

zimulala · 2020-03-05T03:10:14Z

/run-all-tests

zimulala · 2020-03-05T05:57:51Z

/run-integration-ddl-test

zimulala · 2020-03-05T06:02:11Z

/run-all-tests

AilinKid

LGTM

Signed-off-by: sre-bot <sre-bot@pingcap.com>

sre-bot · 2020-03-05T06:14:32Z

cherry pick to release-3.0 in PR #15148

#14342) (#15148)

zimulala added 2 commits January 3, 2020 18:08

ddl: add a channel to limit multiple DDL jobs writing at the same time

706f8b4

*: clean up

dbcce73

zimulala added component/DDL-need-LGT3 labels Jan 3, 2020

bb7133 reviewed Jan 4, 2020

View reviewed changes

ddl/ddl_test.go Outdated Show resolved Hide resolved

bb7133 added the type/enhancement The issue or PR belongs to an enhancement. label Jan 4, 2020

ddl: address a comment

05a2b1d

crazycs520 reviewed Jan 9, 2020

View reviewed changes

ddl/ddl_worker.go Outdated Show resolved Hide resolved

ddl: address a comment

81bf11c

tiancaiamao reviewed Jan 10, 2020

View reviewed changes

crazycs520 reviewed Jan 10, 2020

View reviewed changes

tiancaiamao added the status/LGT2 Indicates that a PR has LGTM 2. label Jan 10, 2020

djshow832 reviewed Jan 10, 2020

View reviewed changes

ddl: address a comment

1b00119

zimulala requested a review from coocood January 20, 2020 02:39

crazycs520 and others added 3 commits February 3, 2020 21:41

Merge branch 'master' into cache-ddl

95a72fa

Merge branch 'master' into cache-ddl

bbc680f

ddl: fix data race

4817a50

AilinKid reviewed Feb 21, 2020

View reviewed changes

AilinKid previously approved these changes Feb 21, 2020

View reviewed changes

zz-jason removed the component/DDL-need-LGT3 label Feb 27, 2020

ddl: remove d.start() to fix data race

a50ad38

zimulala dismissed AilinKid’s stale review via a50ad38 March 3, 2020 05:53

zimulala removed the status/LGT2 Indicates that a PR has LGTM 2. label Mar 3, 2020

zimulala force-pushed the cache-ddl branch from 2e1fabf to 943f125 Compare March 4, 2020 02:34

zimulala added status/can-merge Indicates a PR has been approved by a committer. component/DDL labels Mar 4, 2020

zimulala added sig/sql-infra SIG: SQL Infra and removed component/DDL1 labels Mar 4, 2020

zimulala force-pushed the cache-ddl branch from 1e867d2 to 38029f5 Compare March 5, 2020 03:05

zimulala requested a review from a team as a code owner March 5, 2020 03:05

ghost requested review from lzmhhh123 and winoros and removed request for a team March 5, 2020 03:05

ddl: tiny update

81cb0c1

zimulala force-pushed the cache-ddl branch from 38029f5 to 81cb0c1 Compare March 5, 2020 03:08

Merge branch 'master' into cache-ddl

6d72742

zimulala force-pushed the cache-ddl branch from 66ad6d1 to ef9e35d Compare March 5, 2020 05:39

cmd/ddltest: using "RunWorker" to control if run worker

bef2ef7

zimulala force-pushed the cache-ddl branch from ef9e35d to bef2ef7 Compare March 5, 2020 05:56

AilinKid approved these changes Mar 5, 2020

View reviewed changes

zimulala merged commit 6ccdf64 into pingcap:master Mar 5, 2020

zimulala deleted the cache-ddl branch March 5, 2020 06:13

AilinKid reviewed Mar 5, 2020

View reviewed changes

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 5, 2020

cherry pick pingcap#14342 to release-3.0

71480f6

Signed-off-by: sre-bot <sre-bot@pingcap.com>

sre-bot mentioned this pull request Mar 5, 2020

ddl: add a channel to limit multiple DDL jobs writing at the same time (#14342) #15148

Merged

sthagen mentioned this pull request Mar 5, 2020

ddl: add a channel to limit multiple DDL jobs writing at the same tim… sthagen/pingcap-tidb#29

Merged

zimulala pushed a commit that referenced this pull request Mar 11, 2020

ddl: add a channel to limit multiple DDL jobs writing at the same time (

315b9f2

#14342) (#15148)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: add a channel to limit multiple DDL jobs writing at the same time #14342

ddl: add a channel to limit multiple DDL jobs writing at the same time #14342

zimulala commented Jan 3, 2020 •

edited

Loading

bb7133 commented Jan 4, 2020

zimulala commented Jan 5, 2020

zimulala commented Jan 7, 2020

tiancaiamao Jan 10, 2020

zimulala Jan 10, 2020

crazycs520 left a comment

tiancaiamao commented Jan 10, 2020

djshow832 Jan 10, 2020

zimulala Jan 14, 2020 •

edited

Loading

zimulala Jan 17, 2020

zimulala commented Jan 20, 2020

zimulala commented Feb 21, 2020

AilinKid left a comment

AilinKid Feb 21, 2020 •

edited

Loading

zimulala Feb 21, 2020

AilinKid Feb 21, 2020

AilinKid left a comment

sre-bot commented Mar 4, 2020

sre-bot commented Mar 4, 2020

zimulala commented Mar 5, 2020

zimulala commented Mar 5, 2020

zimulala commented Mar 5, 2020

AilinKid left a comment

sre-bot commented Mar 5, 2020

ddl: add a channel to limit multiple DDL jobs writing at the same time #14342

ddl: add a channel to limit multiple DDL jobs writing at the same time #14342

Conversation

zimulala commented Jan 3, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

bb7133 commented Jan 4, 2020

zimulala commented Jan 5, 2020

zimulala commented Jan 7, 2020

tiancaiamao Jan 10, 2020

Choose a reason for hiding this comment

zimulala Jan 10, 2020

Choose a reason for hiding this comment

crazycs520 left a comment

Choose a reason for hiding this comment

tiancaiamao commented Jan 10, 2020

djshow832 Jan 10, 2020

Choose a reason for hiding this comment

zimulala Jan 14, 2020 • edited Loading

Choose a reason for hiding this comment

zimulala Jan 17, 2020

Choose a reason for hiding this comment

zimulala commented Jan 20, 2020

zimulala commented Feb 21, 2020

AilinKid left a comment

Choose a reason for hiding this comment

AilinKid Feb 21, 2020 • edited Loading

Choose a reason for hiding this comment

zimulala Feb 21, 2020

Choose a reason for hiding this comment

AilinKid Feb 21, 2020

Choose a reason for hiding this comment

AilinKid left a comment

Choose a reason for hiding this comment

sre-bot commented Mar 4, 2020

sre-bot commented Mar 4, 2020

zimulala commented Mar 5, 2020

zimulala commented Mar 5, 2020

zimulala commented Mar 5, 2020

AilinKid left a comment

Choose a reason for hiding this comment

sre-bot commented Mar 5, 2020

zimulala commented Jan 3, 2020 •

edited

Loading

zimulala Jan 14, 2020 •

edited

Loading

AilinKid Feb 21, 2020 •

edited

Loading