-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Implement GTID tracking. #1633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Implement GTID tracking. #1633
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2602,6 +2602,144 @@ func TestExecMultipleResults(t *testing.T) { | |||||||||||||||||||||
}) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
func TestGTIDTracking(t *testing.T) { | ||||||||||||||||||||||
ctx := context.Background() | ||||||||||||||||||||||
runTests(t, dsn+"&trackSessionState=true", func(dbt *DBTest) { | ||||||||||||||||||||||
dbt.mustExec(` | ||||||||||||||||||||||
CREATE TABLE test ( | ||||||||||||||||||||||
id INT NOT NULL AUTO_INCREMENT, | ||||||||||||||||||||||
value VARCHAR(255), | ||||||||||||||||||||||
PRIMARY KEY (id) | ||||||||||||||||||||||
)`) | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Check the current gtid_mode | ||||||||||||||||||||||
var gtidMode string | ||||||||||||||||||||||
if err := dbt.db.QueryRow("SELECT @@global.gtid_mode").Scan(>idMode); err != nil { | ||||||||||||||||||||||
t.Fatalf("failed to get gtid_mode: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if gtidMode == "OFF" { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = OFF_PERMISSIVE") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to change gtid_mode: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
defer func() { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = OFF") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to reset gtid_mode: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}() | ||||||||||||||||||||||
|
||||||||||||||||||||||
gtidMode = "OFF_PERMISSIVE" | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if gtidMode == "OFF_PERMISSIVE" { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = ON_PERMISSIVE") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to change gtid_mode: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
defer func() { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = OFF_PERMISSIVE") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to reset gtid_mode: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}() | ||||||||||||||||||||||
Comment on lines
+2642
to
+2646
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid calling Similar to the previous instance, replace Suggested fix: defer func() {
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = OFF_PERMISSIVE")
if err != nil {
- t.Fatalf("failed while trying to reset gtid_mode: %v", err)
+ t.Errorf("failed while trying to reset gtid_mode: %v", err)
}
}() 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
gtidMode = "ON_PERMISSIVE" | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
var enforceGTIDConsistency string | ||||||||||||||||||||||
if err := dbt.db.QueryRow("SELECT @@global.enforce_gtid_consistency").Scan(&enforceGTIDConsistency); err != nil { | ||||||||||||||||||||||
t.Fatalf("failed to get enforce_gtid_consistency: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if enforceGTIDConsistency == "OFF" { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL enforce_gtid_consistency = ON") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to change enforce_gtid_consistency: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
defer func() { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL enforce_gtid_consistency = OFF") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to reset enforce_gtid_consistency: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}() | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if gtidMode == "ON_PERMISSIVE" { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = ON") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to change gtid_mode: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
defer func() { | ||||||||||||||||||||||
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = ON_PERMISSIVE") | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed while trying to reset gtid_mode: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}() | ||||||||||||||||||||||
Comment on lines
+2675
to
+2679
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid calling Using Suggested fix: defer func() {
_, err := dbt.db.Exec("SET GLOBAL gtid_mode = ON_PERMISSIVE")
if err != nil {
- t.Fatalf("failed while trying to reset gtid_mode: %v", err)
+ t.Errorf("failed while trying to reset gtid_mode: %v", err)
}
}() 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
gtidMode = "ON" | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
conn, err := dbt.db.Conn(ctx) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatalf("failed to connect: %v", err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
defer conn.Close() | ||||||||||||||||||||||
|
||||||||||||||||||||||
var gtid string | ||||||||||||||||||||||
|
||||||||||||||||||||||
conn.Raw(func(conn any) error { | ||||||||||||||||||||||
c := conn.(*mysqlConn) | ||||||||||||||||||||||
|
||||||||||||||||||||||
Comment on lines
+2693
to
+2694
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check type assertion when casting to The type assertion Suggested fix: - c := conn.(*mysqlConn)
+ c, ok := conn.(*mysqlConn)
+ if !ok {
+ return fmt.Errorf("expected *mysqlConn, got %T", conn)
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
res, err := c.Exec("INSERT INTO test (value) VALUES ('a'), ('b')", nil) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatal(err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
gtid, err = res.(Result).LastGTID() | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatal(err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if gtid != "" { | ||||||||||||||||||||||
t.Fatalf("expected empty gtid, got %v", gtid) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
_, err = c.Exec("SET SESSION session_track_gtids = ALL_GTIDS", nil) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatal(err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
res, err = c.Exec("INSERT INTO test (value) VALUES ('a'), ('b')", nil) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatal(err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
gtid, err = res.(Result).LastGTID() | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
t.Fatal(err) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if gtid == "" { | ||||||||||||||||||||||
t.Fatal("expected non-empty gtid") | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+2697
to
+2726
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid calling Calling Suggested fix: conn.Raw(func(conn any) error {
c, ok := conn.(*mysqlConn)
if !ok {
return fmt.Errorf("expected *mysqlConn, got %T", conn)
}
res, err := c.Exec("INSERT INTO test (value) VALUES ('a'), ('b')", nil)
if err != nil {
- t.Fatal(err)
+ return err
}
gtid, err = res.(Result).LastGTID()
if err != nil {
- t.Fatal(err)
+ return err
}
if gtid != "" {
- t.Fatalf("expected empty gtid, got %v", gtid)
+ return fmt.Errorf("expected empty gtid, got %v", gtid)
}
_, err = c.Exec("SET SESSION session_track_gtids = ALL_GTIDS", nil)
if err != nil {
- t.Fatal(err)
+ return err
}
res, err = c.Exec("INSERT INTO test (value) VALUES ('a'), ('b')", nil)
if err != nil {
- t.Fatal(err)
+ return err
}
gtid, err = res.(Result).LastGTID()
if err != nil {
- t.Fatal(err)
+ return err
}
if gtid == "" {
- t.Fatal("expected non-empty gtid")
+ return fmt.Errorf("expected non-empty gtid")
}
return nil
})
+if err != nil {
+ t.Fatal(err)
+}
|
||||||||||||||||||||||
|
||||||||||||||||||||||
return nil | ||||||||||||||||||||||
}) | ||||||||||||||||||||||
|
||||||||||||||||||||||
var gtidExecuted string | ||||||||||||||||||||||
err = conn.QueryRowContext(ctx, "SELECT @@global.gtid_executed").Scan(>idExecuted) | ||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||
dbt.Fatalf("%s", err.Error()) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if gtidExecuted != gtid { | ||||||||||||||||||||||
t.Fatalf("expected gtid %v, got %v", gtidExecuted, gtid) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}) | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
// tests if rows are set in a proper state if some results were ignored before | ||||||||||||||||||||||
// calling rows.NextResultSet. | ||||||||||||||||||||||
func TestSkipResults(t *testing.T) { | ||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -19,9 +19,17 @@ import "database/sql/driver" | |||||||||||||||||||
// res.(mysql.Result).AllRowsAffected() | ||||||||||||||||||||
type Result interface { | ||||||||||||||||||||
driver.Result | ||||||||||||||||||||
|
||||||||||||||||||||
// LastGTID returns the GTID of the last result, if available. | ||||||||||||||||||||
LastGTID() (string, error) | ||||||||||||||||||||
|
||||||||||||||||||||
// AllLastGTIDs returns a slice containing | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incomplete method comment for The comment for // AllLastGTIDs returns a slice containing Please complete the comment to accurately describe what the method returns. |
||||||||||||||||||||
AllLastGTIDs() []string | ||||||||||||||||||||
|
||||||||||||||||||||
// AllRowsAffected returns a slice containing the affected rows for each | ||||||||||||||||||||
// executed statement. | ||||||||||||||||||||
AllRowsAffected() []int64 | ||||||||||||||||||||
|
||||||||||||||||||||
// AllLastInsertIds returns a slice containing the last inserted ID for each | ||||||||||||||||||||
// executed statement. | ||||||||||||||||||||
AllLastInsertIds() []int64 | ||||||||||||||||||||
|
@@ -31,6 +39,7 @@ type mysqlResult struct { | |||||||||||||||||||
// One entry in both slices is created for every executed statement result. | ||||||||||||||||||||
affectedRows []int64 | ||||||||||||||||||||
insertIds []int64 | ||||||||||||||||||||
gtids []string | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func (res *mysqlResult) LastInsertId() (int64, error) { | ||||||||||||||||||||
|
@@ -41,10 +50,18 @@ func (res *mysqlResult) RowsAffected() (int64, error) { | |||||||||||||||||||
return res.affectedRows[len(res.affectedRows)-1], nil | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func (res *mysqlResult) LastGTID() (string, error) { | ||||||||||||||||||||
return res.gtids[len(res.gtids)-1], nil | ||||||||||||||||||||
} | ||||||||||||||||||||
Comment on lines
+53
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential panic in If Consider adding a check to ensure func (res *mysqlResult) LastGTID() (string, error) {
+ if len(res.gtids) == 0 {
+ return "", errors.New("no GTIDs available")
+ }
return res.gtids[len(res.gtids)-1], nil
} Alternatively, define the expected behavior when 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||
|
||||||||||||||||||||
func (res *mysqlResult) AllLastInsertIds() []int64 { | ||||||||||||||||||||
return append([]int64{}, res.insertIds...) // defensive copy | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func (res *mysqlResult) AllRowsAffected() []int64 { | ||||||||||||||||||||
return append([]int64{}, res.affectedRows...) // defensive copy | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func (res *mysqlResult) AllLastGTIDs() []string { | ||||||||||||||||||||
return append([]string{}, res.gtids...) | ||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid calling
t.Fatalf
inside deferred functionsUsing
t.Fatalf
inside a deferred function can lead to unexpected behavior because it may not stop the test execution as intended. Instead, uset.Errorf
to report the error and ensure the test fails by checking for errors after the deferred function executes.Suggested fix:
📝 Committable suggestion