Tweaks

Author: timvaillancourt

go/cmd/gh-ost-localtests/main.go

@@ -25,10 +25,10 @@ func main() {
 	var localtestsDir, testName string
 	var cnf localtests.Config
 
-	flag.StringVar(&cnf.Username, "username", "root", "mysql username")
-	flag.StringVar(&cnf.Password, "password", "", "mysql password")
-	flag.StringVar(&cnf.Host, "host", "127.0.0.1", "mysql host")
-	flag.Int64Var(&cnf.Port, "port", 3306, "mysql port")
+	flag.StringVar(&cnf.Host, "host", localtests.DefaultHost, "mysql host")
+	flag.Int64Var(&cnf.Port, "port", localtests.DefaultPort, "mysql port")
+	flag.StringVar(&cnf.Username, "username", localtests.DefaultUsername, "mysql username")
+	flag.StringVar(&cnf.Password, "password", localtests.DefaultPassword, "mysql password")
 	flag.StringVar(&localtestsDir, "tests-dir", filepath.Join(pwd, "localtests"), "path to localtests directory")
 	flag.StringVar(&testName, "test", "", "run a single test by name (default: run all tests)")
 	flag.BoolVar(&testNoop, "test-noop", false, "run a single noop migration, eg: --alter='ENGINE=InnoDB'")
@@ -53,12 +53,18 @@ func main() {
 	}
 	defer db.Close()
 
+	if err = localtests.WaitForMySQLAvailable(db); err != nil {
+		log.Fatalf("Failed to setup database client: %+v", err)
+	}
+
 	var tests []localtests.Test
 	if testNoop {
 		tests = []localtests.Test{
 			{
-				Name:      "noop",
-				ExtraArgs: `--alter='ENGINE=InnoDB'`,
+				Name: "noop",
+				ExtraArgs: []string{
+					`--alter='ENGINE=InnoDB'`,
+				},
 			},
 		}
 	} else {

go/localtests/localtests.go

@@ -2,32 +2,37 @@ package localtests
 
 import (
 	"database/sql"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"log"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/google/shlex"
 )
 
 const (
-	testUsername     = "gh-ost"
-	testPassword     = "gh-ost"
-	testDatabase     = "test"
-	testTable        = "gh_ost_test"
-	testSocketFile   = "/tmp/gh-ost.test.sock"
-	throttleFlagFile = "/tmp/gh-ost-test.ghost.throttle.flag"
-	throttleQuery    = "select timestampdiff(second, min(last_update), now()) < 5 from _gh_ost_test_ghc"
+	PrimaryHost            = "primary"
+	DefaultHost            = "replica"
+	DefaultPort      int64 = 3306
+	DefaultUsername        = "gh-ost"
+	DefaultPassword        = "gh-ost"
+	testDatabase           = "test"
+	testTable              = "gh_ost_test"
+	testSocketFile         = "/tmp/gh-ost.test.sock"
+	throttleFlagFile       = "/tmp/gh-ost-test.ghost.throttle.flag"
+	throttleQuery          = "select timestampdiff(second, min(last_update), now()) < 5 from _gh_ost_test_ghc"
 )
 
 type Config struct {
-	Username    string
-	Password    string
 	Host        string
 	Port        int64
+	Username    string
+	Password    string
 	GhostBinary string
 	MysqlBinary string
 }
@@ -36,10 +41,29 @@ type Test struct {
 	Name           string
 	Path           string
 	CreateSQLFile  string
-	ExtraArgs      string
+	ExtraArgs      []string
 	IgnoreVersions []string
 }
 
+func WaitForMySQLAvailable(db *sql.DB) error {
+	ticker := time.NewTicker(time.Second)
+	defer ticker.Stop()
+
+	for {
+		select {
+		case <-time.After(10 * time.Minute):
+			return errors.New("timed out waiting for mysql")
+		case <-ticker.C:
+			if err := db.Ping(); err != nil {
+				log.Println("Waiting for mysql to become available")
+			} else {
+				log.Println("MySQL is available")
+				return nil
+			}
+		}
+	}
+}
+
 // Prepare runs a 'mysql' client/shell command to populate the test schema.
 // The create.sql file is read by golang and passed to 'mysql' over stdin.
 func (test *Test) Prepare(config Config) error {
@@ -55,7 +79,7 @@ func (test *Test) Prepare(config Config) error {
 
 	flags := []string{
 		fmt.Sprintf("--defaults-file=%s", defaultsFile),
-		fmt.Sprintf("--host=%s", config.Host),
+		fmt.Sprintf("--host=%s", PrimaryHost), // TODO: fix this
 		fmt.Sprintf("--port=%d", config.Port),
 		"--default-character-set=utf8mb4",
 		testDatabase,
@@ -84,11 +108,11 @@ func (test *Test) Migrate(db *sql.DB, config Config) error {
 	log.Printf("[%s] detected MySQL %s host %s:%d", test.Name, mysqlInfo.Version, config.Host, config.Port)
 
 	flags := []string{
-		fmt.Sprintf("--user=%s", testUsername),
-		fmt.Sprintf("--password=%s", testPassword),
+		fmt.Sprintf("--user=%s", config.Username),
+		fmt.Sprintf("--password=%s", config.Password),
 		fmt.Sprintf("--host=%s", config.Host),
 		fmt.Sprintf("--port=%d", config.Port),
-		fmt.Sprintf("--assume-master-host=%s:%d", mysqlInfo.Host, mysqlInfo.Port),
+		fmt.Sprintf("--assume-master-host=primary:%d", mysqlInfo.Port), // TODO: fix this
 		fmt.Sprintf("--database=%s", testDatabase),
 		fmt.Sprintf("--table=%s", testTable),
 		"--assume-rbr",
@@ -108,14 +132,10 @@ func (test *Test) Migrate(db *sql.DB, config Config) error {
 		"--stack",
 		"--verbose",
 	}
-	if test.ExtraArgs == "" {
-		flags = append(flags, "--alter='engine=innodb'")
+	if len(test.ExtraArgs) > 0 {
+		flags = append(flags, test.ExtraArgs...)
 	} else {
-		extraArgs, err := shlex.Split(test.ExtraArgs)
-		if err != nil {
-			return err
-		}
-		flags = append(flags, extraArgs...)
+		flags = append(flags, `--alter='ENGINE=InnoDB'`)
 	}
 
 	log.Printf("[%s] running gh-ost command: %s\n    %s", test.Name, config.GhostBinary, strings.Join(flags, "\n    "))
@@ -150,7 +170,12 @@ func ReadTests(testsDir string) (tests []Test, err error) {
 
 		extraArgsFile := filepath.Join(test.Path, "extra_args")
 		if _, err = os.Stat(extraArgsFile); err == nil {
-			if test.ExtraArgs, err = readTestFile(extraArgsFile); err != nil {
+			extraArgsStr, err := readTestFile(extraArgsFile)
+			if err != nil {
+				log.Printf("Failed to read extra_args file %q: %+v", extraArgsFile, err)
+				return tests, err
+			}
+			if test.ExtraArgs, err = shlex.Split(extraArgsStr); err != nil {
 				log.Printf("Failed to read extra_args file %q: %+v", extraArgsFile, err)
 				return tests, err
 			}

go/localtests/utils.go

@@ -17,12 +17,11 @@ type MysqlInfo struct {
 	Host    string
 	Port    int64
 	Version string
-	SQLMode string
 }
 
 func getMysqlHostInfo(db *sql.DB) (info MysqlInfo, err error) {
-	res := db.QueryRow("select @@hostname, @@port, @@version, @@sql_mode")
-	err = res.Scan(&info.Host, &info.Port, &info.Version, &info.SQLMode)
+	res := db.QueryRow("select @@hostname, @@port, @@version")
+	err = res.Scan(&info.Host, &info.Port, &info.Version)
 	return info, err
 }
 

go/sql/parser.go

@@ -135,6 +135,12 @@ func (this *AlterTableParser) parseAlterToken(alterToken string) {
 
 func (this *AlterTableParser) ParseAlterStatement(alterStatement string) (err error) {
 	this.alterStatementOptions = alterStatement
+	for _, trimQuote := range []string{`'`, `"`} {
+		if strings.HasPrefix(this.alterStatementOptions, trimQuote) && strings.HasSuffix(this.alterStatementOptions, trimQuote) {
+			this.alterStatementOptions = strings.TrimPrefix(this.alterStatementOptions, trimQuote)
+			this.alterStatementOptions = strings.TrimSuffix(this.alterStatementOptions, trimQuote)
+		}
+	}
 	for _, alterTableRegexp := range alterTableExplicitSchemaTableRegexps {
 		if submatch := alterTableRegexp.FindStringSubmatch(this.alterStatementOptions); len(submatch) > 0 {
 			this.explicitSchema = submatch[1]

go/sql/parser_test.go

@@ -6,6 +6,7 @@
 package sql
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -18,13 +19,40 @@ func init() {
 }
 
 func TestParseAlterStatement(t *testing.T) {
-	statement := "add column t int, engine=innodb"
-	parser := NewAlterTableParser()
-	err := parser.ParseAlterStatement(statement)
-	test.S(t).ExpectNil(err)
-	test.S(t).ExpectEquals(parser.alterStatementOptions, statement)
-	test.S(t).ExpectFalse(parser.HasNonTrivialRenames())
-	test.S(t).ExpectFalse(parser.IsAutoIncrementDefined())
+	// plain alter
+	{
+		statement := "add column t int, engine=innodb"
+		parser := NewAlterTableParser()
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(parser.alterStatementOptions, statement)
+		test.S(t).ExpectFalse(parser.HasNonTrivialRenames())
+		test.S(t).ExpectFalse(parser.IsAutoIncrementDefined())
+	}
+	// single-quoted alter
+	{
+		statement := "add column t int, engine=innodb"
+		parser := NewAlterTableParser()
+		err := parser.ParseAlterStatement(fmt.Sprintf(`'%s'`, statement))
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(parser.alterStatementOptions, statement)
+	}
+	// single-quoted w/comment alter
+	{
+		statement := "add column t int 'single-quoted comment'"
+		parser := NewAlterTableParser()
+		err := parser.ParseAlterStatement(fmt.Sprintf(`'%s'`, statement))
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(parser.alterStatementOptions, statement)
+	}
+	// double-quoted alter
+	{
+		statement := "add column t int, engine=innodb"
+		parser := NewAlterTableParser()
+		err := parser.ParseAlterStatement(fmt.Sprintf(`"%s"`, statement))
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(parser.alterStatementOptions, statement)
+	}
 }
 
 func TestParseAlterStatementTrivialRename(t *testing.T) {

localtests/docker-compose.yml

@@ -4,21 +4,23 @@ services:
     build:
       context: ../
       dockerfile: localtests/Dockerfile
-    command: "--host primary"
     depends_on:
       - primary
       - replica
   primary:
-    image: mysql:${MYSQL_VERSION}
-    command: "--log-bin --server-id=1 --log-slave-updates"
+    image: mysql:${MYSQL_TAG}
+    command: "--enforce-gtid-consistency --gtid-mode=ON --log-bin --log-slave-updates --server-id=1"
     environment:
       MYSQL_ALLOW_EMPTY_PASSWORD: true
     volumes:
-      - "./init.sql:/docker-entrypoint-initdb.d/init.sql:ro"
+      - "./init.sql:/docker-entrypoint-initdb.d/01-init.sql:ro"
   replica:
-    image: mysql:${MYSQL_VERSION}
-    command: "--log-bin --server-id=2 --log-slave-updates --read-only=ON"
+    image: mysql:${MYSQL_TAG}
+    command: "--enforce-gtid-consistency --gtid-mode=ON --log-bin --log-slave-updates --read-only=ON --server-id=2"
     environment:
       MYSQL_ALLOW_EMPTY_PASSWORD: true
+    ports:
+      - "3306:3306"
     volumes:
-      - "./init.sql:/docker-entrypoint-initdb.d/init.sql:ro"
+      - "./init.sql:/docker-entrypoint-initdb.d/01-init.sql:ro"
+      - "./init-replica.sql:/docker-entrypoint-initdb.d/02-init-replica.sql:ro"

localtests/init-replica.sql

@@ -0,0 +1,9 @@
+STOP SLAVE;
+RESET SLAVE;
+RESET MASTER;
+
+CHANGE MASTER TO MASTER_HOST='primary', MASTER_USER='gh-ost', MASTER_PASSWORD='gh-ost', MASTER_PORT=3306,
+  MASTER_AUTO_POSITION=1, MASTER_CONNECT_RETRY=1;
+
+START SLAVE;
+SET @@GLOBAL.read_only=ON;

localtests/keyword-column/extra_args

@@ -1 +1 @@
---alter='add column `index` int unsigned' \
+--alter='add column `index` int unsigned'

localtests/trivial/extra_args

@@ -1 +1 @@
---throttle-query='select false' \
+--throttle-query='select false'