From c6d246d7f0e8c5e6f7db584371f6782363aad8db Mon Sep 17 00:00:00 2001 From: deepthi Date: Fri, 6 Sep 2024 14:26:41 -0700 Subject: [PATCH 1/3] FindErrantGTIDs: superset is not an errant GTID situation Signed-off-by: deepthi --- go/mysql/replication/replication_status.go | 8 ++++++++ go/mysql/replication/replication_status_test.go | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/go/mysql/replication/replication_status.go b/go/mysql/replication/replication_status.go index 9b7d674f2a9..751eccb6633 100644 --- a/go/mysql/replication/replication_status.go +++ b/go/mysql/replication/replication_status.go @@ -201,6 +201,14 @@ func (s *ReplicationStatus) FindErrantGTIDs(otherReplicaStatuses []*ReplicationS otherSets = append(otherSets, otherSet) } + if len(otherSets) == 1 { + // If there is only one replica to compare against, and one is a subset of the other, then we consider them not to be errant. + // It simply means that one replica might be behind on replication. + if relayLogSet.Contains(otherSets[0]) || otherSets[0].Contains(relayLogSet) { + return nil, nil + } + } + // Copy set for final diffSet so we don't mutate receiver. diffSet := make(Mysql56GTIDSet, len(relayLogSet)) for sid, intervals := range relayLogSet { diff --git a/go/mysql/replication/replication_status_test.go b/go/mysql/replication/replication_status_test.go index 25ff48dcd9c..659da9f9273 100644 --- a/go/mysql/replication/replication_status_test.go +++ b/go/mysql/replication/replication_status_test.go @@ -105,6 +105,16 @@ func TestFindErrantGTIDs(t *testing.T) { otherRepStatuses: []*ReplicationStatus{{SourceUUID: sid1, RelayLogPosition: Position{GTIDSet: set1}}}, // servers with the same GTID sets should not be diagnosed with errant GTIDs want: nil, + }, { + mainRepStatus: &ReplicationStatus{SourceUUID: sourceSID, RelayLogPosition: Position{GTIDSet: set2}}, + otherRepStatuses: []*ReplicationStatus{{SourceUUID: sid1, RelayLogPosition: Position{GTIDSet: set3}}}, + // set2 is a strict subset of set3 + want: nil, + }, { + mainRepStatus: &ReplicationStatus{SourceUUID: sourceSID, RelayLogPosition: Position{GTIDSet: set3}}, + otherRepStatuses: []*ReplicationStatus{{SourceUUID: sid1, RelayLogPosition: Position{GTIDSet: set2}}}, + // set3 is a strict superset of set2 + want: nil, }} for _, testcase := range testcases { From 77018e9e0430ad5b5c2397e1b4e2e6877d5a2e49 Mon Sep 17 00:00:00 2001 From: deepthi Date: Fri, 6 Sep 2024 15:02:43 -0700 Subject: [PATCH 2/3] fix unit tests Signed-off-by: deepthi --- go/vt/vtctl/reparentutil/replication.go | 2 +- go/vt/vtctl/reparentutil/replication_test.go | 30 ++++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/go/vt/vtctl/reparentutil/replication.go b/go/vt/vtctl/reparentutil/replication.go index da67e735882..d04a1992d2e 100644 --- a/go/vt/vtctl/reparentutil/replication.go +++ b/go/vt/vtctl/reparentutil/replication.go @@ -123,7 +123,7 @@ func FindValidEmergencyReparentCandidates( case len(errantGTIDs) != 0: // This tablet has errant GTIDs. It's not a valid candidate for // reparent, so don't insert it into the final mapping. - log.Errorf("skipping %v because we detected errant GTIDs - %v", alias, errantGTIDs) + log.Errorf("skipping %v with GTIDSet:%v because we detected errant GTIDs - %v", alias, relayLogGTIDSet, errantGTIDs) continue } diff --git a/go/vt/vtctl/reparentutil/replication_test.go b/go/vt/vtctl/reparentutil/replication_test.go index 922dc6bc2da..bd244c369b9 100644 --- a/go/vt/vtctl/reparentutil/replication_test.go +++ b/go/vt/vtctl/reparentutil/replication_test.go @@ -161,7 +161,7 @@ func TestFindValidEmergencyReparentCandidates(t *testing.T) { shouldErr: false, }, { - name: "tablet with errant GTIDs is excluded", + name: "tablet with superset GTIDs is included", statusMap: map[string]*replicationdatapb.StopReplicationStatus{ "r1": { After: &replicationdatapb.Status{ @@ -169,7 +169,7 @@ func TestFindValidEmergencyReparentCandidates(t *testing.T) { RelayLogPosition: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5", }, }, - "errant": { + "r2": { After: &replicationdatapb.Status{ SourceUuid: "3E11FA47-71CA-11E1-9E33-C80AA9429562", RelayLogPosition: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5,AAAAAAAA-71CA-11E1-9E33-C80AA9429562:1", @@ -181,7 +181,31 @@ func TestFindValidEmergencyReparentCandidates(t *testing.T) { Position: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5", }, }, - expected: []string{"r1", "p1"}, + expected: []string{"r1", "r2", "p1"}, + shouldErr: false, + }, + { + name: "tablets with errant GTIDs are excluded", + statusMap: map[string]*replicationdatapb.StopReplicationStatus{ + "r1": { + After: &replicationdatapb.Status{ + SourceUuid: "3E11FA47-71CA-11E1-9E33-C80AA9429562", + RelayLogPosition: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5,AAAAAAAA-71CA-11E1-9E33-C80AA9429562:1", + }, + }, + "r2": { + After: &replicationdatapb.Status{ + SourceUuid: "3E11FA47-71CA-11E1-9E33-C80AA9429562", + RelayLogPosition: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5,AAAAAAAA-71CA-11E1-9E33-C80AA9429562:2-3", + }, + }, + }, + primaryStatusMap: map[string]*replicationdatapb.PrimaryStatus{ + "p1": { + Position: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5", + }, + }, + expected: []string{"p1"}, shouldErr: false, }, { From 80a88494b04860f3332820b24c8ad16f9f629b15 Mon Sep 17 00:00:00 2001 From: deepthi Date: Fri, 6 Sep 2024 15:57:56 -0700 Subject: [PATCH 3/3] change unit test to be clearer about the test setup Signed-off-by: deepthi --- go/vt/vtctl/reparentutil/replication_test.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/go/vt/vtctl/reparentutil/replication_test.go b/go/vt/vtctl/reparentutil/replication_test.go index bd244c369b9..666b41859cf 100644 --- a/go/vt/vtctl/reparentutil/replication_test.go +++ b/go/vt/vtctl/reparentutil/replication_test.go @@ -176,12 +176,7 @@ func TestFindValidEmergencyReparentCandidates(t *testing.T) { }, }, }, - primaryStatusMap: map[string]*replicationdatapb.PrimaryStatus{ - "p1": { - Position: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5", - }, - }, - expected: []string{"r1", "r2", "p1"}, + expected: []string{"r1", "r2"}, shouldErr: false, }, { @@ -200,12 +195,7 @@ func TestFindValidEmergencyReparentCandidates(t *testing.T) { }, }, }, - primaryStatusMap: map[string]*replicationdatapb.PrimaryStatus{ - "p1": { - Position: "MySQL56/3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5", - }, - }, - expected: []string{"p1"}, + expected: []string{}, shouldErr: false, }, {