relay: start with the min binlog position in checkpoint #457

WangXiangUSTC · 2020-01-15T15:56:23Z

What problem does this PR solve?

relay use the latest binlog position as default, if increment task migrate from dm-worker1 to dm-worker2, sync unit will get binlog from checkpoint's binlog position, and this position <= the latest position, so sync unit can't get the binlog.

What is changed and how it works?

get the min binlog position from all subtasks, and relay start with this min position.

Check List

Tests

Integration test

Co-Authored-By: Xuecheng Zhang <csuzhangxc@gmail.com>

WangXiangUSTC · 2020-01-15T16:08:27Z

dm/master/coordinator/coordinator.go

@@ -418,7 +425,7 @@ func (c *Coordinator) restartMysqlTask(w *Worker, cfg *config.MysqlConfig) bool
 		Op:     pb.WorkerOp_StartWorker,
 		Config: task,
 	}
-	resp, err := w.OperateMysqlWorker(context.Background(), req, time.Second*3)
+	resp, err := w.OperateMysqlWorker(context.Background(), req, time.Second*5)


just add duration to let test pass, maybe @lichunzhu will fix it

pkg/binlog/position.go

WangXiangUSTC · 2020-01-15T16:10:22Z

this pr contain #451, we can merge 451 at the first, or review this pr directly and close 451 @lichunzhu @csuzhangxc

lichunzhu · 2020-01-16T02:43:32Z

this pr contain #451, we can merge 451 at the first, or review this pr directly and close 451 @lichunzhu @csuzhangxc

@WangXiangUSTC I have already approved #451. If this PR runs fine in your machine, I think it's OK to merge it directly.

WangXiangUSTC · 2020-01-16T14:07:04Z

/run-all-tests tidb=release-3.0

WangXiangUSTC · 2020-01-19T03:01:08Z

/run-all-tests tidb=release-3.0

csuzhangxc

rest LGTM

dm/master/coordinator/coordinator.go

dm/worker/worker.go

relay/relay.go

dm/worker/server.go

lichunzhu · 2020-01-20T03:06:10Z

dm/master/coordinator/coordinator.go

