Skip to content

Commit 59bcb00

Browse files
pooknullhors
pooknull
and
hors
authored
K8SPS-548: fix comparePrimaryPurged function (#1125)
* K8SPS-548: fix rare `gr-self-healing` test failures https://perconadev.atlassian.net/browse/K8SPS-548 * fix comparePrimaryPurged * comparePrimaryPurged always returned `false` before this PR * replace os.Exit(1) * remove restart during bootstrap --------- Co-authored-by: Viacheslav Sarzhan <slava.sarzhan@percona.com>
1 parent 68a5c9f commit 59bcb00

File tree

3 files changed

+54
-44
lines changed

3 files changed

+54
-44
lines changed

‎cmd/bootstrap/gr/group_replication.go‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,14 @@ func (m *mysqlsh) rescanCluster(ctx context.Context) error {
117117
}
118118

119119
type SQLResult struct {
120-
Error string `json:"error,omitempty"`
121-
Rows []map[string]string `json:"rows,omitempty"`
120+
Error string `json:"error,omitempty"`
121+
Rows []map[string]any `json:"rows,omitempty"`
122122
}
123123

124124
func (m *mysqlsh) runSQL(ctx context.Context, sql string) (SQLResult, error) {
125125
var stdoutb, stderrb bytes.Buffer
126126

127-
cmd := fmt.Sprintf("session.runSql(\"%s\")", sql)
127+
cmd := fmt.Sprintf("session.runSql(`%s`)", sql)
128128
args := []string{"--uri", m.getURI(), "--js", "--json=raw", "--interactive", "--quiet-start", "2", "-e", cmd}
129129

130130
c := exec.CommandContext(ctx, "mysqlsh", args...)
@@ -159,7 +159,12 @@ func (m *mysqlsh) getGTIDExecuted(ctx context.Context) (string, error) {
159159
return "", errors.Errorf("unexpected output: %+v", result)
160160
}
161161

162-
return v, nil
162+
s, ok := v.(string)
163+
if !ok {
164+
return "", errors.Errorf("unexpected type: %T", v)
165+
}
166+
167+
return s, nil
163168
}
164169

165170
func (m *mysqlsh) getGTIDPurged(ctx context.Context) (string, error) {
@@ -173,7 +178,12 @@ func (m *mysqlsh) getGTIDPurged(ctx context.Context) (string, error) {
173178
return "", errors.Errorf("unexpected output: %+v", result)
174179
}
175180

176-
return v, nil
181+
s, ok := v.(string)
182+
if !ok {
183+
return "", errors.Errorf("unexpected type: %T", v)
184+
}
185+
186+
return s, nil
177187
}
178188

179189
func (m *mysqlsh) setGroupSeeds(ctx context.Context, seeds string) error {
@@ -308,7 +318,7 @@ func (m *mysqlsh) removeInstance(ctx context.Context, instanceDef string, force
308318
return nil
309319
}
310320

311-
func connectToLocal(ctx context.Context, version *v.Version) (*mysqlsh, error) {
321+
func connectToLocal(version *v.Version) (*mysqlsh, error) {
312322
fqdn, err := utils.GetFQDN(os.Getenv("SERVICE_NAME"))
313323
if err != nil {
314324
return nil, errors.Wrap(err, "get FQDN")
@@ -333,7 +343,7 @@ func connectToCluster(ctx context.Context, peers sets.Set[string], version *v.Ve
333343
}
334344

335345
func handleFullClusterCrash(ctx context.Context, version *v.Version) error {
336-
localShell, err := connectToLocal(ctx, version)
346+
localShell, err := connectToLocal(version)
337347
if err != nil {
338348
return errors.Wrap(err, "connect to local")
339349
}
@@ -403,7 +413,7 @@ func Bootstrap(ctx context.Context) error {
403413
}
404414
log.Println("mysql-shell version:", mysqlshVer)
405415

406-
localShell, err := connectToLocal(ctx, mysqlshVer)
416+
localShell, err := connectToLocal(mysqlshVer)
407417
if err != nil {
408418
return errors.Wrap(err, "connect to local")
409419
}

‎cmd/bootstrap/gr/recovery_method.go‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66
"log"
7-
"strconv"
87
"strings"
98

109
"github.com/pkg/errors"
@@ -34,7 +33,12 @@ func gtidSubtract(ctx context.Context, shell SQLRunner, a, b string) (string, er
3433
return "", errors.Errorf("unexpected output: %+v", result)
3534
}
3635

37-
return v, nil
36+
s, ok := v.(string)
37+
if !ok {
38+
return "", errors.Errorf("unexpected type: %T", v)
39+
}
40+
41+
return s, nil
3842
}
3943

4044
func gtidSubtractIntersection(ctx context.Context, shell SQLRunner, a, b string) (string, error) {
@@ -53,7 +57,12 @@ func gtidSubtractIntersection(ctx context.Context, shell SQLRunner, a, b string)
5357
return "", errors.Errorf("unexpected output: %+v", result)
5458
}
5559

56-
return v, nil
60+
s, ok := v.(string)
61+
if !ok {
62+
return "", errors.Errorf("unexpected type: %T", v)
63+
}
64+
65+
return s, nil
5766
}
5867

5968
type GTIDSetRelation string
@@ -114,25 +123,25 @@ func compareGTIDs(ctx context.Context, shell SQLRunner, a, b string) (GTIDSetRel
114123

115124
// If purged has more gtids than the executed on the replica
116125
// it means some data will not be recoverable
117-
func comparePrimaryPurged(ctx context.Context, shell SQLRunner, purged, executed string) bool {
118-
query := fmt.Sprintf("SELECT GTID_SUBTRACT('%s', '%s') = ''", purged, executed)
126+
func comparePrimaryPurged(ctx context.Context, shell SQLRunner, purged, executed string) (bool, error) {
127+
query := fmt.Sprintf("SELECT GTID_SUBTRACT('%s', '%s') = '' AS is_subset", purged, executed)
119128

120129
result, err := shell.runSQL(ctx, query)
121130
if err != nil {
122-
return false
131+
return false, errors.Wrap(err, "run sql")
123132
}
124133

125-
v, ok := result.Rows[0]["GTID_SUBTRACT"]
134+
v, ok := result.Rows[0]["is_subset"]
126135
if !ok {
127-
return false
136+
return false, errors.Errorf("unexpected output: %+v", result)
128137
}
129138

130-
sub, err := strconv.Atoi(v)
131-
if err != nil {
132-
return false
139+
s, ok := v.(float64)
140+
if !ok {
141+
return false, errors.Errorf("unexpected type: %T", v)
133142
}
134143

135-
return sub == 0
144+
return s == 1, nil
136145
}
137146

138147
func checkReplicaState(ctx context.Context, primary, replica SQLRunner) (innodbcluster.ReplicaGtidState, error) {
@@ -169,7 +178,14 @@ func checkReplicaState(ctx context.Context, primary, replica SQLRunner) (innodbc
169178
case GTIDSetEqual:
170179
return innodbcluster.ReplicaGtidIdentical, nil
171180
case GTIDSetContains:
172-
if primaryPurged == "" || comparePrimaryPurged(ctx, primary, primaryPurged, replicaExecuted) {
181+
if primaryPurged == "" {
182+
return innodbcluster.ReplicaGtidRecoverable, nil
183+
}
184+
compareRes, err := comparePrimaryPurged(ctx, primary, primaryPurged, replicaExecuted)
185+
if err != nil {
186+
return "", errors.Wrap(err, "compare primary purged")
187+
}
188+
if compareRes {
173189
return innodbcluster.ReplicaGtidRecoverable, nil
174190
}
175191
return innodbcluster.ReplicaGtidIrrecoverable, nil

‎cmd/bootstrap/gr/recovery_method_test.go‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ import (
1111
)
1212

1313
type mockSQLRunner struct {
14-
sqlResponses map[string]string
14+
sqlResponses map[string]any
1515
gtidExecuted string
1616
gtidPurged string
1717
}
1818

1919
func newMockSQLRunner() *mockSQLRunner {
2020
return &mockSQLRunner{
21-
sqlResponses: make(map[string]string),
21+
sqlResponses: make(map[string]any),
2222
gtidExecuted: "",
2323
gtidPurged: "",
2424
}
@@ -27,10 +27,10 @@ func newMockSQLRunner() *mockSQLRunner {
2727
func (m *mockSQLRunner) runSQL(ctx context.Context, sql string) (SQLResult, error) {
2828
if result, ok := m.sqlResponses[sql]; ok {
2929
return SQLResult{
30-
Rows: []map[string]string{
30+
Rows: []map[string]any{
3131
{
32-
"sub": result,
33-
"GTID_SUBTRACT": result, // for comparePrimaryPurged
32+
"sub": result,
33+
"is_subset": result, // for comparePrimaryPurged
3434
},
3535
},
3636
}, nil
@@ -66,7 +66,6 @@ func TestCompareGTIDs_Equal(t *testing.T) {
6666
mock.setGTIDSubtractResponse("b:1-3", "a:1-5", "")
6767

6868
result, err := compareGTIDs(ctx, mock, "a:1-5", "b:1-3")
69-
7069
if err != nil {
7170
t.Fatalf("unexpected error: %v", err)
7271
}
@@ -84,7 +83,6 @@ func TestCompareGTIDs_AContainsB(t *testing.T) {
8483
mock.setGTIDSubtractResponse("a:1-5", "a:1-10", "")
8584

8685
result, err := compareGTIDs(ctx, mock, "a:1-10", "a:1-5")
87-
8886
if err != nil {
8987
t.Fatalf("unexpected error: %v", err)
9088
}
@@ -102,7 +100,6 @@ func TestCompareGTIDs_BContainsA(t *testing.T) {
102100
mock.setGTIDSubtractResponse("a:1-10", "a:1-5", "a:6-10")
103101

104102
result, err := compareGTIDs(ctx, mock, "a:1-5", "a:1-10")
105-
106103
if err != nil {
107104
t.Fatalf("unexpected error: %v", err)
108105
}
@@ -122,7 +119,6 @@ func TestCompareGTIDs_Disjoint(t *testing.T) {
122119
mock.setGTIDSubtractIntersectionResponse("a:1-5", "b:1-5", "")
123120

124121
result, err := compareGTIDs(ctx, mock, "a:1-5", "b:1-5")
125-
126122
if err != nil {
127123
t.Fatalf("unexpected error: %v", err)
128124
}
@@ -142,7 +138,6 @@ func TestCompareGTIDs_Intersects(t *testing.T) {
142138
mock.setGTIDSubtractIntersectionResponse("a:1-10", "a:6-15", "a:6-10")
143139

144140
result, err := compareGTIDs(ctx, mock, "a:1-10", "a:6-15")
145-
146141
if err != nil {
147142
t.Fatalf("unexpected error: %v", err)
148143
}
@@ -156,7 +151,6 @@ func TestCompareGTIDs_BothEmpty(t *testing.T) {
156151
mock := newMockSQLRunner()
157152

158153
result, err := compareGTIDs(ctx, mock, "", "")
159-
160154
if err != nil {
161155
t.Fatalf("unexpected error: %v", err)
162156
}
@@ -170,7 +164,6 @@ func TestCompareGTIDs_AEmpty(t *testing.T) {
170164
mock := newMockSQLRunner()
171165

172166
result, err := compareGTIDs(ctx, mock, "", "b:1-5")
173-
174167
if err != nil {
175168
t.Fatalf("unexpected error: %v", err)
176169
}
@@ -184,7 +177,6 @@ func TestCompareGTIDs_BEmpty(t *testing.T) {
184177
mock := newMockSQLRunner()
185178

186179
result, err := compareGTIDs(ctx, mock, "a:1-5", "")
187-
188180
if err != nil {
189181
t.Fatalf("unexpected error: %v", err)
190182
}
@@ -207,7 +199,6 @@ func TestCheckReplicaState_New(t *testing.T) {
207199
replica := newMockSQLRunnerWithGTIDs("", "")
208200

209201
result, err := checkReplicaState(ctx, primary, replica)
210-
211202
if err != nil {
212203
t.Fatalf("unexpected error: %v", err)
213204
}
@@ -226,7 +217,6 @@ func TestCheckReplicaState_Identical(t *testing.T) {
226217
primary.setGTIDSubtractResponse("a:1-10", "a:1-10", "")
227218

228219
result, err := checkReplicaState(ctx, primary, replica)
229-
230220
if err != nil {
231221
t.Fatalf("unexpected error: %v", err)
232222
}
@@ -245,7 +235,6 @@ func TestCheckReplicaState_Recoverable_NoPurged(t *testing.T) {
245235
primary.setGTIDSubtractResponse("a:1-5", "a:1-10", "")
246236

247237
result, err := checkReplicaState(ctx, primary, replica)
248-
249238
if err != nil {
250239
t.Fatalf("unexpected error: %v", err)
251240
}
@@ -264,10 +253,9 @@ func TestCheckReplicaState_Recoverable_WithPurgedButOK(t *testing.T) {
264253
primary.setGTIDSubtractResponse("a:1-5", "a:1-10", "")
265254

266255
// comparePrimaryPurged check - purged is subset of replica executed
267-
primary.sqlResponses["SELECT GTID_SUBTRACT('a:1-3', 'a:1-5') = ''"] = "0"
256+
primary.sqlResponses["SELECT GTID_SUBTRACT('a:1-3', 'a:1-5') = '' AS is_subset"] = float64(1.0)
268257

269258
result, err := checkReplicaState(ctx, primary, replica)
270-
271259
if err != nil {
272260
t.Fatalf("unexpected error: %v", err)
273261
}
@@ -286,10 +274,9 @@ func TestCheckReplicaState_Irrecoverable(t *testing.T) {
286274
primary.setGTIDSubtractResponse("a:1-5", "a:1-10", "")
287275

288276
// comparePrimaryPurged check fails - purged has more than replica executed
289-
primary.sqlResponses["SELECT GTID_SUBTRACT('a:1-8', 'a:1-5') = ''"] = "1"
277+
primary.sqlResponses["SELECT GTID_SUBTRACT('a:1-8', 'a:1-5') = '' AS is_subset"] = float64(0.0)
290278

291279
result, err := checkReplicaState(ctx, primary, replica)
292-
293280
if err != nil {
294281
t.Fatalf("unexpected error: %v", err)
295282
}
@@ -309,7 +296,6 @@ func TestCheckReplicaState_Diverged_Intersects(t *testing.T) {
309296
primary.setGTIDSubtractIntersectionResponse("a:1-10", "a:5-15", "a:5-10")
310297

311298
result, err := checkReplicaState(ctx, primary, replica)
312-
313299
if err != nil {
314300
t.Fatalf("unexpected error: %v", err)
315301
}
@@ -329,7 +315,6 @@ func TestCheckReplicaState_Diverged_Disjoint(t *testing.T) {
329315
primary.setGTIDSubtractIntersectionResponse("a:1-5", "b:1-5", "")
330316

331317
result, err := checkReplicaState(ctx, primary, replica)
332-
333318
if err != nil {
334319
t.Fatalf("unexpected error: %v", err)
335320
}
@@ -348,7 +333,6 @@ func TestCheckReplicaState_Diverged_Contained(t *testing.T) {
348333
primary.setGTIDSubtractResponse("a:1-10", "a:1-5", "a:6-10")
349334

350335
result, err := checkReplicaState(ctx, primary, replica)
351-
352336
if err != nil {
353337
t.Fatalf("unexpected error: %v", err)
354338
}

0 commit comments

Comments
 (0)