shard DDL: rewrite shard DDL in pessimism mode for HA #456

csuzhangxc · 2020-01-15T06:43:08Z

What problem does this PR solve?

rewrite shard DDL in pessimism mode to adjust the framework of HA

What is changed and how it works?

use etcd to coordinate the shard DDL lock instead of gRPC.
- to reviewers: it's better to review pkg/shardddl/pessimism/doc.go firstly
fix some integration test cases, NOTE, in order to make the case stable somethings need to be done:
- rewrite the coordinator for the scheduler
- fix the deadlock problem in StreamerController

Things have not to be done in this PR (I'll do them in the next PR)

add terror for shard DDL in pessimism mode
refactor manual handlers for shard DDL (e.g. unlock-ddl-lock)

Check List

Tests

Unit test
Integration test

Code changes

Has exported function/method change
Has exported variable/fields change
Has interface methods change
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

Need to update the documentation
Need to be included in the release note

csuzhangxc · 2020-02-08T05:07:01Z

@WangXiangUSTC @lichunzhu PTAL

pkg/shardddl/pessimism/doc.go

pkg/shardddl/pessimism/info.go

pkg/shardddl/pessimism/doc.go

lichunzhu · 2020-02-10T06:34:10Z

dm/master/shardddl/pessimist.go

+	p.lk.Clear() // clear all previous locks to support re-Start.
+
+	// get the history shard DDL info.
+	ifm, rev, err := pessimism.GetAllInfo(etcdCli)


Shall we synchronize the variable name of rev to revision-W1 in the explannation go program?

add the ref to doc.go in 3ae58b6.

dm/master/shardddl/pessimist.go

lichunzhu · 2020-02-10T07:04:36Z

dm/master/shardddl/pessimist.go

+			p.wg.Done()
+			close(opCh)
+		}()
+		pessimism.WatchOperationPut(ctx, etcdCli, "", "", rev, opCh)


should this rev be rev2?

rev can work correctly because rev < rev2, but rev2 is more readable, updated in 3ae58b6.

dm/master/shardddl/pessimist.go

pkg/shardddl/pessimism/doc.go

csuzhangxc · 2020-02-10T13:14:51Z

/run-all-tests tidb=release-3.0

dm/master/shardddl/pessimist.go

pkg/shardddl/pessimism/operation.go

syncer/shardddl/pessimist.go

syncer/syncer.go

WangXiangUSTC · 2020-02-10T15:30:39Z

syncer/syncer.go

-			shardLockResolving.WithLabelValues(s.cfg.Name).Set(0)
-			break
+		shardOp, err2 := s.pessimist.GetOperation(ec.tctx.Ctx, shardInfo, rev)
+		shardLockResolving.WithLabelValues(s.cfg.Name).Set(0)


do this after err2 is nil?

YES, if err2 == nil, syncer will abort the replication, then it's not waiting for the lock to be resolved anymore, but can be in resolving after resumed without error.

Co-Authored-By: WangXiangUSTC <wx347249478@gmail.com>

csuzhangxc · 2020-02-11T02:08:30Z

/run-all-tests tidb=release-3.0

codecov · 2020-02-11T02:21:10Z

Codecov Report

❗ No coverage uploaded for pull request base (ha-dev@71b6b9a). Click here to learn what that means.
The diff coverage is 58.1395%.

@@             Coverage Diff             @@
##             ha-dev       #456   +/-   ##
===========================================
  Coverage          ?   54.4227%           
===========================================
  Files             ?        174           
  Lines             ?      16890           
  Branches          ?          0           
===========================================
  Hits              ?       9192           
  Misses            ?       6780           
  Partials          ?        918

lichunzhu

Rest LGTM

lichunzhu · 2020-02-11T02:31:36Z

dm/master/shardddl/pessimist.go

+	lk  *pessimism.LockKeeper
+
+	// sources used to get all sources relative to the give task.
+	sources func(task string) []string


How about refract to taskSources or getTaskSources? sources seems weird.

renamed in e2972a2.

WangXiangUSTC

rest LGTM

WangXiangUSTC · 2020-02-11T07:19:57Z

pkg/shardddl/pessimism/ops.go

