Merge branch 'master' into fix-charset

This commit is contained in:
Tim Vaillancourt 2022-08-18 22:36:24 +02:00 committed by GitHub
commit 2ad614ab1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 218 additions and 43 deletions

View File

@ -19,3 +19,5 @@ jobs:
- uses: actions/checkout@v3
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: v1.46.2

View File

@ -6,6 +6,10 @@ A more in-depth discussion of various `gh-ost` command line flags: implementatio
Add this flag when executing on Aliyun RDS.
### allow-zero-in-date
Allows the user to make schema changes that include a zero date or zero in date (e.g. adding a `datetime default '0000-00-00 00:00:00'` column), even if global `sql_mode` on MySQL has `NO_ZERO_IN_DATE,NO_ZERO_DATE`.
### azure
Add this flag when executing on Azure Database for MySQL.
@ -242,6 +246,14 @@ Provide a command delimited list of replicas; `gh-ost` will throttle when any of
Provide an HTTP endpoint; `gh-ost` will issue `HEAD` requests on given URL and throttle whenever response status code is not `200`. The URL can be queried and updated dynamically via [interactive commands](interactive-commands.md). Empty URL disables the HTTP check.
### throttle-http-interval-millis
Defaults to 100. Configures the HTTP throttle check interval in milliseconds.
### throttle-http-timeout-millis
Defaults to 1000 (1 second). Configures the HTTP throttler check timeout in milliseconds.
### timestamp-old-table
Makes the _old_ table include a timestamp value. The _old_ table is what the original table is renamed to at the end of a successful migration. For example, if the table is `gh_ost_test`, then the _old_ table would normally be `_gh_ost_test_del`. With `--timestamp-old-table` it would be, for example, `_gh_ost_test_20170221103147_del`.

View File

@ -20,6 +20,8 @@ The `SUPER` privilege is required for `STOP SLAVE`, `START SLAVE` operations. Th
- Switching your `binlog_format` to `ROW`, in the case where it is _not_ `ROW` and you explicitly specified `--switch-to-rbr`
- If your replication is already in RBR (`binlog_format=ROW`) you can specify `--assume-rbr` to avoid the `STOP SLAVE/START SLAVE` operations, hence no need for `SUPER`.
- `gh-ost` uses the `REPEATABLE_READ` transaction isolation level for all MySQL connections, regardless of the server default.
- Running `--test-on-replica`: before the cut-over phase, `gh-ost` stops replication so that you can compare the two tables and satisfy that the migration is sound.
### Limitations

View File

