Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Mar 11, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented Mar 5, 2020

cherry-pick #14342 to release-3.0


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".

Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor Author

sre-bot commented Mar 5, 2020

/run-all-tests

ddl/column_test.go Outdated Show resolved Hide resolved
ddl/column_test.go Outdated Show resolved Hide resolved
@lzmhhh123 lzmhhh123 removed their request for review March 9, 2020 09:46
@zimulala
Copy link
Contributor

PTAL @djshow832 @AilinKid

Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

LGTM

@zimulala zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 11, 2020
AilinKid
AilinKid previously approved these changes Mar 11, 2020
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

LGTM

@AilinKid AilinKid added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 11, 2020
@zimulala zimulala added the status/can-merge Indicates a PR has been approved by a committer. label Mar 11, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Mar 11, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented Mar 11, 2020

@sre-bot merge failed.

@zimulala
Copy link
Contributor

/run-integration-ddl-test

@zimulala
Copy link
Contributor

/run-all-tests

@zimulala zimulala merged commit 315b9f2 into pingcap:release-3.0 Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants