diff --git a/.github/workflows/replica-tests.yml b/.github/workflows/replica-tests.yml index d418efd02..281a19359 100644 --- a/.github/workflows/replica-tests.yml +++ b/.github/workflows/replica-tests.yml @@ -10,7 +10,7 @@ jobs: strategy: fail-fast: false matrix: - image: ['mysql:5.7.41','mysql:8.0.41','mysql:8.4.3','percona/percona-server:8.0.41-32'] + 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'] env: TEST_MYSQL_IMAGE: ${{ matrix.image }} diff --git a/go/logic/applier.go b/go/logic/applier.go index 29269ed11..50ad45ce2 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -174,7 +174,7 @@ func (apl *Applier) AcquireMigrationLock(ctx context.Context) error { // Use a dedicated *sql.DB so the pinned connection does not consume a // slot in apl.db's small pool (mysql.MaxDBPoolConnections). lockURI := apl.connectionConfig.GetDBUri(apl.migrationContext.DatabaseName) - lockDB, err := gosql.Open("mysql", lockURI) + lockDB, err := mysql.OpenDB(lockURI) if err != nil { return fmt.Errorf("failed to open migration lock DB: %w", err) } diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 982be1d98..f010f6e9a 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -244,7 +244,8 @@ func (isp *Inspector) validateGrants() error { if strings.Contains(grant, `SUPER`) && strings.Contains(grant, ` ON *.*`) { foundSuper = true } - if strings.Contains(grant, `REPLICATION CLIENT`) && strings.Contains(grant, ` ON *.*`) { + // MariaDB renamed REPLICATION CLIENT to BINLOG MONITOR (10.5+); accept either. + if (strings.Contains(grant, `REPLICATION CLIENT`) || strings.Contains(grant, `BINLOG MONITOR`)) && strings.Contains(grant, ` ON *.*`) { foundReplicationClient = true } if strings.Contains(grant, `REPLICATION SLAVE`) && strings.Contains(grant, ` ON *.*`) { diff --git a/go/mysql/replica_terminology_map.go b/go/mysql/replica_terminology_map.go index ce6ebbd16..140fcbaca 100644 --- a/go/mysql/replica_terminology_map.go +++ b/go/mysql/replica_terminology_map.go @@ -1,6 +1,8 @@ package mysql import ( + "strings" + version "github.com/hashicorp/go-version" ) @@ -25,6 +27,13 @@ var MysqlReplicaTermMap = map[string]string{ } func ReplicaTermFor(mysqlVersion string, term string) string { + // MariaDB reports versions >= 10, which compare greater than the 8.4 + // cutoff, but it never adopted the new replica/source terminology. Keep + // the legacy terms for it. + if strings.Contains(strings.ToLower(mysqlVersion), "mariadb") { + return term + } + vs, err := version.NewVersion(mysqlVersion) if err != nil { // default to returning the same term if we cannot determine the version diff --git a/go/mysql/replica_terminology_map_test.go b/go/mysql/replica_terminology_map_test.go new file mode 100644 index 000000000..f628343f3 --- /dev/null +++ b/go/mysql/replica_terminology_map_test.go @@ -0,0 +1,24 @@ +/* + Copyright 2025 GitHub Inc. + See https://github.com/github/gh-ost/blob/master/LICENSE +*/ + +package mysql + +import "testing" + +func TestReplicaTermForMariaDB11KeepsLegacyTerms(t *testing.T) { + tests := []string{ + "master status", + "slave status", + "Slave_IO_Running", + "Seconds_Behind_Master", + } + + version := "11.4.8-MariaDB-ubu2404-log" + for _, term := range tests { + if actual := ReplicaTermFor(version, term); actual != term { + t.Fatalf("term %q: expected MariaDB to keep legacy term %q, got %q", term, term, actual) + } + } +} diff --git a/go/mysql/utils.go b/go/mysql/utils.go index 654cb099e..2ad200bdd 100644 --- a/go/mysql/utils.go +++ b/go/mysql/utils.go @@ -6,7 +6,9 @@ package mysql import ( + "context" gosql "database/sql" + "database/sql/driver" "fmt" "strings" "sync" @@ -14,6 +16,7 @@ import ( "github.com/github/gh-ost/go/sql" + gomysql "github.com/go-sql-driver/mysql" "github.com/openark/golib/log" "github.com/openark/golib/sqlutils" ) @@ -48,6 +51,60 @@ func (rlg *ReplicationLagResult) HasLag() bool { var knownDBs map[string]*gosql.DB = make(map[string]*gosql.DB) var knownDBsMutex = &sync.Mutex{} +// initConnector wraps a driver.Connector to run a fixed set of statements on +// every newly established connection (e.g. setting the transaction isolation +// level), which the DSN-param mechanism can't express portably. +type initConnector struct { + driver.Connector + statements []string +} + +func (c *initConnector) Connect(ctx context.Context) (driver.Conn, error) { + conn, err := c.Connector.Connect(ctx) + if err != nil { + return nil, err + } + execer, ok := conn.(driver.ExecerContext) + if !ok { + conn.Close() + return nil, fmt.Errorf("mysql: driver connection does not implement driver.ExecerContext") + } + for _, stmt := range c.statements { + if _, err := execer.ExecContext(ctx, stmt, nil); err != nil { + conn.Close() + return nil, err + } + } + return conn, nil +} + +// OpenDB opens a MySQL connection pool for the given DSN. A transaction_isolation +// param is applied via the SQL-standard "SET SESSION TRANSACTION ISOLATION LEVEL" +// statement on each new connection rather than being passed to the driver as a +// system variable: +// - "transaction_isolation" doesn't exist on MariaDB < 11.1, while +// - "tx_isolation" doesn't exist on MySQL 8.0+ anymore, so no single variable name is portable. +// The standard statement is accepted by every supported MySQL and MariaDB version. +func OpenDB(mysql_uri string) (*gosql.DB, error) { + cfg, err := gomysql.ParseDSN(mysql_uri) + if err != nil { + return nil, err + } + var statements []string + if iso := strings.Trim(cfg.Params["transaction_isolation"], `"`); iso != "" { + delete(cfg.Params, "transaction_isolation") + statements = append(statements, "SET SESSION TRANSACTION ISOLATION LEVEL "+strings.ReplaceAll(iso, "-", " ")) + } + connector, err := gomysql.NewConnector(cfg) + if err != nil { + return nil, err + } + if len(statements) > 0 { + connector = &initConnector{Connector: connector, statements: statements} + } + return gosql.OpenDB(connector), nil +} + func GetDB(migrationUuid string, mysql_uri string) (db *gosql.DB, exists bool, err error) { cacheKey := migrationUuid + ":" + mysql_uri @@ -55,7 +112,7 @@ func GetDB(migrationUuid string, mysql_uri string) (db *gosql.DB, exists bool, e defer knownDBsMutex.Unlock() if db, exists = knownDBs[cacheKey]; !exists { - db, err = gosql.Open("mysql", mysql_uri) + db, err = OpenDB(mysql_uri) if err != nil { return nil, false, err } @@ -88,7 +145,7 @@ func GetReplicationLagFromSlaveStatus(dbVersion string, informationSchemaDb *gos func GetMasterKeyFromSlaveStatus(dbVersion string, connectionConfig *ConnectionConfig) (masterKey *InstanceKey, err error) { currentUri := connectionConfig.GetDBUri("information_schema") // This function is only called once, okay to not have a cached connection pool - db, err := gosql.Open("mysql", currentUri) + db, err := OpenDB(currentUri) if err != nil { return nil, err } diff --git a/localtests/fail-datetime-with-zero/sql_mode b/localtests/fail-datetime-with-zero/sql_mode new file mode 100644 index 000000000..ae71ccee1 --- /dev/null +++ b/localtests/fail-datetime-with-zero/sql_mode @@ -0,0 +1 @@ +STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION diff --git a/localtests/fail-existing-datetime-with-zero/sql_mode b/localtests/fail-existing-datetime-with-zero/sql_mode new file mode 100644 index 000000000..ae71ccee1 --- /dev/null +++ b/localtests/fail-existing-datetime-with-zero/sql_mode @@ -0,0 +1 @@ +STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION diff --git a/localtests/generated-columns-add/extra_args b/localtests/generated-columns-add/extra_args index b2bf5bc00..965b796c6 100644 --- a/localtests/generated-columns-add/extra_args +++ b/localtests/generated-columns-add/extra_args @@ -1 +1 @@ ---alter="add column sum_ab int as (a + b) virtual not null" +--alter="add column sum_ab int as (a + b) virtual" diff --git a/localtests/generated-columns-rename/create.sql b/localtests/generated-columns-rename/create.sql index e244ca3c0..e9ae19149 100644 --- a/localtests/generated-columns-rename/create.sql +++ b/localtests/generated-columns-rename/create.sql @@ -3,7 +3,7 @@ create table gh_ost_test ( id int auto_increment, a int not null, b int not null, - sum_ab int as (a + b) virtual not null, + sum_ab int as (a + b) virtual, primary key(id) ) auto_increment=1; diff --git a/localtests/generated-columns-rename/extra_args b/localtests/generated-columns-rename/extra_args index 6a190988a..b451e80b0 100644 --- a/localtests/generated-columns-rename/extra_args +++ b/localtests/generated-columns-rename/extra_args @@ -1 +1 @@ ---alter="change sum_ab total_ab int as (a + b) virtual not null" --approve-renamed-columns +--alter="change sum_ab total_ab int as (a + b) virtual" --approve-renamed-columns diff --git a/localtests/generated-columns-unique/ignore_versions b/localtests/generated-columns-unique/ignore_versions index cf02abe24..c7ba43c2e 100644 --- a/localtests/generated-columns-unique/ignore_versions +++ b/localtests/generated-columns-unique/ignore_versions @@ -1 +1 @@ -Percona \ No newline at end of file +Percona|mariadb diff --git a/localtests/generated-columns/create.sql b/localtests/generated-columns/create.sql index 357d4a3b4..1173b99fa 100644 --- a/localtests/generated-columns/create.sql +++ b/localtests/generated-columns/create.sql @@ -3,7 +3,7 @@ create table gh_ost_test ( id int auto_increment, a int not null, b int not null, - sum_ab int as (a + b) virtual not null, + sum_ab int as (a + b) virtual, primary key(id) ) auto_increment=1; diff --git a/localtests/test.sh b/localtests/test.sh index a9f4a82a5..49ed780d6 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -95,7 +95,12 @@ verify_master_and_replica() { # Cache replica_terminology and seconds_behind_source values to avoid later checks. mysql_version="$(gh-ost-test-mysql-replica -s -s -e "select @@version")" mysql_version_comment="$(gh-ost-test-mysql-master -s -s -e "select @@version_comment")" - if [[ $mysql_version =~ "8.4" ]]; then + if [[ "$mysql_version" == *MariaDB* ]]; then + # MariaDB reports versions >= 10 but never adopted the replica/source + # terminology, so it keeps the legacy slave/Master wording. + replica_terminology="slave" + seconds_behind_source="Seconds_Behind_Master" + elif [[ $mysql_version =~ "8.4" ]]; then replica_terminology="replica" seconds_behind_source="Seconds_Behind_Source" else @@ -129,13 +134,25 @@ echo_dot() { # there is no lag; waits up to 10s otherwise. # Relies on GTID (gtid_mode=ON); falls back to a short sleep if GTID is unavailable. wait_replica_caught_up() { - local master_gtid - master_gtid="$(gh-ost-test-mysql-master -s -s -e "select @@global.gtid_executed" 2>/dev/null)" - if [ -n "$master_gtid" ]; then - gh-ost-test-mysql-replica -s -s -e "do WAIT_FOR_EXECUTED_GTID_SET('${master_gtid}', 10)" >/dev/null 2>&1 + if [[ "$mysql_version" == *MariaDB* ]]; then + # MariaDB has no gtid_executed / WAIT_FOR_EXECUTED_GTID_SET; use its own + # binlog GTID position and MASTER_GTID_WAIT. + local master_pos + master_pos="$(gh-ost-test-mysql-master -s -s -e "select @@global.gtid_binlog_pos")" + if [ -n "$master_pos" ]; then + gh-ost-test-mysql-replica -s -s -e "select master_gtid_wait('${master_pos}', 10)" >/dev/null 2>&1 else - sleep 1 - fi + sleep 1 + fi + return + fi + local master_gtid + master_gtid="$(gh-ost-test-mysql-master -s -s -e "select @@global.gtid_executed" 2>/dev/null)" + if [ -n "$master_gtid" ]; then + gh-ost-test-mysql-replica -s -s -e "do WAIT_FOR_EXECUTED_GTID_SET('${master_gtid}', 10)" >/dev/null 2>&1 + else + sleep 1 + fi } start_replication() { @@ -291,10 +308,16 @@ test_single() { if [ -f $tests_path/$test_name/gtid_mode ]; then target_gtid_mode=$(cat $tests_path/$test_name/gtid_mode) if [ "$current_gtid_mode" != "$target_gtid_mode" ]; then - echo "gtid_mode is ${current_gtid_mode}, expected ${target_gtid_mode}" - exit 1 - fi + # MariaDB has no @@gtid_mode (reports "unsupported"); these tests + # target MySQL GTID behaviour, so skip rather than abort the run. + if [ "$current_gtid_mode" = "unsupported" ]; then + echo -n " skipping (gtid_mode unsupported)" + return 0 + fi + echo "gtid_mode is ${current_gtid_mode}, expected ${target_gtid_mode}" + exit 1 fi + fi if [ -f $tests_path/$test_name/sql_mode ]; then gh-ost-test-mysql-master --default-character-set=utf8mb4 test -e "set @@global.sql_mode='$(cat $tests_path/$test_name/sql_mode)'" diff --git a/script/docker-compose.yml b/script/docker-compose.yml index ae2222127..c0150423b 100644 --- a/script/docker-compose.yml +++ b/script/docker-compose.yml @@ -12,7 +12,9 @@ services: - $CONF_PATH/create_user.sql:/docker-entrypoint-initdb.d/create_user.sql:ro - $CONF_PATH/common.cnf:$MYSQL_CONFD/common.cnf:ro healthcheck: - test: "mysql -h 127.0.0.1 -uroot -p$MYSQL_ROOT_PASSWORD -e 'select 1;' || exit 1" + # Client binary differs per flavor: MariaDB 11.x dropped the mysql-named + # symlinks, so its flavor.env sets MYSQL_CLIENT=mariadb. + test: "${MYSQL_CLIENT:-mysql} -h 127.0.0.1 -uroot -p$MYSQL_ROOT_PASSWORD -e 'select 1;' || exit 1" interval: 5s # Generous first-boot window: emulated amd64 5.7 on arm64 can take 60s+ to # init a fresh datadir. Health failures during start_period don't count, so diff --git a/script/docker-gh-ost-replica-tests b/script/docker-gh-ost-replica-tests index e7c45f467..4ff4b0f39 100755 --- a/script/docker-gh-ost-replica-tests +++ b/script/docker-gh-ost-replica-tests @@ -110,16 +110,30 @@ compose_setup() { # Per-version config lives in docker/- (e.g. mysql-8.0) # Derive from the image repo basename and from the tag, # so registry/repo prefixes like percona/percona-server:8.0.41-32 resolve correctly. - local repo tag flavor major_minor + local repo tag flavor major_minor major repo="${TEST_MYSQL_IMAGE%%:*}" tag="${TEST_MYSQL_IMAGE##*:}" flavor="${repo##*/}" major_minor="$(echo "$tag" | cut -d. -f1-2)" - export CONF_PATH="$compose_path/docker/${flavor}-${major_minor}" - if [[ ! -d "$CONF_PATH" ]]; then - echo "No config dir for image '$TEST_MYSQL_IMAGE' (expected $CONF_PATH)" >&2 - exit 1 + major="$(echo "$tag" | cut -d. -f1)" + + # Resolve by specificity: exact -, then -, + # then bare . Versions that share one config (e.g. all MariaDB) live + # in a single docker/ dir; a version (or major line) needing + # different config overrides it by adding its own more-specific dir. + local candidate + CONF_PATH="" + for candidate in "${flavor}-${major_minor}" "${flavor}-${major}" "${flavor}"; do + if [[ -d "$compose_path/docker/$candidate" ]]; then + CONF_PATH="$compose_path/docker/$candidate" + break + fi + done + if [[ -z "$CONF_PATH" ]]; then + echo "No config dir for image '$TEST_MYSQL_IMAGE' (looked for docker/${flavor}-${major_minor}, docker/${flavor}-${major}, docker/${flavor})" >&2 + exit 1 fi + export CONF_PATH # Per-flavor knobs, with defaults. Each flavor dir may override these in an # optional flavor.env (e.g. Percona reads config from /etc/my.cnf.d). diff --git a/script/docker/mariadb/common.cnf b/script/docker/mariadb/common.cnf new file mode 100644 index 000000000..a0023d628 --- /dev/null +++ b/script/docker/mariadb/common.cnf @@ -0,0 +1,10 @@ +[mysqld] +character-set-server=utf8mb4 +# MariaDB ships the binary log off and binlog_format=MIXED by default; gh-ost +# tails ROW events from the replica's own binlog under --test-on-replica, so +# the replica needs log-bin + ROW + log-slave-updates. MariaDB tracks GTID +# (gtid_binlog_pos) automatically once the binary log is on; it has no +# gtid_mode/enforce_gtid_consistency knobs (those are MySQL-only). +log-bin=mariadb-bin +binlog-format=ROW +log-slave-updates=ON diff --git a/script/docker/mariadb/create_user.sql b/script/docker/mariadb/create_user.sql new file mode 100644 index 000000000..fd5639054 --- /dev/null +++ b/script/docker/mariadb/create_user.sql @@ -0,0 +1,5 @@ +create user if not exists 'repl'@'%' identified by 'repl'; +grant replication slave on *.* to 'repl'@'%'; +create user if not exists 'gh-ost'@'%' identified by 'gh-ost'; +grant all on *.* to 'gh-ost'@'%'; +flush privileges; diff --git a/script/docker/mariadb/flavor.env b/script/docker/mariadb/flavor.env new file mode 100644 index 000000000..45f98d6d7 --- /dev/null +++ b/script/docker/mariadb/flavor.env @@ -0,0 +1,4 @@ +# MariaDB 11.x ships only the `mariadb`-named client; the `mysql` symlink is +# gone. The compose healthcheck runs inside the container, so it needs the +# real binary name. +export MYSQL_CLIENT=mariadb diff --git a/script/docker/mariadb/start_replication.sql b/script/docker/mariadb/start_replication.sql new file mode 100644 index 000000000..94820bb28 --- /dev/null +++ b/script/docker/mariadb/start_replication.sql @@ -0,0 +1,2 @@ +change master to master_host='mysql-primary', master_port=3307, master_user='repl', master_password='repl', master_use_gtid=slave_pos; +start slave;