Skip to content

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

@lzmhhh123 lzmhhh123 removed their request for review March 9, 2020 09:46
@zimulala zimulala force-pushed the release-3.0-6ccdf645dc7b branch from b2cb293 to 40a73d5 Compare March 11, 2020 03:44
@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 The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants