VTGR: Vitess + MySQL group replication by 5antelope · Pull Request #8387 · vitessio/vitess

5antelope · 2021-06-26T02:08:08Z

Description

This adds single primary mysql group replication support for Vitess. Specifically it does the followings:

VTTablet needs to understand and report the lag under MySQL group replication
VTGR handles group bootstrap and maintain the group memberships
VTGR handles necessary Vitess and MySQL failover and reconcile between the two

The structure of the PR is:

directory	purpose
vtgr/config	The configurations for VTGR
vtgr/controller/diagnose.go	VTGR diagnose the status for a Vitess shard
vtgr/controller/repair.go	VTGR repair the shard based on diagnose result
vtgr/controller/group.go	VTGR store MySQL group information
vtgr/controller/refresh.go	VTGR refresh tablet records from topo server
vtgr/db	DB client that talks to MySQL

Related Issue(s)

#8386

Checklist

Tests were added or are not required
Documentation was added or is not required

Deployment Notes

Signed-off-by: crowu <y.wu4515@gmail.com>

vmg · 2021-06-28T10:33:17Z

go/vt/vtgr/controller/diagnose.go

+	go func() { c <- shard.tmc.Ping(pingCtx, instance.tablet) }()
+	select {
+	case <-pingCtx.Done():
+		log.Errorf("Ping abort timeout %v", *pingTabletTimeout)
+		return false
+	case err := <-c:
+		if err != nil {
+			log.Errorf("Ping error host=%v: %v", instance.instanceKey.Hostname, err)
+		}
+		return err == nil
+	}


This is bad but it's actually our fault. Once #8368 is merged we should be able to fix the TMC API so that the initial dial is context-aware and we don't need this hack.

I don't think switching to DialContext there will actually fix this issue, though. From the docs

By default, it's a non-blocking dial (the function won't wait for connections to be established, and connecting happens in the background). To make it a blocking dial, use WithBlock() dial option.

In the non-blocking case, the ctx does not act against the connection. It only controls the setup steps.

I don't think we should unilaterally use WithBlock as that will result in slowdowns in the rest of the tmc use-cases, so what I think we should do is a follow-up refactor to the tmc api (and merge both #8368 (pending review / further changes) and this PR as-is) to allow callers to specify dial options to use, something like:

tmc := tmclient.TabletManagerClient().WithDialOptions(grpc.WithBlock())

I think this sounds about right: the comment that @narcsfz added to this method was a bit confusing, because it complains about dial/retry timeouts when (in theory) we shouldn't be seeing dial timeouts at all, since this API is not called with WithBlock and hence the actual connection will only happen during the actual call to Ping, and will be context-aware. I think what @narcsfz was seeing are the basic setup steps which by default are not controlled by context but will be if we switch to DialContext.

Thanks @vmg @ajm188. I'm happy to leave a TODO here and refactor it later if that makes sense. Since it is hard to reproduce in unit test, I also need to set up the test case that can reproduce the problem to test the change out.

Signed-off-by: crowu <y.wu4515@gmail.com>

go/cmd/vtgr/main.go

go/mysql/flavor_mysqlgr.go

go/mysql/flavor_mysqlgr_test.go

go/mysql/flavor_mysqlgr.go

Signed-off-by: crowu <y.wu4515@gmail.com>

5antelope · 2021-07-06T08:12:39Z

Addressed your comments ptal @systay

deepthi

Well-written and well-documented contribution.
These comments are from a first pass. I will be doing another pass over the PR.

go/mysql/flavor_mysqlgr.go

go/mysql/flavor.go

go/test/endtoend/vtgr/test_config.json

go/vt/vtgr/controller/diagnose.go

go/vt/vtgr/vtgr.go

Signed-off-by: crowu <y.wu4515@gmail.com>

5antelope · 2021-07-08T21:08:17Z

There is a test failure after a lot of retries. Looks like it's related to some network issue: https://github.com/vitessio/vitess/pull/8387/checks?check_run_id=3023339855

I will try to rerun it later, but it should not affect the review cc @deepthi

deepthi

Mostly LGTM. We can merge once feedback has been addressed and tests are passing.

go/cmd/vtgr/main.go

go/mysql/flavor_mysqlgr.go

go/vt/vtgr/controller/diagnose_test.go

go/vt/vtgr/controller/refresh_test.go

go/vt/vtgr/controller/repair_test.go

Signed-off-by: crowu <y.wu4515@gmail.com>

deepthi

Nice work!

5antelope added 4 commits June 23, 2021 09:46

Add mysql gr flavor

9bab27d

Signed-off-by: crowu <y.wu4515@gmail.com>

Add more gr status to orc

73a7eaf

Signed-off-by: crowu <y.wu4515@gmail.com>

vtgr core

1655b9d

Signed-off-by: crowu <y.wu4515@gmail.com>

Set up end to end test

6f18a72

Signed-off-by: crowu <y.wu4515@gmail.com>

5antelope requested a review from deepthi June 26, 2021 02:08

5antelope changed the title ~~Ywu/vtgr~~ VTGR: Vitess + MySQL group replication Jun 26, 2021

Fix race condition in test

6e08329

Signed-off-by: crowu <y.wu4515@gmail.com>

5antelope added the Component: Cluster management label Jun 26, 2021

Fix race in tests

7054d0b

Signed-off-by: crowu <y.wu4515@gmail.com>

5antelope added the Type: Feature label Jun 26, 2021

deepthi requested a review from vmg June 27, 2021 21:28

vmg marked this pull request as ready for review June 28, 2021 10:07

vmg requested review from harshit-gangal, shlomi-noach and systay as code owners June 28, 2021 10:07

vmg reviewed Jun 28, 2021

View reviewed changes

vmg mentioned this pull request Jun 28, 2021

[grpctmclient] Add support for (bounded) per-host connection reuse #8368

Merged

2 tasks

deepthi requested a review from fatih June 28, 2021 16:52

Remove over-optimization code in rebootstrap

eea9346

Signed-off-by: crowu <y.wu4515@gmail.com>

systay reviewed Jul 5, 2021

View reviewed changes

go/cmd/vtgr/main.go Show resolved Hide resolved

systay reviewed Jul 5, 2021

View reviewed changes

go/mysql/flavor_mysqlgr.go Outdated Show resolved Hide resolved

systay reviewed Jul 5, 2021

View reviewed changes

go/mysql/flavor_mysqlgr_test.go Show resolved Hide resolved

systay reviewed Jul 5, 2021

View reviewed changes

go/mysql/flavor_mysqlgr.go Outdated Show resolved Hide resolved

5antelope added 4 commits July 5, 2021 17:10

Add licence header and use vterrors

4f374bc

Signed-off-by: crowu <y.wu4515@gmail.com>

Add licence for VTGR

4c0e507

Signed-off-by: crowu <y.wu4515@gmail.com>

Clean up repair test format

51fbb6a

Signed-off-by: crowu <y.wu4515@gmail.com>

More licence headers

573f045

Signed-off-by: crowu <y.wu4515@gmail.com>

deepthi reviewed Jul 8, 2021

View reviewed changes

Clean up comments and rename functions

39263f2

Signed-off-by: crowu <y.wu4515@gmail.com>

move ErrNoGroupStatus into flavor

dee20cf

Signed-off-by: crowu <y.wu4515@gmail.com>

deepthi reviewed Jul 8, 2021

View reviewed changes

5antelope added 2 commits July 8, 2021 16:50

Fix format and imports

5177214

Signed-off-by: crowu <y.wu4515@gmail.com>

Merge remote-tracking branch 'upstream/main' into ywu/vtgr

c93b2d4

Signed-off-by: crowu <y.wu4515@gmail.com>

deepthi approved these changes Jul 13, 2021

View reviewed changes

deepthi merged commit 9fb976a into vitessio:main Jul 13, 2021

frouioui added the release notes label Sep 28, 2021

Conversation

5antelope commented Jun 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Checklist

Deployment Notes

Uh oh!

vmg Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajm188 Jun 29, 2021

Choose a reason for hiding this comment

Uh oh!

vmg Jun 29, 2021

Choose a reason for hiding this comment

Uh oh!

5antelope Jun 29, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

5antelope commented Jul 6, 2021

Uh oh!

deepthi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

5antelope commented Jul 8, 2021

Uh oh!

deepthi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deepthi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

5antelope commented Jun 26, 2021 •

edited

Loading

vmg Jun 28, 2021 •

edited

Loading