@ -92,6 +92,7 @@ type MigrationContext struct {
AssumeRBR bool
SkipForeignKeyChecks bool
SkipStrictMode bool
AllowZeroInDate bool
NullableUniqueKeyAllowed bool
ApproveRenamedColumns bool
SkipRenamedColumns bool

View File

@ -78,6 +78,7 @@ func main() {
flag.BoolVar(&migrationContext.DiscardForeignKeys, "discard-foreign-keys", false, "DANGER! This flag will migrate a table that has foreign keys and will NOT create foreign keys on the ghost table, thus your altered table will have NO foreign keys. This is useful for intentional dropping of foreign keys")
flag.BoolVar(&migrationContext.SkipForeignKeyChecks, "skip-foreign-key-checks", false, "set to 'true' when you know for certain there are no foreign keys on your table, and wish to skip the time it takes for gh-ost to verify that")
flag.BoolVar(&migrationContext.SkipStrictMode, "skip-strict-mode", false, "explicitly tell gh-ost binlog applier not to enforce strict sql mode")
flag.BoolVar(&migrationContext.AllowZeroInDate, "allow-zero-in-date", false, "explicitly tell gh-ost binlog applier to ignore NO_ZERO_IN_DATE,NO_ZERO_DATE in sql_mode")
flag.BoolVar(&migrationContext.AliyunRDS, "aliyun-rds", false, "set to 'true' when you execute on Aliyun RDS.")
flag.BoolVar(&migrationContext.GoogleCloudPlatform, "gcp", false, "set to 'true' when you execute on a 1st generation Google Cloud Platform (GCP).")
flag.BoolVar(&migrationContext.AzureMySQL, "azure", false, "set to 'true' when you execute on Azure Database on MySQL.")
@ -180,7 +181,7 @@ func main() {
}
if migrationContext.AlterStatement == "" {
log.Fatalf("--alter must be provided and statement must not be empty")
log.Fatal("--alter must be provided and statement must not be empty")
}
parser := sql.NewParserFromAlterStatement(migrationContext.AlterStatement)
migrationContext.AlterStatementOptions = parser.GetAlterStatementOptions()
@ -189,7 +190,7 @@ func main() {
if parser.HasExplicitSchema() {
migrationContext.DatabaseName = parser.GetExplicitSchema()
} else {
log.Fatalf("--database must be provided and database name must not be empty, or --alter must specify database name")
log.Fatal("--database must be provided and database name must not be empty, or --alter must specify database name")
}
}
@ -201,48 +202,48 @@ func main() {
if parser.HasExplicitTable() {
migrationContext.OriginalTableName = parser.GetExplicitTable()
} else {
log.Fatalf("--table must be provided and table name must not be empty, or --alter must specify table name")
log.Fatal("--table must be provided and table name must not be empty, or --alter must specify table name")
}
}
migrationContext.Noop = !(*executeFlag)
if migrationContext.AllowedRunningOnMaster && migrationContext.TestOnReplica {
migrationContext.Log.Fatalf("--allow-on-master and --test-on-replica are mutually exclusive")
migrationContext.Log.Fatal("--allow-on-master and --test-on-replica are mutually exclusive")
}
if migrationContext.AllowedRunningOnMaster && migrationContext.MigrateOnReplica {
migrationContext.Log.Fatalf("--allow-on-master and --migrate-on-replica are mutually exclusive")
migrationContext.Log.Fatal("--allow-on-master and --migrate-on-replica are mutually exclusive")
}
if migrationContext.MigrateOnReplica && migrationContext.TestOnReplica {
migrationContext.Log.Fatalf("--migrate-on-replica and --test-on-replica are mutually exclusive")
migrationContext.Log.Fatal("--migrate-on-replica and --test-on-replica are mutually exclusive")
}
if migrationContext.SwitchToRowBinlogFormat && migrationContext.AssumeRBR {
migrationContext.Log.Fatalf("--switch-to-rbr and --assume-rbr are mutually exclusive")
migrationContext.Log.Fatal("--switch-to-rbr and --assume-rbr are mutually exclusive")
}
if migrationContext.TestOnReplicaSkipReplicaStop {
if !migrationContext.TestOnReplica {
migrationContext.Log.Fatalf("--test-on-replica-skip-replica-stop requires --test-on-replica to be enabled")
migrationContext.Log.Fatal("--test-on-replica-skip-replica-stop requires --test-on-replica to be enabled")
}
migrationContext.Log.Warning("--test-on-replica-skip-replica-stop enabled. We will not stop replication before cut-over. Ensure you have a plugin that does this.")
}
if migrationContext.CliMasterUser != "" && migrationContext.AssumeMasterHostname == "" {
migrationContext.Log.Fatalf("--master-user requires --assume-master-host")
migrationContext.Log.Fatal("--master-user requires --assume-master-host")
}
if migrationContext.CliMasterPassword != "" && migrationContext.AssumeMasterHostname == "" {
migrationContext.Log.Fatalf("--master-password requires --assume-master-host")
migrationContext.Log.Fatal("--master-password requires --assume-master-host")
}
if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS {
migrationContext.Log.Fatalf("--ssl-ca requires --ssl")
migrationContext.Log.Fatal("--ssl-ca requires --ssl")
}
if migrationContext.TLSCertificate != "" && !migrationContext.UseTLS {
migrationContext.Log.Fatalf("--ssl-cert requires --ssl")
migrationContext.Log.Fatal("--ssl-cert requires --ssl")
}
if migrationContext.TLSKey != "" && !migrationContext.UseTLS {
migrationContext.Log.Fatalf("--ssl-key requires --ssl")
migrationContext.Log.Fatal("--ssl-key requires --ssl")
}
if migrationContext.TLSAllowInsecure && !migrationContext.UseTLS {
migrationContext.Log.Fatalf("--ssl-allow-insecure requires --ssl")
migrationContext.Log.Fatal("--ssl-allow-insecure requires --ssl")
}
if *replicationLagQuery != "" {
migrationContext.Log.Warningf("--replication-lag-query is deprecated")
migrationContext.Log.Warning("--replication-lag-query is deprecated")
}
switch *cutOver {

View File

@ -117,6 +117,24 @@ func (this *Applier) validateAndReadTimeZone() error {
return nil
}
// generateSqlModeQuery return a `sql_mode = ...` query, to be wrapped with a `set session` or `set global`,
// based on gh-ost configuration:
// - User may skip strict mode
// - User may allow zero dats or zero in dates
func (this *Applier) generateSqlModeQuery() string {
sqlModeAddendum := `,NO_AUTO_VALUE_ON_ZERO`
if !this.migrationContext.SkipStrictMode {
sqlModeAddendum = fmt.Sprintf("%s,STRICT_ALL_TABLES", sqlModeAddendum)
}
sqlModeQuery := fmt.Sprintf("CONCAT(@@session.sql_mode, ',%s')", sqlModeAddendum)
if this.migrationContext.AllowZeroInDate {
sqlModeQuery = fmt.Sprintf("REPLACE(REPLACE(%s, 'NO_ZERO_IN_DATE', ''), 'NO_ZERO_DATE', '')", sqlModeQuery)
}
sqlModeQuery = fmt.Sprintf("sql_mode = %s", sqlModeQuery)
return sqlModeQuery
}
// readTableColumns reads table columns on applier
func (this *Applier) readTableColumns() (err error) {
this.migrationContext.Log.Infof("Examining table structure on applier")
@ -182,11 +200,33 @@ func (this *Applier) CreateGhostTable() error {
sql.EscapeName(this.migrationContext.DatabaseName),
sql.EscapeName(this.migrationContext.GetGhostTableName()),
)
if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
return err
}
this.migrationContext.Log.Infof("Ghost table created")
return nil
err := func() error {
tx, err := this.db.Begin()
if err != nil {
return err
}
defer tx.Rollback()
sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s'`, this.migrationContext.ApplierTimeZone)
sessionQuery = fmt.Sprintf("%s, %s", sessionQuery, this.generateSqlModeQuery())
if _, err := tx.Exec(sessionQuery); err != nil {
return err
}
if _, err := tx.Exec(query); err != nil {
return err
}
this.migrationContext.Log.Infof("Ghost table created")
if err := tx.Commit(); err != nil {
// Neither SET SESSION nor ALTER are really transactional, so strictly speaking
// there's no need to commit; but let's do this the legit way anyway.
return err
}
return nil
}()
return err
}
// AlterGhost applies `alter` statement on ghost table
@ -201,11 +241,33 @@ func (this *Applier) AlterGhost() error {
sql.EscapeName(this.migrationContext.GetGhostTableName()),
)
this.migrationContext.Log.Debugf("ALTER statement: %s", query)
if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
return err
}
this.migrationContext.Log.Infof("Ghost table altered")
return nil
err := func() error {
tx, err := this.db.Begin()
if err != nil {
return err
}
defer tx.Rollback()
sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s'`, this.migrationContext.ApplierTimeZone)
sessionQuery = fmt.Sprintf("%s, %s", sessionQuery, this.generateSqlModeQuery())
if _, err := tx.Exec(sessionQuery); err != nil {
return err
}
if _, err := tx.Exec(query); err != nil {
return err
}
this.migrationContext.Log.Infof("Ghost table altered")
if err := tx.Commit(); err != nil {
// Neither SET SESSION nor ALTER are really transactional, so strictly speaking
// there's no need to commit; but let's do this the legit way anyway.
return err
}
return nil
}()
return err
}
// AlterGhost applies `alter` statement on ghost table
@ -539,12 +601,9 @@ func (this *Applier) ApplyIterationInsertQuery() (chunkSize int64, rowsAffected
return nil, err
}
defer tx.Rollback()
sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s'`, this.migrationContext.ApplierTimeZone)
sqlModeAddendum := `,NO_AUTO_VALUE_ON_ZERO`
if !this.migrationContext.SkipStrictMode {
sqlModeAddendum = fmt.Sprintf("%s,STRICT_ALL_TABLES", sqlModeAddendum)
}
sessionQuery = fmt.Sprintf("%s, sql_mode = CONCAT(@@session.sql_mode, ',%s')", sessionQuery, sqlModeAddendum)
sessionQuery = fmt.Sprintf("%s, %s", sessionQuery, this.generateSqlModeQuery())
if _, err := tx.Exec(sessionQuery); err != nil {
return nil, err
@ -1056,12 +1115,7 @@ func (this *Applier) ApplyDMLEventQueries(dmlEvents [](*binlog.BinlogDMLEvent))
}
sessionQuery := "SET SESSION time_zone = '+00:00'"
sqlModeAddendum := `,NO_AUTO_VALUE_ON_ZERO`
if !this.migrationContext.SkipStrictMode {
sqlModeAddendum = fmt.Sprintf("%s,STRICT_ALL_TABLES", sqlModeAddendum)
}
sessionQuery = fmt.Sprintf("%s, sql_mode = CONCAT(@@session.sql_mode, ',%s')", sessionQuery, sqlModeAddendum)
sessionQuery = fmt.Sprintf("%s, %s", sessionQuery, this.generateSqlModeQuery())
if _, err := tx.Exec(sessionQuery); err != nil {
return rollback(err)

View File

@ -1,5 +1,5 @@
/*
Copyright 2016 GitHub Inc.
Copyright 2022 GitHub Inc.
See https://github.com/github/gh-ost/blob/master/LICENSE
*/
@ -12,12 +12,14 @@ import (
"fmt"
"io/ioutil"
"net"
"strings"
"github.com/go-sql-driver/mysql"
)
const (
TLS_CONFIG_KEY = "ghost"
transactionIsolation = "REPEATABLE-READ"
TLS_CONFIG_KEY = "ghost"
)
// ConnectionConfig is the minimal configuration required to connect to a MySQL server
@ -112,12 +114,23 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string {
// Wrap IPv6 literals in square brackets
hostname = fmt.Sprintf("[%s]", hostname)
}
interpolateParams := true
// go-mysql-driver defaults to false if tls param is not provided; explicitly setting here to
// simplify construction of the DSN below.
tlsOption := "false"
if this.tlsConfig != nil {
tlsOption = TLS_CONFIG_KEY
}
return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?timeout=%fs&readTimeout=%fs&writeTimeout=%fs&interpolateParams=%t&autocommit=true&charset=utf8mb4,utf8,latin1&tls=%s", this.User, this.Password, hostname, this.Key.Port, databaseName, this.Timeout, this.Timeout, this.Timeout, interpolateParams, tlsOption)
connectionParams := []string{
"autocommit=true",
"charset=utf8mb4,utf8,latin1",
"interpolateParams=true",
fmt.Sprintf("tls=%s", tlsOption),
fmt.Sprintf("transaction_isolation=%q", transactionIsolation),
fmt.Sprintf("timeout=%fs", this.Timeout),
fmt.Sprintf("readTimeout=%fs", this.Timeout),
fmt.Sprintf("writeTimeout=%fs", this.Timeout),
}
return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?%s", this.User, this.Password, hostname, this.Key.Port, databaseName, strings.Join(connectionParams, "&"))
}