@@ -392,18 +392,26 @@ func (c *Coordinator) tryRestartMysqlTask() {
 	for hasTaskToSchedule {
 		select {
 		case source := <-c.waitingTask:
+			log.L().Info("try restart mysql task", zap.String("source", source))


Actually the log try restart mysql task info is not accurate here. The tasks started for the first time are also scheduled in this function. Check c.waitingTask in goland you will find the only read access is here.

I update the log 70d6e8e

WangXiangUSTC · 2020-01-20T06:59:06Z

/run-all-tests tidb=release-3.0

WangXiangUSTC · 2020-01-20T07:47:04Z

/run-all-tests tidb=release-3.0

dm/worker/server.go

WangXiangUSTC · 2020-01-20T08:31:53Z

/run-all-tests tidb=release-3.0

csuzhangxc

LGTM

csuzhangxc · 2020-01-20T08:52:22Z

@lichunzhu PTAL again.

lichunzhu

LGTM

The unit test didn't pass

lichunzhu · 2020-01-21T16:42:33Z

The unit test didn't pass. Could you pls check it?

WangXiangUSTC · 2020-02-03T13:19:25Z

/run-all-tests tidb=release-3.0

WangXiangUSTC · 2020-02-03T13:41:48Z

/run-all-tests tidb=release-3.0

WangXiangUSTC · 2020-02-03T13:55:08Z

@lichunzhu I fix unit test in 7802a15 and 1f82f5a, PTAL again

lichunzhu

LGTM

WangXiangUSTC and others added 13 commits January 11, 2020 23:49

fix get subtask

f7c67bd

add test

bc8ed8c

minor fix

6338cc8

Apply suggestions from code review

c58e767

Co-Authored-By: Xuecheng Zhang <csuzhangxc@gmail.com>

address comment

34cd486

remove deploy config in dm-master

fca171d

check dm-worker offline

3721168

merge ha-dev and resolve conflicts

2a26366

add function to get min pos in all subtasks

0d200ea

set relay binlog name if not set

c4a6c25

minor fix

7cf89d8

remove useless log

6a0b42e

minor fix

f044c4a

WangXiangUSTC added priority/important Major change, requires approval from ≥2 primary reviewers status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jan 15, 2020

WangXiangUSTC added 2 commits January 15, 2020 23:58

merge ha-dev

7951f72

update true and false

594320f

WangXiangUSTC commented Jan 15, 2020

View reviewed changes

merge ha-dev and some fix

83b7e9a

WangXiangUSTC added 4 commits January 17, 2020 11:25

add db close

14ef879

add simple unit test

b93ca7f

merge ha-dev and resolve conflicts

65a75bf

minor fix

8617e18

csuzhangxc reviewed Jan 19, 2020

View reviewed changes

lichunzhu reviewed Jan 20, 2020

View reviewed changes

address comment

99e32ce

WangXiangUSTC added 4 commits January 20, 2020 13:40

update log

70d6e8e

add TODO

736aa48

use minPos when minPos is not nil

79442de

use checkpoint

69363d7

return error with annotate

e3a4ba7

csuzhangxc reviewed Jan 20, 2020

View reviewed changes

dm/worker/server.go Outdated Show resolved Hide resolved

dm/worker/server.go Outdated Show resolved Hide resolved

fix context

ce3635a

csuzhangxc reviewed Jan 20, 2020

View reviewed changes

csuzhangxc 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 Jan 20, 2020

lichunzhu previously approved these changes Jan 21, 2020

View reviewed changes

fix unit test

7802a15

WangXiangUSTC force-pushed the xiang/relay-pos branch from 0023942 to 7802a15 Compare February 3, 2020 04:08

WangXiangUSTC added 2 commits February 3, 2020 13:48

merge ha-dev

4b3f764

fix test

1f82f5a

lichunzhu approved these changes Feb 3, 2020

View reviewed changes

WangXiangUSTC merged commit 256723e into ha-dev Feb 4, 2020

WangXiangUSTC deleted the xiang/relay-pos branch February 4, 2020 01:47

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

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

relay: start with the min binlog position in checkpoint (pingcap#457)

310879c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

relay: start with the min binlog position in checkpoint #457

relay: start with the min binlog position in checkpoint #457

WangXiangUSTC commented Jan 15, 2020

WangXiangUSTC Jan 15, 2020

WangXiangUSTC commented Jan 15, 2020

lichunzhu commented Jan 16, 2020

WangXiangUSTC commented Jan 16, 2020

WangXiangUSTC commented Jan 19, 2020

csuzhangxc left a comment •

edited

Loading

lichunzhu Jan 20, 2020

WangXiangUSTC Jan 20, 2020

WangXiangUSTC commented Jan 20, 2020

WangXiangUSTC commented Jan 20, 2020

WangXiangUSTC commented Jan 20, 2020

csuzhangxc left a comment

csuzhangxc commented Jan 20, 2020

lichunzhu left a comment

lichunzhu commented Jan 21, 2020

WangXiangUSTC commented Feb 3, 2020

WangXiangUSTC commented Feb 3, 2020

WangXiangUSTC commented Feb 3, 2020

lichunzhu left a comment

relay: start with the min binlog position in checkpoint #457

relay: start with the min binlog position in checkpoint #457

Conversation

WangXiangUSTC commented Jan 15, 2020

What problem does this PR solve?

What is changed and how it works?

Check List

WangXiangUSTC Jan 15, 2020

Choose a reason for hiding this comment

WangXiangUSTC commented Jan 15, 2020

lichunzhu commented Jan 16, 2020

WangXiangUSTC commented Jan 16, 2020

WangXiangUSTC commented Jan 19, 2020

csuzhangxc left a comment • edited Loading

Choose a reason for hiding this comment

lichunzhu Jan 20, 2020

Choose a reason for hiding this comment

WangXiangUSTC Jan 20, 2020

Choose a reason for hiding this comment

WangXiangUSTC commented Jan 20, 2020

WangXiangUSTC commented Jan 20, 2020

WangXiangUSTC commented Jan 20, 2020

csuzhangxc left a comment

Choose a reason for hiding this comment

csuzhangxc commented Jan 20, 2020

lichunzhu left a comment

Choose a reason for hiding this comment

lichunzhu commented Jan 21, 2020

WangXiangUSTC commented Feb 3, 2020

WangXiangUSTC commented Feb 3, 2020

WangXiangUSTC commented Feb 3, 2020

lichunzhu left a comment

Choose a reason for hiding this comment

csuzhangxc left a comment •

edited

Loading