Repository: pingcap/dumpling
Pull request #172 (2020-10-22 07:08:22)

fix metadata for earlier mysql
<!--
Thank you for contributing to Dumpling! Please read the [CONTRIBUTING](https://github.com/pingcap/dumpling/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR.
-->

### What problem does this PR solve? <!--add issue link with summary if exists-->
When dumpling dumps data from earlier MySQL who doesn't have GTID (for example, MySQL 5.5.60), dumpling will fail to generate metadata.
```sql
mysql> select version();
+------------+
| version()  |
+------------+
| 5.5.60-log |
+------------+
1 row in set (0.00 sec)

mysql> show master status;
+------------------+----------+--------------+------------------+
| File             | Position | Binlog_Do_DB | Binlog_Ignore_DB |
+------------------+----------+--------------+------------------+
| mysql-bin.000001 |     4879 |              |                  |
+------------------+----------+--------------+------------------+
1 row in set (0.00 sec)
```
```log
[2020/10/22 14:33:07.112 +08:00] [INFO] [config.go:180] ["detect server type"] [type=MySQL]
[2020/10/22 14:33:07.112 +08:00] [INFO] [config.go:198] ["detect server version"] [version=5.5.60-log]
[2020/10/22 14:33:07.120 +08:00] [INFO] [dump.go:164] ["get global metadata failed"] [error="SHOW MASTER STATUS: SHOW MASTER STATUS: sql: expected 4 destination arguments in Scan, not 5"] [errorVerbose="err = SHOW MASTER STATUS: sql: expected 4 destination arguments in Scan, not 5\ngoroutine 1 [running]:\nruntime/debug.Stack(0x209a6c0, 0xc00000e5a0, 0x209c300)\n\t/usr/local/go/src/runtime/debug/stack.go:24 +0x9d\ngithub.com/pingcap/dumpling/v4/export.withStack(0x209a6c0, 0xc00000e5a0, 0x0, 0x0)\n\t/Users/chauncy/code/goPath/src/github.com/pingcap/dumpling/v4/export/error.go:59 +0x8d\ngithub.com/pingcap/dumpling/v4/export.simpleQueryWithArgs(0xc000429500, 0xc000282ff8, 0x1e1f09b, 0x12, 0x0, 0x0, 0x0, 0x0, 0x0)\n\t/Users/chauncy/code/goPath/src/github.com/pingcap/dumpling/v4/export/sql.go:580 +0x2ae\ngithub.com/pingcap/dumpling/v4/export.simpleQuery(...)\n\t/Users/chauncy/code/goPath/src/github.com/pingcap/dumpling/v4/export/sql.go:567\ngithub.com/pingcap/dumpling/v4/export.ShowMasterStatus(0xc000429500, 0x5, 0x1, 0x1, 0x0, 0x0, 0xc00014f580)\n\t/Users/chauncy/code/goPath/src/github.com/pingcap/dumpling/v4/export/sql.go:388 +0x14a\ngithub.com/pingcap/dumpling/v4/export.(*globalMetadata).recordGlobalMetaData(0xc000090e00, 0xc000429500, 0x1aa0001, 0x28c5fa0, 0x0)\n\t/Users/chauncy/code/goPath/src/github.com/pingcap/dumpling/v4/export/metadata.go:77 +0xba\ngithub.com/pingcap/dumpling/v4/export.Dump(0x20bd620, 0xc0000bc000, 0xc00045a000, 0x0, 0x0)\n\t/Users/chauncy/code/goPath/src/github.com/pingcap/dumpling/v4/export/dump.go:162 +0x760\nmain.main()\n\t/Users/chauncy/code/goPath/src/github.com/pingcap/dumpling/cmd/dumpling/main.go:228 +0x1755\n\nSHOW MASTER STATUS"]
[2020/10/22 14:33:07.345 +08:00] [INFO] [main.go:234] ["dump data successfully, dumpling will exit now"]
```

### What is changed and how it works?
1. Don't set fixed columns when `show master status`.
2. Unify the metadata format with mydumper's.

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

 - Unit test
 - Integration test
 - Manual test (add detailed scripts or steps below)
  Test dumping MySQL 5.5.60, the metadata is generated correctly.
```log
[2020/10/22 15:01:00.356 +08:00] [INFO] [config.go:180] ["detect server type"] [type=MySQL]
[2020/10/22 15:01:00.356 +08:00] [INFO] [config.go:198] ["detect server version"] [version=5.5.60-log]
[2020/10/22 15:01:00.588 +08:00] [INFO] [main.go:234] ["dump data successfully, dumpling will exit now"]
```
```
Started dump at: 2020-10-22 15:01:00
SHOW MASTER STATUS:
	Log: mysql-bin.000001
	Pos: 4879
	GTID:

Finished dump at: 2020-10-22 15:01:00
```

Related changes

 - Need to cherry-pick to the release branch
 
### Release note

<!-- bugfixes or new feature need a release note, must in the form of a list, such as

- support -T/--tables-list argument

or if no need to be included in the release note, just add the following line

- No release note
-->
- Fix the problem that metadata is not generated correctly for earlier MySQL

lichunzhu: cc @lance6716 
lance6716: LGTM

Patch:
diff --git a/v4/export/consistency.go b/v4/export/consistency.go
index 4452ff8..839d802 100644
--- a/v4/export/consistency.go
+++ b/v4/export/consistency.go
@@ -102,7 +102,6 @@ func (c *ConsistencyLockDumpingTables) TearDown(ctx context.Context) error {
 	return UnlockTables(ctx, c.conn)
 }
 
-const showMasterStatusFieldNum = 5
 const snapshotFieldIndex = 1
 
 func resolveAutoConsistency(conf *Config) {
diff --git a/v4/export/metadata.go b/v4/export/metadata.go
index 9d41e49..b3e1c2e 100644
--- a/v4/export/metadata.go
+++ b/v4/export/metadata.go
@@ -10,6 +10,8 @@ import (
 	"time"
 
 	"github.com/pingcap/br/pkg/storage"
+	"github.com/pingcap/dumpling/v4/log"
+	"go.uber.org/zap"
 )
 
 type globalMetadata struct {
@@ -57,6 +59,7 @@ func (m *globalMetadata) recordGlobalMetaData(db *sql.Conn, serverType ServerTyp
 	m.buffer.WriteString("\n")
 	switch serverType {
 	// For MySQL:
+	// mysql 5.6+
 	// mysql> SHOW MASTER STATUS;
 	// +-----------+----------+--------------+------------------+-------------------------------------------+
 	// | File      | Position | Binlog_Do_DB | Binlog_Ignore_DB | Executed_Gtid_Set                         |
@@ -64,6 +67,7 @@ func (m *globalMetadata) recordGlobalMetaData(db *sql.Conn, serverType ServerTyp
 	// | ON.000001 |     7502 |              |                  | 6ce40be3-e359-11e9-87e0-36933cb0ca5a:1-29 |
 	// +-----------+----------+--------------+------------------+-------------------------------------------+
 	// 1 row in set (0.00 sec)
+	// mysql 5.5- doesn't have column Executed_Gtid_Set
 	//
 	// For TiDB:
 	// mysql> SHOW MASTER STATUS;
@@ -74,18 +78,16 @@ func (m *globalMetadata) recordGlobalMetaData(db *sql.Conn, serverType ServerTyp
 	// +-------------+--------------------+--------------+------------------+-------------------+
 	// 1 row in set (0.00 sec)
 	case ServerTypeMySQL, ServerTypeTiDB:
-		str, err := ShowMasterStatus(db, showMasterStatusFieldNum)
+		str, err := ShowMasterStatus(db)
 		if err != nil {
 			return err
 		}
-		if logFile := str[fileFieldIndex]; logFile != "" {
-			m.buffer.WriteString("\tLog: " + logFile + "\n")
-		}
-		if pos := str[posFieldIndex]; pos != "" {
-			m.buffer.WriteString("\tPos: " + pos + "\n")
-		}
-		if gtidSet := str[gtidSetFieldIndex]; gtidSet != "" {
-			m.buffer.WriteString("\tGTID:" + gtidSet + "\n")
+		logFile := getValidStr(str, fileFieldIndex)
+		pos := getValidStr(str, posFieldIndex)
+		gtidSet := getValidStr(str, gtidSetFieldIndex)
+
+		if logFile != "" {
+			fmt.Fprintf(&m.buffer, "\tLog: %s\n\tPos: %s\n\tGTID:%s\n", logFile, pos, gtidSet)
 		}
 	// For MariaDB:
 	// SHOW MASTER STATUS;
@@ -102,23 +104,20 @@ func (m *globalMetadata) recordGlobalMetaData(db *sql.Conn, serverType ServerTyp
 	// +--------------------------+
 	// 1 row in set (0.00 sec)
 	case ServerTypeMariaDB:
-		str, err := ShowMasterStatus(db, mariadbShowMasterStatusFieldNum)
+		str, err := ShowMasterStatus(db)
 		if err != nil {
 			return err
 		}
-		if logFile := str[fileFieldIndex]; logFile != "" {
-			m.buffer.WriteString("\tLog: " + logFile + "\n")
-		}
-		if pos := str[posFieldIndex]; pos != "" {
-			m.buffer.WriteString("\tPos: " + pos + "\n")
-		}
+		logFile := getValidStr(str, fileFieldIndex)
+		pos := getValidStr(str, posFieldIndex)
 		var gtidSet string
 		err = db.QueryRowContext(context.Background(), "SELECT @@global.gtid_binlog_pos").Scan(&gtidSet)
 		if err != nil {
-			return err
+			log.Error("fail to get gtid for mariaDB", zap.Error(err))
 		}
-		if gtidSet != "" {
-			m.buffer.WriteString("\tGTID:" + gtidSet + "\n")
+
+		if logFile != "" {
+			fmt.Fprintf(&m.buffer, "\tLog: %s\n\tPos: %s\n\tGTID:%s\n", logFile, pos, gtidSet)
 		}
 	default:
 		return errors.New("unsupported serverType" + serverType.String() + "for recordGlobalMetaData")
@@ -199,3 +198,10 @@ func (m *globalMetadata) writeGlobalMetaData(ctx context.Context) error {
 
 	return write(ctx, fileWriter, m.String())
 }
+
+func getValidStr(str []string, idx int) string {
+	if idx < len(str) {
+		return str[idx]
+	}
+	return ""
+}
diff --git a/v4/export/sql.go b/v4/export/sql.go
index 6590344..b463e71 100644
--- a/v4/export/sql.go
+++ b/v4/export/sql.go
@@ -386,13 +386,19 @@ func UseDatabase(db *sql.DB, databaseName string) error {
 	return withStack(err)
 }
 
-func ShowMasterStatus(db *sql.Conn, fieldNum int) ([]string, error) {
-	oneRow := make([]string, fieldNum)
-	addr := make([]interface{}, fieldNum)
-	for i := range oneRow {
-		addr[i] = &oneRow[i]
-	}
+func ShowMasterStatus(db *sql.Conn) ([]string, error) {
+	var oneRow []string
 	handleOneRow := func(rows *sql.Rows) error {
+		cols, err := rows.Columns()
+		if err != nil {
+			return err
+		}
+		fieldNum := len(cols)
+		oneRow = make([]string, fieldNum)
+		addr := make([]interface{}, fieldNum)
+		for i := range oneRow {
+			addr[i] = &oneRow[i]
+		}
 		return rows.Scan(addr...)
 	}
 	const showMasterStatusQuery = "SHOW MASTER STATUS"
@@ -464,7 +470,7 @@ func CheckTiDBWithTiKV(db *sql.DB) (bool, error) {
 }
 
 func getSnapshot(db *sql.Conn) (string, error) {
-	str, err := ShowMasterStatus(db, showMasterStatusFieldNum)
+	str, err := ShowMasterStatus(db)
 	if err != nil {
 		return "", err
 	}
diff --git a/v4/export/writer_util.go b/v4/export/writer_util.go
index 131f58f..2ea4e5a 100644
--- a/v4/export/writer_util.go
+++ b/v4/export/writer_util.go
@@ -8,9 +8,8 @@ import (
 	"strings"
 	"sync"
 
-	"go.uber.org/zap"
-
 	"github.com/pingcap/br/pkg/storage"
+	"go.uber.org/zap"
 
 	"github.com/pingcap/dumpling/v4/log"
 )


Test patch:
diff --git a/v4/export/metadata_test.go b/v4/export/metadata_test.go
index 13b0eb3..f1bd040 100644
--- a/v4/export/metadata_test.go
+++ b/v4/export/metadata_test.go
@@ -207,3 +207,29 @@ func (s *testMetaDataSuite) TestMariaDBWithFollowersMetaData(c *C) {
 		"\tGTID:6ce40be3-e359-11e9-87e0-36933cb0ca5a:1-29\n\n")
 	c.Assert(mock.ExpectationsWereMet(), IsNil)
 }
+
+func (s *testMetaDataSuite) TestEarlierMysqlMetaData(c *C) {
+	db, mock, err := sqlmock.New()
+	c.Assert(err, IsNil)
+	defer db.Close()
+	conn, err := db.Conn(context.Background())
+	c.Assert(err, IsNil)
+
+	logFile := "mysql-bin.000001"
+	pos := "4879"
+	rows := sqlmock.NewRows([]string{"File", "Position", "Binlog_Do_DB", "Binlog_Ignore_DB"}).
+		AddRow(logFile, pos, "", "")
+	mock.ExpectQuery("SHOW MASTER STATUS").WillReturnRows(rows)
+	mock.ExpectQuery("SELECT @@default_master_connection").WillReturnError(fmt.Errorf("mock error"))
+	mock.ExpectQuery("SHOW SLAVE STATUS").WillReturnRows(
+		sqlmock.NewRows([]string{"exec_master_log_pos", "relay_master_log_file", "master_host", "Executed_Gtid_Set", "Seconds_Behind_Master"}))
+
+	m := newGlobalMetadata(s.createStorage(c))
+	c.Assert(m.recordGlobalMetaData(conn, ServerTypeMySQL, false), IsNil)
+
+	c.Assert(m.buffer.String(), Equals, "SHOW MASTER STATUS:\n"+
+		"\tLog: mysql-bin.000001\n"+
+		"\tPos: 4879\n"+
+		"\tGTID:\n\n")
+	c.Assert(mock.ExpectationsWereMet(), IsNil)
+}