View File

@ -1,5 +1,5 @@
/*
Copyright 2016 GitHub Inc.
Copyright 2022 GitHub Inc.
See https://github.com/github/gh-ost/blob/master/LICENSE
*/
@ -67,9 +67,10 @@ func TestGetDBUri(t *testing.T) {
c.Key = InstanceKey{Hostname: "myhost", Port: 3306}
c.User = "gromit"
c.Password = "penguin"
c.Timeout = 1.2345
uri := c.GetDBUri("test")
test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?timeout=0.000000s&readTimeout=0.000000s&writeTimeout=0.000000s&interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=false")
test.S(t).ExpectEquals(uri, `gromit:penguin@tcp(myhost:3306)/test?autocommit=true&charset=utf8mb4,utf8,latin1&interpolateParams=true&tls=false&transaction_isolation="REPEATABLE-READ"&timeout=1.234500s&readTimeout=1.234500s&writeTimeout=1.234500s`)
}
func TestGetDBUriWithTLSSetup(t *testing.T) {
@ -77,8 +78,9 @@ func TestGetDBUriWithTLSSetup(t *testing.T) {
c.Key = InstanceKey{Hostname: "myhost", Port: 3306}
c.User = "gromit"
c.Password = "penguin"
c.Timeout = 1.2345
c.tlsConfig = &tls.Config{}
uri := c.GetDBUri("test")
test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?timeout=0.000000s&readTimeout=0.000000s&writeTimeout=0.000000s&interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=ghost")
test.S(t).ExpectEquals(uri, `gromit:penguin@tcp(myhost:3306)/test?autocommit=true&charset=utf8mb4,utf8,latin1&interpolateParams=true&tls=ghost&transaction_isolation="REPEATABLE-READ"&timeout=1.234500s&readTimeout=1.234500s&writeTimeout=1.234500s`)
}

View File

@ -0,0 +1,20 @@
drop table if exists gh_ost_test;
create table gh_ost_test (
id int unsigned auto_increment,
i int not null,
dt datetime,
primary key(id)
) auto_increment=1;
drop event if exists gh_ost_test;
delimiter ;;
create event gh_ost_test
on schedule every 1 second
starts current_timestamp
ends current_timestamp + interval 60 second
on completion not preserve
enable
do
begin
insert into gh_ost_test values (null, 7, '2010-10-20 10:20:30');
end ;;

View File

@ -0,0 +1 @@
--allow-zero-in-date --alter="change column dt dt datetime not null default '1970-00-00 00:00:00'"

View File

@ -0,0 +1,21 @@
set session sql_mode='';
drop table if exists gh_ost_test;
create table gh_ost_test (
id int unsigned auto_increment,
i int not null,
dt datetime not null default '1970-00-00 00:00:00',
primary key(id)
) auto_increment=1;
drop event if exists gh_ost_test;
delimiter ;;
create event gh_ost_test
on schedule every 1 second
starts current_timestamp
ends current_timestamp + interval 60 second
on completion not preserve
enable
do
begin
insert into gh_ost_test values (null, 7, '2010-10-20 10:20:30');
end ;;

View File

@ -0,0 +1 @@
--allow-zero-in-date --alter="engine=innodb"

View File

@ -0,0 +1,20 @@
drop table if exists gh_ost_test;
create table gh_ost_test (
id int unsigned auto_increment,
i int not null,
dt datetime,
primary key(id)
) auto_increment=1;
drop event if exists gh_ost_test;
delimiter ;;
create event gh_ost_test
on schedule every 1 second
starts current_timestamp
ends current_timestamp + interval 60 second
on completion not preserve
enable
do
begin
insert into gh_ost_test values (null, 7, '2010-10-20 10:20:30');
end ;;

View File

@ -0,0 +1 @@
Invalid default value for 'dt'

View File

@ -0,0 +1 @@
--alter="change column dt dt datetime not null default '1970-00-00 00:00:00'"

View File

@ -0,0 +1,21 @@
set session sql_mode='';
drop table if exists gh_ost_test;
create table gh_ost_test (
id int unsigned auto_increment,
i int not null,
dt datetime not null default '1970-00-00 00:00:00',
primary key(id)
) auto_increment=1;
drop event if exists gh_ost_test;
delimiter ;;
create event gh_ost_test
on schedule every 1 second
starts current_timestamp
ends current_timestamp + interval 60 second
on completion not preserve
enable
do
begin
insert into gh_ost_test values (null, 7, '2010-10-20 10:20:30');
end ;;

View File

@ -0,0 +1 @@
Invalid default value for 'dt'

View File

@ -0,0 +1 @@
--alter="engine=innodb"