Skip to content

Commit 653b99e

Browse files
authored
feat: 🦭 MariaDB support 🦭 (#1724)
* feat: support MariaDB 11.4 and 11.8 gh-ost mostly works against MariaDB unchanged; this makes the few MariaDB-specific spots correct and runs the replica test suite against MariaDB 11.4 and 11.8 (LTS) alongside the MySQL/Percona flavors. Core: - ReplicaTermFor keeps the legacy slave/master replication terminology on MariaDB. MariaDB reports versions >= 10 (greater than the 8.4 cutoff) but never adopted the new replica/source terms, and its SHOW REPLICA STATUS still emits legacy column names, so the MySQL 8.4 term map doesn't apply. - Accept BINLOG MONITOR as well as REPLICATION CLIENT in the grant check (MariaDB 10.5+ renamed the privilege). Test harness (localtests): - test.sh: pick slave/Master terminology for MariaDB, wait for replica catch-up via MASTER_GTID_WAIT/gtid_binlog_pos, and skip gtid_mode-gated tests where @@gtid_mode is unsupported instead of aborting the run. - Resolve the per-flavor config dir by specificity (docker/<flavor>-<major.minor>, then <flavor>-<major>, then bare <flavor>) so versions that share one config live in a single dir. All MariaDB versions use one docker/mariadb dir (log-bin + ROW + log-slave-updates, native-auth users, MariaDB-GTID replication, MYSQL_CLIENT=mariadb since 11.x dropped the mysql-named client); a version (or major line) needing different config overrides it by adding its own dir. The compose healthcheck honours MYSQL_CLIENT. - Make the zero-date failure tests deterministic across flavors with an explicit sql_mode (NO_ZERO_IN_DATE,NO_ZERO_DATE,...); they relied on MySQL's default sql_mode, which MariaDB doesn't share. - Drop the incidental NOT NULL from the generated-column tests (MariaDB forbids NOT NULL on generated columns) so they run on MariaDB; keep generated-columns-unique skipped (it puts a generated column in the PRIMARY KEY, which MariaDB forbids, Error 1903). - Add mariadb:11.4.12 and mariadb:11.8.8 to the CI matrix. * feat: support MariaDB 10.5, 10.6 and 10.11 Add the MariaDB 10.x LTS lines (10.5, 10.6, 10.11) to the replica test suite. MariaDB versions below 11.1 need one core fix: gh-ost passes transaction_isolation as a DSN param, which the driver applies as `SET transaction_isolation=<v>` on each connection. That system variable does not exist on MariaDB < 11.1 (which has only tx_isolation), so the connection fails with Error 1193 "Unknown system variable 'transaction_isolation'". tx_isolation was in turn removed in MySQL 8.0, so no single variable name is portable across the supported servers. - mysql.OpenDB strips the transaction_isolation param and instead runs the SQL-standard `SET SESSION TRANSACTION ISOLATION LEVEL <level>` on each new connection via a connector wrapper, accepted by every supported MySQL and MariaDB version. GetDBUri still emits the param unchanged, so DSN construction and its tests are unaffected. - Add mariadb:10.5.29, mariadb:10.6.27 and mariadb:10.11.18 to the CI matrix.
1 parent 06b3a7e commit 653b99e

20 files changed

Lines changed: 179 additions & 26 deletions

File tree

.github/workflows/replica-tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
strategy:
1111
fail-fast: false
1212
matrix:
13-
image: ['mysql:5.7.41','mysql:8.0.41','mysql:8.4.3','percona/percona-server:8.0.41-32']
13+
image: ['mysql:5.7.41','mysql:8.0.41','mysql:8.4.3','percona/percona-server:8.0.41-32','mariadb:10.5.29','mariadb:10.6.27','mariadb:10.11.18','mariadb:11.4.12','mariadb:11.8.8']
1414
env:
1515
TEST_MYSQL_IMAGE: ${{ matrix.image }}
1616

go/logic/applier.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ func (apl *Applier) AcquireMigrationLock(ctx context.Context) error {
174174
// Use a dedicated *sql.DB so the pinned connection does not consume a
175175
// slot in apl.db's small pool (mysql.MaxDBPoolConnections).
176176
lockURI := apl.connectionConfig.GetDBUri(apl.migrationContext.DatabaseName)
177-
lockDB, err := gosql.Open("mysql", lockURI)
177+
lockDB, err := mysql.OpenDB(lockURI)
178178
if err != nil {
179179
return fmt.Errorf("failed to open migration lock DB: %w", err)
180180
}

go/logic/inspect.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,8 @@ func (isp *Inspector) validateGrants() error {
244244
if strings.Contains(grant, `SUPER`) && strings.Contains(grant, ` ON *.*`) {
245245
foundSuper = true
246246
}
247-
if strings.Contains(grant, `REPLICATION CLIENT`) && strings.Contains(grant, ` ON *.*`) {
247+
// MariaDB renamed REPLICATION CLIENT to BINLOG MONITOR (10.5+); accept either.
248+
if (strings.Contains(grant, `REPLICATION CLIENT`) || strings.Contains(grant, `BINLOG MONITOR`)) && strings.Contains(grant, ` ON *.*`) {
248249
foundReplicationClient = true
249250
}
250251
if strings.Contains(grant, `REPLICATION SLAVE`) && strings.Contains(grant, ` ON *.*`) {

go/mysql/replica_terminology_map.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package mysql
22

33
import (
4+
"strings"
5+
46
version "github.com/hashicorp/go-version"
57
)
68

@@ -25,6 +27,13 @@ var MysqlReplicaTermMap = map[string]string{
2527
}
2628

2729
func ReplicaTermFor(mysqlVersion string, term string) string {
30+
// MariaDB reports versions >= 10, which compare greater than the 8.4
31+
// cutoff, but it never adopted the new replica/source terminology. Keep
32+
// the legacy terms for it.
33+
if strings.Contains(strings.ToLower(mysqlVersion), "mariadb") {
34+
return term
35+
}
36+
2837
vs, err := version.NewVersion(mysqlVersion)
2938
if err != nil {
3039
// default to returning the same term if we cannot determine the version
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
Copyright 2025 GitHub Inc.
3+
See https://github.com/github/gh-ost/blob/master/LICENSE
4+
*/
5+
6+
package mysql
7+
8+
import "testing"
9+
10+
func TestReplicaTermForMariaDB11KeepsLegacyTerms(t *testing.T) {
11+
tests := []string{
12+
"master status",
13+
"slave status",
14+
"Slave_IO_Running",
15+
"Seconds_Behind_Master",
16+
}
17+
18+
version := "11.4.8-MariaDB-ubu2404-log"
19+
for _, term := range tests {
20+
if actual := ReplicaTermFor(version, term); actual != term {
21+
t.Fatalf("term %q: expected MariaDB to keep legacy term %q, got %q", term, term, actual)
22+
}
23+
}
24+
}

go/mysql/utils.go

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@
66
package mysql
77

88
import (
9+
"context"
910
gosql "database/sql"
11+
"database/sql/driver"
1012
"fmt"
1113
"strings"
1214
"sync"
1315
"time"
1416

1517
"github.com/github/gh-ost/go/sql"
1618

19+
gomysql "github.com/go-sql-driver/mysql"
1720
"github.com/openark/golib/log"
1821
"github.com/openark/golib/sqlutils"
1922
)
@@ -48,14 +51,68 @@ func (rlg *ReplicationLagResult) HasLag() bool {
4851
var knownDBs map[string]*gosql.DB = make(map[string]*gosql.DB)
4952
var knownDBsMutex = &sync.Mutex{}
5053

54+
// initConnector wraps a driver.Connector to run a fixed set of statements on
55+
// every newly established connection (e.g. setting the transaction isolation
56+
// level), which the DSN-param mechanism can't express portably.
57+
type initConnector struct {
58+
driver.Connector
59+
statements []string
60+
}
61+
62+
func (c *initConnector) Connect(ctx context.Context) (driver.Conn, error) {
63+
conn, err := c.Connector.Connect(ctx)
64+
if err != nil {
65+
return nil, err
66+
}
67+
execer, ok := conn.(driver.ExecerContext)
68+
if !ok {
69+
conn.Close()
70+
return nil, fmt.Errorf("mysql: driver connection does not implement driver.ExecerContext")
71+
}
72+
for _, stmt := range c.statements {
73+
if _, err := execer.ExecContext(ctx, stmt, nil); err != nil {
74+
conn.Close()
75+
return nil, err
76+
}
77+
}
78+
return conn, nil
79+
}
80+
81+
// OpenDB opens a MySQL connection pool for the given DSN. A transaction_isolation
82+
// param is applied via the SQL-standard "SET SESSION TRANSACTION ISOLATION LEVEL"
83+
// statement on each new connection rather than being passed to the driver as a
84+
// system variable:
85+
// - "transaction_isolation" doesn't exist on MariaDB < 11.1, while
86+
// - "tx_isolation" doesn't exist on MySQL 8.0+ anymore, so no single variable name is portable.
87+
// The standard statement is accepted by every supported MySQL and MariaDB version.
88+
func OpenDB(mysql_uri string) (*gosql.DB, error) {
89+
cfg, err := gomysql.ParseDSN(mysql_uri)
90+
if err != nil {
91+
return nil, err
92+
}
93+
var statements []string
94+
if iso := strings.Trim(cfg.Params["transaction_isolation"], `"`); iso != "" {
95+
delete(cfg.Params, "transaction_isolation")
96+
statements = append(statements, "SET SESSION TRANSACTION ISOLATION LEVEL "+strings.ReplaceAll(iso, "-", " "))
97+
}
98+
connector, err := gomysql.NewConnector(cfg)
99+
if err != nil {
100+
return nil, err
101+
}
102+
if len(statements) > 0 {
103+
connector = &initConnector{Connector: connector, statements: statements}
104+
}
105+
return gosql.OpenDB(connector), nil
106+
}
107+
51108
func GetDB(migrationUuid string, mysql_uri string) (db *gosql.DB, exists bool, err error) {
52109
cacheKey := migrationUuid + ":" + mysql_uri
53110

54111
knownDBsMutex.Lock()
55112
defer knownDBsMutex.Unlock()
56113

57114
if db, exists = knownDBs[cacheKey]; !exists {
58-
db, err = gosql.Open("mysql", mysql_uri)
115+
db, err = OpenDB(mysql_uri)
59116
if err != nil {
60117
return nil, false, err
61118
}
@@ -88,7 +145,7 @@ func GetReplicationLagFromSlaveStatus(dbVersion string, informationSchemaDb *gos
88145
func GetMasterKeyFromSlaveStatus(dbVersion string, connectionConfig *ConnectionConfig) (masterKey *InstanceKey, err error) {
89146
currentUri := connectionConfig.GetDBUri("information_schema")
90147
// This function is only called once, okay to not have a cached connection pool
91-
db, err := gosql.Open("mysql", currentUri)
148+
db, err := OpenDB(currentUri)
92149
if err != nil {
93150
return nil, err
94151
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
--alter="add column sum_ab int as (a + b) virtual not null"
1+
--alter="add column sum_ab int as (a + b) virtual"

localtests/generated-columns-rename/create.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ create table gh_ost_test (
33
id int auto_increment,
44
a int not null,
55
b int not null,
6-
sum_ab int as (a + b) virtual not null,
6+
sum_ab int as (a + b) virtual,
77
primary key(id)
88
) auto_increment=1;
99

0 commit comments

Comments
 (0)