Skip to content

Commit 8917d1d

Browse files
authored
mysqltopo: use the block-mysql driver (#22)
* mysqltopo: use the block-mysql driver strata links github.com/block/mysql — Block's fork of go-sql-driver/mysql — for capabilities upstream does not carry. That fork is moving from a `replace` directive to its own module path (block/mysql#3), because `replace` is not inherited across module boundaries and so cannot reach consumers of a library. Once the path differs, a binary linking both packages gets two distinct `*mysql.MySQLError` types, and `errors.As` across that boundary silently returns false. convertError here depends on exactly that assertion to map 1062 to NodeExists and 1213/1205 to Timeout, and strata's own test helpers assert on the errors this package returns — so mysqltopo and strata must agree on which package the type comes from. Scope is deliberately mysqltopo only: it is Block-added, so editing it costs nothing when merging upstream vitess. The remaining go-sql-driver imports live in upstream-owned endtoend tests, which keep using upstream — the module stays in go.mod and both can coexist. The dependency is pinned to the block/mysql PR branch and must be re-pointed at its master commit before this merges. Verified: go/vt/topo/mysqltopo tests pass against MySQL 8.0.44. * go mod tidy: block/mysql is a direct dependency now * Pin block/mysql at merged master (block/mysql#3) * mysqltopo: state the driver invariant this package actually holds The import comment claimed one driver in the binary; eight end-to-end test files still import go-sql-driver, so that is false. The local invariant is both true and sufficient: every *sql.DB this package opens uses the driver whose *mysql.MySQLError convertError asserts on. Add a package const for the name and use it at all five sql.Open sites, so that invariant is one grep rather than five, and fix the convertError comment to credit block-mysql.
1 parent 46598d3 commit 8917d1d

8 files changed

Lines changed: 33 additions & 16 deletions

File tree

go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ require (
9595
github.com/aws/aws-sdk-go-v2/feature/s3/transfermanager v0.1.15
9696
github.com/aws/aws-sdk-go-v2/service/s3 v1.99.0
9797
github.com/aws/smithy-go v1.24.3
98+
github.com/block/mysql v0.0.0-20260906201522-a3178f8dca69
9899
github.com/bndr/gotabulate v1.1.2
99100
github.com/brianvoe/gofakeit/v7 v7.14.1
100101
github.com/dustin/go-humanize v1.0.1
@@ -121,7 +122,7 @@ require (
121122
)
122123

123124
require (
124-
filippo.io/edwards25519 v1.1.1 // indirect
125+
filippo.io/edwards25519 v1.2.0 // indirect
125126
github.com/DataDog/datadog-agent/comp/core/tagger/origindetection v0.77.3 // indirect
126127
github.com/DataDog/datadog-agent/pkg/opentelemetry-mapping-go/otlp/attributes v0.77.3 // indirect
127128
github.com/DataDog/datadog-agent/pkg/proto v0.77.3 // indirect

go.sum

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ cloud.google.com/go/storage v1.62.1 h1:Os0G3XbUbjZumkpDUf2Y0rLoXJTCF1kU2kWUujKYX
2222
cloud.google.com/go/storage v1.62.1/go.mod h1:cpYz/kRVZ+UQAF1uHeea10/9ewcRbxGoGNKsS9daSXA=
2323
cloud.google.com/go/trace v1.13.0 h1:RfqsqPOiSCG8ql50UZt5F65KrVa1zbY9mJrO7xvZfbE=
2424
cloud.google.com/go/trace v1.13.0/go.mod h1:r+bdAn16dKLSV1G2D5v3e58IlQlizfxWrUfjx7kM7X0=
25-
filippo.io/edwards25519 v1.1.1 h1:YpjwWWlNmGIDyXOn8zLzqiD+9TyIlPhGFG96P39uBpw=
26-
filippo.io/edwards25519 v1.1.1/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
25+
filippo.io/edwards25519 v1.2.0 h1:crnVqOiS4jqYleHd9vaKZ+HKtHfllngJIiOpNpoJsjo=
26+
filippo.io/edwards25519 v1.2.0/go.mod h1:xzAOLCNug/yB62zG1bQ8uziwrIqIuxhctzJT18Q77mc=
2727
github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6 h1:He8afgbRMd7mFxO99hRNu+6tazq8nFF9lIwo9JFroBk=
2828
github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8=
2929
github.com/Azure/azure-pipeline-go v0.2.3 h1:7U9HBg1JFK3jHl5qmo4CTZKFTVgMwdFHMVtCdfBE21U=
@@ -156,6 +156,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
156156
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
157157
github.com/bitfield/gotestdox v0.2.2 h1:x6RcPAbBbErKLnapz1QeAlf3ospg8efBsedU93CDsnE=
158158
github.com/bitfield/gotestdox v0.2.2/go.mod h1:D+gwtS0urjBrzguAkTM2wodsTQYFHdpx8eqRJ3N+9pY=
159+
github.com/block/mysql v0.0.0-20260906201522-a3178f8dca69 h1:rCWVZKT5PdrdfosvMavnnfUBAdtaM3iepvADt5pMSJI=
160+
github.com/block/mysql v0.0.0-20260906201522-a3178f8dca69/go.mod h1:KEo73lbxXs9cFlq+x3Z35UqGg3MTxAPfjDOR/ob/iik=
159161
github.com/bndr/gotabulate v1.1.2 h1:yC9izuZEphojb9r+KYL4W9IJKO/ceIO8HDwxMA24U4c=
160162
github.com/bndr/gotabulate v1.1.2/go.mod h1:0+8yUgaPTtLRTjf49E8oju7ojpU11YmXyvq1LbPAb3U=
161163
github.com/brianvoe/gofakeit/v7 v7.14.1 h1:a7fe3fonbj0cW3wgl5VwIKfZtiH9C3cLnwcIXWT7sow=

go/vt/topo/mysqltopo/main_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"path"
2727
"testing"
2828

29-
"github.com/go-sql-driver/mysql"
29+
"github.com/block/mysql"
3030
"github.com/stretchr/testify/require"
3131
"go.uber.org/goleak"
3232

@@ -125,7 +125,7 @@ func createTestServer(t *testing.T, schemaName string) (*Server, string, func())
125125
schemaName = generateRandomSchemaName()
126126
}
127127
cfg.DBName = "" // to create schema
128-
baseDB, err := sql.Open("mysql", cfg.FormatDSN())
128+
baseDB, err := sql.Open(driverName, cfg.FormatDSN())
129129
require.NoError(t, err)
130130

131131
_, err = baseDB.Exec(fmt.Sprintf("CREATE DATABASE IF NOT EXISTS `%s`", schemaName))

go/vt/topo/mysqltopo/notification.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import (
2929
"sync/atomic"
3030
"time"
3131

32-
mysqldriver "github.com/go-sql-driver/mysql"
32+
mysqldriver "github.com/block/mysql"
3333

3434
"vitess.io/vitess/go/mysql"
3535
"vitess.io/vitess/go/mysql/replication"
@@ -209,7 +209,7 @@ func newNotificationSystem(schemaName, serverAddr string) (*notificationSystem,
209209
cfg.TLSConfig = "rds-topo"
210210
}
211211

212-
db, err := sql.Open("mysql", cfg.FormatDSN())
212+
db, err := sql.Open(driverName, cfg.FormatDSN())
213213
if err != nil {
214214
return nil, fmt.Errorf("failed to connect to MySQL: %v", err)
215215
}
@@ -236,7 +236,7 @@ func newNotificationSystem(schemaName, serverAddr string) (*notificationSystem,
236236
// Create connection parameters for binlog streaming
237237
cfg.DBName = schemaName
238238

239-
// Parse host and port from cfg.Addr. The go-sql-driver/mysql DSN allows
239+
// Parse host and port from cfg.Addr. The driver's DSN format allows
240240
// addresses without a port (e.g. `tcp(localhost)/db`); fall back to the
241241
// default MySQL port in that case.
242242
host, portStr, err := net.SplitHostPort(cfg.Addr)

go/vt/topo/mysqltopo/notification_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import (
5656
"testing"
5757
"time"
5858

59-
"github.com/go-sql-driver/mysql"
59+
"github.com/block/mysql"
6060
"github.com/stretchr/testify/assert"
6161
"github.com/stretchr/testify/require"
6262

@@ -765,7 +765,7 @@ func TestNotificationSystemReconnection(t *testing.T) {
765765
require.Equal(t, initialData, current.Contents)
766766

767767
// Create a separate connection to monitor and kill connections
768-
monitorDB, err := sql.Open("mysql", mySQLTopoTestAddr)
768+
monitorDB, err := sql.Open(driverName, mySQLTopoTestAddr)
769769
require.NoError(t, err)
770770
defer monitorDB.Close()
771771

go/vt/topo/mysqltopo/server.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,14 @@ import (
5151
"sync"
5252
"time"
5353

54-
"github.com/go-sql-driver/mysql"
54+
// Block's fork of go-sql-driver/mysql, registered as "block-mysql". strata
55+
// links the fork for capabilities upstream does not carry; this package
56+
// follows so that every *sql.DB it opens is served by the same driver whose
57+
// *mysql.MySQLError type convertError below asserts on. That is a statement
58+
// about this package, not about the binary: elsewhere in the repo, end-to-end
59+
// tests still import upstream, and an error crossing from one of those would
60+
// not match the assertion.
61+
"github.com/block/mysql"
5562
"github.com/spf13/pflag"
5663

5764
"vitess.io/vitess/go/mysql/sqlerror"
@@ -62,6 +69,13 @@ import (
6269
)
6370

6471
const (
72+
// driverName is the database/sql driver every connection in this package is
73+
// opened with. It has to be block/mysql's registered name and not upstream's
74+
// "mysql": convertError asserts on block/mysql's *mysql.MySQLError, and a
75+
// *sql.DB opened with a different driver would return a type that assertion
76+
// silently misses.
77+
driverName = "block-mysql"
78+
6579
// DefaultSchema is the default database schema name for MySQL topo
6680
DefaultSchema = "topo"
6781

@@ -269,7 +283,7 @@ func connect(cfg *mysql.Config) (*sql.DB, error) {
269283
cfg.TLSConfig = "rds-topo"
270284
}
271285

272-
db, err := sql.Open("mysql", cfg.FormatDSN())
286+
db, err := sql.Open(driverName, cfg.FormatDSN())
273287
if err != nil {
274288
return nil, fmt.Errorf("failed to connect to MySQL topo at %s (schema %q, user %q): %v", cfg.Addr, cfg.DBName, cfg.User, err)
275289
}
@@ -539,7 +553,7 @@ func convertError(err error, path string) error {
539553
return topo.NewError(topo.NoNode, path)
540554
}
541555

542-
// Handle MySQL-specific errors. go-sql-driver returns *mysql.MySQLError,
556+
// Handle MySQL-specific errors. block-mysql returns *mysql.MySQLError,
543557
// which carries the server error number directly; its message format
544558
// ("Error 1062 (23000): ...") is not recognized by
545559
// sqlerror.NewSQLErrorFromError, so check the typed error first and only

go/vt/topo/mysqltopo/server_error_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"sync"
2525
"testing"
2626

27-
"github.com/go-sql-driver/mysql"
27+
"github.com/block/mysql"
2828
"github.com/stretchr/testify/require"
2929

3030
"vitess.io/vitess/go/vt/topo"

go/vt/topo/mysqltopo/server_resolve_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"net"
2323
"testing"
2424

25-
"github.com/go-sql-driver/mysql"
25+
"github.com/block/mysql"
2626
"github.com/stretchr/testify/require"
2727
)
2828

@@ -36,7 +36,7 @@ func newRawSchema(t *testing.T) (baseDB *sql.DB, schemaDSN, schemaName string, c
3636

3737
schemaName = generateRandomSchemaName()
3838
cfg.DBName = ""
39-
baseDB, err = sql.Open("mysql", cfg.FormatDSN())
39+
baseDB, err = sql.Open(driverName, cfg.FormatDSN())
4040
require.NoError(t, err)
4141
_, err = baseDB.Exec(fmt.Sprintf("CREATE DATABASE IF NOT EXISTS `%s`", schemaName))
4242
require.NoError(t, err)

0 commit comments

Comments
 (0)