chore: merge upstream (#29)

* Add script and docs for linter (github#1151)

* Enable more `golang-ci` linters (github#1149)

* Only build RPM and deb packages for amd64

* Convert character to bytes and insert into table using latin1

* delete junk files

* restore connection charset to utf8mb4

* Allow zero in dates (github#1161)

* Merge pull request #31 from openark/zero-date

Support zero date and zero in date, via dedicated command line flag

* Merge pull request #32 from openark/existing-date-with-zero

Support tables with existing zero dates

* Remove un-needed ignore_versions file

* Fix new lint errors from golang-ci update

Co-authored-by: Shlomi Noach <[email protected]>

* Add missing doc from PR github#1131 (github#1162)

* Set a transaction isolation level for MySQL connections (github#1156)

* Set transaction isolation in connections

* Revert load_map.go change

* Var rename

* Restore comment

* Some fix to unit tests.

* convert to bytes if character string without charsetConversion.

* chore: remove duplicate word in comments (github#1175)

Signed-off-by: Abirdcfly <[email protected]>

Signed-off-by: Abirdcfly <[email protected]>

* Improve applier `.ReadMigrationRangeValues()` func accuracy (github#1164)

* Use a transaction in applier `ReadMigrationRangeValues` func

* Private func names

* Add basic tests for applier (github#1165)

* Add basic tests for applier

* Add header

* Add basic test for inspector (github#1166)

* Add basic test for inspector

* Add header

* Fix return

* Add basic tests to migrator (github#1168)

* add-rocksdb-as-transactional-engine

* Add basic test for hooks (github#1179)

* Enable more `golangci-lint` linters (github#1181)

* Print status to migration context logger

* fix CI tests to ubuntu-20.04 because ubuntu-22.04 (current -latest) doesn't support MySQL 5.7

* temp commit to investigate datetime-with-zero test failure

* more testing

* add extra debugging output

* debugging

* add error detection for test setup, sort tests to make it easier to track progress

* fix broken test by removing invalid insert statement

* Fix: Change table name

table name is 'tbl' not 'tble'

* Attempt instant DDL if supported

* minor cleanup

* Add tests, incorporate feedback

* Improve docs

* Address PR feedback

* Make it clear in docs it is disabled by default but safe.

* Update go/logic/migrator.go

Co-authored-by: dm-2 <[email protected]>

* remove useless func per review

* support rocksdb as transactional engine

* Modify tests to support rocksdb tests

* SetConnectionConfig

* add support for rocksdb

* add support for rocksdb

* add percona to versions in workflows

* add description and optimize tests

* Apply suggestions from code review

Co-authored-by: dm-2 <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tim Vaillancourt <[email protected]>

* Update go/logic/applier.go

Signed-off-by: Abirdcfly <[email protected]>
Co-authored-by: Tim Vaillancourt <[email protected]>
Co-authored-by: dm-2 <[email protected]>
Co-authored-by: wangzihuacool <[email protected]>
Co-authored-by: wangzihuacool <[email protected]>
Co-authored-by: Shlomi Noach <[email protected]>
Co-authored-by: Abirdcfly <[email protected]>
Co-authored-by: Nicholas Calugar <[email protected]>
Co-authored-by: Hasan Mshawrab <[email protected]>
Co-authored-by: Morgan Tocker <[email protected]>
Co-authored-by: Morgan Tocker <[email protected]>
Co-authored-by: lukelewang <[email protected]>

.github/workflows/ci.yml

@@ -5,7 +5,7 @@ on: [pull_request]
 jobs:
   build:
 
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
 
     steps:
     - uses: actions/checkout@v2

.github/workflows/golangci-lint.yml

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

.github/workflows/replica-tests.yml

@@ -5,10 +5,10 @@ on: [pull_request]
 jobs:
   build:
 
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     strategy:
       matrix:
-        version: [mysql-5.7.25,mysql-8.0.16]
+        version: [mysql-5.7.25,mysql-8.0.16,PerconaServer-8.0.21]
 
     steps:
     - uses: actions/checkout@v2

.golangci.yml

@@ -1,13 +1,30 @@
 run:
   timeout: 5m
-  modules-download-mode: readonly
-
 linters:
   disable:
     - errcheck
   enable:
+    - bodyclose
+    - containedctx
+    - contextcheck
+    - dogsled
+    - durationcheck
+    - errname
+    - errorlint
+    - execinquery
+    - gofmt
+    - ifshort
+    - misspell
+    - nilerr
+    - nilnil
     - noctx
+    - nolintlint
+    - nosprintfhostport
+    - prealloc
     - rowserrcheck
     - sqlclosecheck
+    - unconvert
+    - unparam
     - unused
-
+    - wastedassign
+    - whitespace

build.sh

@@ -35,7 +35,8 @@ function build {
 
   (cd $buildpath && tar cfz ./gh-ost-binary-${osshort}-${GOARCH}-${timestamp}.tar.gz $target)
 
-  if [ "$GOOS" == "linux" ] ; then
+  # build RPM and deb for Linux, x86-64 only
+  if [ "$GOOS" == "linux" ] && [ "$GOARCH" == "amd64" ] ; then
     echo "Creating Distro full packages"
     builddir=$(setuptree)
     cp $buildpath/$target $builddir/gh-ost/usr/bin

doc/coding-ghost.md

@@ -5,7 +5,7 @@
 Getting started with gh-ost development is simple!
 
 - First obtain the repository with `git clone` or `go get`.
-- From inside of the repository run `script/cibuild`
+- From inside of the repository run `script/cibuild`.
 - This will bootstrap the environment if needed, format the code, build the code, and then run the unit test.
 
 ## CI build workflow
@@ -14,6 +14,12 @@ Getting started with gh-ost development is simple!
 
 If additional steps are needed, please add them into this workflow so that the workflow remains simple.
 
+## `golang-ci` linter
+
+To enfore best-practices, Pull Requests are automatically linted by [`golang-ci`](https://golangci-lint.run/). The linter config is located at [`.golangci.yml`](https://github.com/github/gh-ost/blob/master/.golangci.yml) and the `golangci-lint` GitHub Action is located at [`.github/workflows/golangci-lint.yml`](https://github.com/github/gh-ost/blob/master/.github/workflows/golangci-lint.yml).
+
+To run the `golang-ci` linters locally _(recommended before push)_, use `script/lint`.
+
 ## Notes:
 
 Currently, `script/ensure-go-installed` will install `go` for Mac OS X and Linux. We welcome PR's to add other platforms.

doc/command-line-flags.md

@@ -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.
@@ -41,6 +45,22 @@ If you happen to _know_ your servers use RBR (Row Based Replication, i.e. `binlo
 Skipping this step means `gh-ost` would not need the `SUPER` privilege in order to operate.
 You may want to use this on Amazon RDS.
 
+### attempt-instant-ddl
+
+MySQL 8.0 supports "instant DDL" for some operations. If an alter statement can be completed with instant DDL, only a metadata change is required internally. Instant operations include:
+
+- Adding a column
+- Dropping a column
+- Dropping an index
+- Extending a varchar column
+- Adding a virtual generated column
+
+It is not reliable to parse the `ALTER` statement to determine if it is instant or not. This is because the table might be in an older row format, or have some other incompatibility that is difficult to identify.
+
+`--attempt-instant-ddl` is disabled by default, but the risks of enabling it are relatively minor: `gh-ost` may need to acquire a metadata lock at the start of the operation. This is not a problem for most scenarios, but it could be a problem for users that start the DDL during a period with long running transactions.
+
+`gh-ost` will automatically fallback to the normal DDL process if the attempt to use instant DDL is unsuccessful.
+
 ### conf
 
 `--conf=/path/to/my.cnf`: file where credentials are specified. Should be in (or contain) the following format:
@@ -226,6 +246,18 @@ Allows `gh-ost` to connect to the MySQL servers using encrypted connections, but
 
 `--ssl-key=/path/to/ssl-key.key`: SSL private key file (in PEM format).
 
+### storage-engine
+Default is `innodb`, and `rocksdb` support is currently experimental. InnoDB and RocksDB are both transactional engines, supporting both shared and exclusive row locks.
+
+But RocksDB currently lacks a few features support compared to InnoDB:
+- Gap Locks
+- Foreign Key
+- Generated Columns
+- Spatial
+- Geometry
+
+When `--storage-engine=rocksdb`, `gh-ost` will make some changes necessary (e.g. sets isolation level to `READ_COMMITTED`) to support RocksDB.
+
 ### test-on-replica
 
 Issue the migration on a replica; do not modify data on master. Useful for validating, testing and benchmarking. See [`testing-on-replica`](testing-on-replica.md)
@@ -242,6 +274,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`.

doc/requirements-and-limitations.md

@@ -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

doc/shared-key.md

@@ -29,7 +29,7 @@ CREATE TABLE tbl (
 
 (This is also the definition of the _ghost_ table, except that that table would be called `_tbl_gho`). 
 
-In this migration, the _before_ and _after_ versions contain the same unique not-null key (the PRIMARY KEY). To run this migration, `gh-ost` would iterate through the `tbl` table using the primary key, copy rows from `tbl` to the _ghost_ table `_tbl_gho` in primary key order, while also applying the binlog event writes from `tble` onto `_tbl_gho`.
+In this migration, the _before_ and _after_ versions contain the same unique not-null key (the PRIMARY KEY). To run this migration, `gh-ost` would iterate through the `tbl` table using the primary key, copy rows from `tbl` to the _ghost_ table `_tbl_gho` in primary key order, while also applying the binlog event writes from `tbl` onto `_tbl_gho`.
 
 The applying of the binlog events is what requires the shared unique key. For example, an `UPDATE` statement to `tbl` translates to a `REPLACE` statement which `gh-ost` applies to `_tbl_gho`. A `REPLACE` statement expects to insert or replace an existing row based on its row's values and the table's unique key constraints. In particular, if inserting that row would result in a unique key violation (e.g., a row with that primary key already exists), it would _replace_ that existing row with the new values.
 

go/base/context.go

@@ -92,6 +92,7 @@ type MigrationContext struct {
 	AssumeRBR                bool
 	SkipForeignKeyChecks     bool
 	SkipStrictMode           bool
+	AllowZeroInDate          bool
 	NullableUniqueKeyAllowed bool
 	ApproveRenamedColumns    bool
 	SkipRenamedColumns       bool
@@ -100,6 +101,7 @@ type MigrationContext struct {
 	AliyunRDS                bool
 	GoogleCloudPlatform      bool
 	AzureMySQL               bool
+	AttemptInstantDDL        bool
 
 	config            ContextConfig
 	configMutex       *sync.Mutex
@@ -289,6 +291,19 @@ func NewMigrationContext() *MigrationContext {
 	}
 }
 
+func (this *MigrationContext) SetConnectionConfig(storageEngine string) error {
+	var transactionIsolation string
+	switch storageEngine {
+	case "rocksdb":
+		transactionIsolation = "READ-COMMITTED"
+	default:
+		transactionIsolation = "REPEATABLE-READ"
+	}
+	this.InspectorConnectionConfig.TransactionIsolation = transactionIsolation
+	this.ApplierConnectionConfig.TransactionIsolation = transactionIsolation
+	return nil
+}
+
 func getSafeTableName(baseName string, suffix string) string {
 	name := fmt.Sprintf("~%s_%s", baseName, suffix)
 	if len(name) <= mysql.MaxTableNameLength {
@@ -438,6 +453,10 @@ func (this *MigrationContext) IsTransactionalTable() bool {
 		{
 			return true
 		}
+	case "rocksdb":
+		{
+			return true
+		}
 	}
 	return false
 }
@@ -869,7 +888,7 @@ func (this *MigrationContext) ReadConfigFile() error {
 	if cfg.Section("osc").HasKey("chunk_size") {
 		this.config.Osc.Chunk_Size, err = cfg.Section("osc").Key("chunk_size").Int64()
 		if err != nil {
-			return fmt.Errorf("Unable to read osc chunk size: %s", err.Error())
+			return fmt.Errorf("Unable to read osc chunk size: %w", err)
 		}
 	}
 
@@ -884,7 +903,7 @@ func (this *MigrationContext) ReadConfigFile() error {
 	if cfg.Section("osc").HasKey("max_lag_millis") {
 		this.config.Osc.Max_Lag_Millis, err = cfg.Section("osc").Key("max_lag_millis").Int64()
 		if err != nil {
-			return fmt.Errorf("Unable to read max lag millis: %s", err.Error())
+			return fmt.Errorf("Unable to read max lag millis: %w", err)
 		}
 	}
 

go/cmd/gh-ost/main.go

@@ -67,6 +67,9 @@ func main() {
 	flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)")
 	flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)")
 	flag.StringVar(&migrationContext.AlterStatement, "alter", "", "alter statement (mandatory)")
+	flag.BoolVar(&migrationContext.AttemptInstantDDL, "attempt-instant-ddl", false, "Attempt to use instant DDL for this migration first")
+	storageEngine := flag.String("storage-engine", "innodb", "Specify table storage engine (default: 'innodb'). When 'rocksdb': the session transaction isolation level is changed from REPEATABLE_READ to READ_COMMITTED.")
+
 	flag.BoolVar(&migrationContext.CountTableRows, "exact-rowcount", false, "actually count table rows as opposed to estimate them (results in more accurate progress estimation)")
 	flag.BoolVar(&migrationContext.ConcurrentCountTableRows, "concurrent-rowcount", true, "(with --exact-rowcount), when true (default): count rows after row-copy begins, concurrently, and adjust row estimate later on; when false: first count rows, then start row copy")
 	flag.BoolVar(&migrationContext.AllowedRunningOnMaster, "allow-on-master", false, "allow this migration to run directly on master. Preferably it would run on a replica")
@@ -78,6 +81,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,6 +184,10 @@ func main() {
 		migrationContext.Log.SetLevel(zap.ErrorLevel)
 	}
 
+	if err := migrationContext.SetConnectionConfig(*storageEngine); err != nil {
+		migrationContext.Log.Fatale(err)
+	}
+
 	if migrationContext.AlterStatement == "" {
 		migrationContext.Log.Fatalf("--alter must be provided and statement must not be empty")
 	}
@@ -207,43 +215,46 @@ func main() {
 	}
 	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")
+	}
+	if *storageEngine == "rocksdb" {
+		migrationContext.Log.Warning("RocksDB storage engine support is experimental")
 	}
 
 	switch *cutOver {
@@ -271,7 +282,7 @@ func main() {
 	}
 	if *askPass {
 		fmt.Println("Password:")
-		bytePassword, err := term.ReadPassword(int(syscall.Stdin))
+		bytePassword, err := term.ReadPassword(syscall.Stdin)
 		if err != nil {
 			migrationContext.Log.Fatale(err)
 		}
@@ -301,10 +312,9 @@ func main() {
 	acceptSignals(migrationContext)
 
 	migrator := logic.NewMigrator(migrationContext, AppVersion)
-	err := migrator.Migrate()
-	if err != nil {
+	if err := migrator.Migrate(); err != nil {
 		migrator.ExecOnFailureHook()
 		migrationContext.Log.Fatale(err)
 	}
-	fmt.Fprintf(os.Stdout, "# Done\n")
+	fmt.Fprintln(os.Stdout, "# Done")
 }