+
+	resp, err := cli.Txn(ctx).Then(putOp, delOp).Commit()
+	if err != nil {
+		return 0, nil


return err？

Good catch! fixed in e2972a2.

csuzhangxc · 2020-02-11T07:26:40Z

/run-all-tests tidb=release-3.0

csuzhangxc · 2020-02-11T07:26:49Z

@lichunzhu @WangXiangUSTC PTAL again.

lichunzhu

LGTM

WangXiangUSTC

LGTM

lichunzhu and others added 30 commits January 3, 2020 17:09

fix all_mode integration_test

ad3e066

fix dmctl_advance & dmctl_basic integration_tests

b3b3761

initital commit for dmctl_command

eb6ec4c

fix full_mode test

7230952

refine http_apis integration_test

1a865e4

fix import goroutine leak test

9a47eb0

fix incremental_mode integration_test

80e0269

fix ha-dev bug

2400863

refine initial_unit integration_test

f337799

fix load_interrupt integration_tests

634d67b

fix online ddl integration_tests

1a807a4

fix worker start for all integration_tests

18b63d1

fix tests

6bd6452

add operator_worker test

efd038b

fix bug

b9c7819

wait more time in retry_cancel test

060bbd4

wait more time in incremental_mode test

ce7819f

wait more time in load_interrupt test

03fcf74

address comments

565c9e9

wait for time in relay_interrupt

a59eb51

*: handle shard DDL info in etcd

ea0bf02

*: refine pkg

c471f84

*: add shard DDL lock

5a06721

*: add LockKeeper

0edab10

*: add Operation for etcd

7596e48

shardddl: add the doc about the coordinate of a shard DDL lock

9b6dbcd

shardddl: add DELETE op for Info

666d315

shardddl: add more operations for Operation

385ba93

shardddl: add PutOperationDeleteInfo; refine test suite

f1b02f1

Merge remote-tracking branch 'remotes/origin/ha-dev' into ha-shard-ddl

af586a2

csuzhangxc requested review from WangXiangUSTC and lichunzhu February 8, 2020 05:06

WangXiangUSTC reviewed Feb 9, 2020

View reviewed changes

pkg/shardddl/pessimism/doc.go Show resolved Hide resolved

pkg/shardddl/pessimism/info.go Show resolved Hide resolved

pkg/shardddl/pessimism/doc.go Show resolved Hide resolved

lichunzhu reviewed Feb 10, 2020

View reviewed changes

pkg/shardddl/pessimism/doc.go Show resolved Hide resolved

*: address comments

3ae58b6

WangXiangUSTC reviewed Feb 10, 2020

View reviewed changes

csuzhangxc and others added 2 commits February 11, 2020 09:29

Update dm/master/shardddl/pessimist.go

0f841ca

Co-Authored-By: WangXiangUSTC <wx347249478@gmail.com>

sharddll: debug CI

1ac1871

lichunzhu reviewed Feb 11, 2020

View reviewed changes

WangXiangUSTC reviewed Feb 11, 2020

View reviewed changes

*: address comments

e2972a2

lichunzhu reviewed Feb 11, 2020

View reviewed changes

lichunzhu added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Feb 11, 2020

WangXiangUSTC approved these changes Feb 11, 2020

View reviewed changes

WangXiangUSTC added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Feb 11, 2020

csuzhangxc merged commit 2a8a594 into pingcap:ha-dev Feb 11, 2020

csuzhangxc deleted the ha-shard-ddl branch February 11, 2020 07:46

lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020

shard DDL: rewrite shard DDL in pessimism mode for HA (pingcap#456)

997a5d1

csuzhangxc removed the needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated label Jun 17, 2020

csuzhangxc added already-update-docs The docs related to this PR already updated. Add this label once the docs are updated and removed needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated labels Oct 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

shard DDL: rewrite shard DDL in pessimism mode for HA #456

shard DDL: rewrite shard DDL in pessimism mode for HA #456

csuzhangxc commented Jan 15, 2020 •

edited

Loading

csuzhangxc commented Feb 8, 2020

lichunzhu Feb 10, 2020

csuzhangxc Feb 10, 2020

lichunzhu Feb 10, 2020

csuzhangxc Feb 10, 2020 •

edited

Loading

csuzhangxc commented Feb 10, 2020

WangXiangUSTC Feb 10, 2020

csuzhangxc Feb 11, 2020

csuzhangxc commented Feb 11, 2020

codecov bot commented Feb 11, 2020

lichunzhu left a comment

lichunzhu Feb 11, 2020

csuzhangxc Feb 11, 2020

WangXiangUSTC left a comment

WangXiangUSTC Feb 11, 2020

csuzhangxc Feb 11, 2020

csuzhangxc commented Feb 11, 2020

csuzhangxc commented Feb 11, 2020

lichunzhu left a comment

WangXiangUSTC left a comment

shard DDL: rewrite shard DDL in pessimism mode for HA #456

shard DDL: rewrite shard DDL in pessimism mode for HA #456

Conversation

csuzhangxc commented Jan 15, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Things have not to be done in this PR (I'll do them in the next PR)

Check List

csuzhangxc commented Feb 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csuzhangxc Feb 10, 2020 • edited Loading

Choose a reason for hiding this comment

csuzhangxc commented Feb 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csuzhangxc commented Feb 11, 2020

codecov bot commented Feb 11, 2020

Codecov Report

lichunzhu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WangXiangUSTC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csuzhangxc commented Feb 11, 2020

csuzhangxc commented Feb 11, 2020

lichunzhu left a comment

Choose a reason for hiding this comment

WangXiangUSTC left a comment

Choose a reason for hiding this comment

csuzhangxc commented Jan 15, 2020 •

edited

Loading

csuzhangxc Feb 10, 2020 •

edited

Loading