utils: don't panic when no row returned for SHOW MASTER STATUS (#1733)

_utils/terror_gen/errors_release.txt

@@ -128,6 +128,7 @@ ErrNoRelayPosMatchGTID,[code=11121:class=functional:scope=internal:level=high],
 ErrReaderReachEndOfFile,[code=11122:class=functional:scope=internal:level=low]
 ErrMetadataNoBinlogLoc,[code=11123:class=functional:scope=upstream:level=low], "Message: didn't found binlog location in dumped metadata file %s, Workaround: Please check log of dump unit, there maybe errors when read upstream binlog status"
 ErrPreviousGTIDNotExist,[code=11124:class=functional:scope=internal:level=high], "Message: no previous gtid event from binlog %s"
+ErrNoMasterStatus,[code=11125:class=functional:scope=upstream:level=medium], "Message: upstream returns an empty result for SHOW MASTER STATUS, Workaround: Please check the upstream settings like privileges, RDS settings to read data from SHOW MASTER STATUS."
 ErrConfigCheckItemNotSupport,[code=20001:class=config:scope=internal:level=medium], "Message: checking item %s is not supported\n%s, Workaround: Please check `ignore-checking-items` config in task configuration file, which can be set including `all`/`dump_privilege`/`replication_privilege`/`version`/`binlog_enable`/`binlog_format`/`binlog_row_image`/`table_schema`/`schema_of_shard_tables`/`auto_increment_ID`."
 ErrConfigTomlTransform,[code=20002:class=config:scope=internal:level=medium], "Message: %s, Workaround: Please check the configuration file has correct TOML format."
 ErrConfigYamlTransform,[code=20003:class=config:scope=internal:level=medium], "Message: %s, Workaround: Please check the configuration file has correct YAML format."

errors.toml

@@ -778,6 +778,12 @@ description = ""
 workaround = ""
 tags = ["internal", "high"]
 
+[error.DM-functional-11125]
+message = "upstream returns an empty result for SHOW MASTER STATUS"
+description = ""
+workaround = "Please check the upstream settings like privileges, RDS settings to read data from SHOW MASTER STATUS."
+tags = ["upstream", "medium"]
+
 [error.DM-config-20001]
 message = "checking item %s is not supported\n%s"
 description = ""

pkg/terror/error_list.go

@@ -185,6 +185,9 @@ const (
 
 	// pkg/streamer.
 	codePreviousGTIDNotExist
+
+	// pkg/utils.
+	codeNoMasterStatus
 )
 
 // Config related error code list.
@@ -797,6 +800,9 @@ var (
 
 	ErrPreviousGTIDNotExist = New(codePreviousGTIDNotExist, ClassFunctional, ScopeInternal, LevelHigh, "no previous gtid event from binlog %s", "")
 
+	// pkg/utils.
+	ErrNoMasterStatus = New(codeNoMasterStatus, ClassFunctional, ScopeUpstream, LevelMedium, "upstream returns an empty result for SHOW MASTER STATUS", "Please check the upstream settings like privileges, RDS settings to read data from SHOW MASTER STATUS.")
+
 	// Config related error.
 	ErrConfigCheckItemNotSupport    = New(codeConfigCheckItemNotSupport, ClassConfig, ScopeInternal, LevelMedium, "checking item %s is not supported\n%s", "Please check `ignore-checking-items` config in task configuration file, which can be set including `all`/`dump_privilege`/`replication_privilege`/`version`/`binlog_enable`/`binlog_format`/`binlog_row_image`/`table_schema`/`schema_of_shard_tables`/`auto_increment_ID`.")
 	ErrConfigTomlTransform          = New(codeConfigTomlTransform, ClassConfig, ScopeInternal, LevelMedium, "%s", "Please check the configuration file has correct TOML format.")

pkg/utils/db.go

@@ -169,6 +169,7 @@ func GetSlaveServerID(ctx context.Context, db *sql.DB) (map[uint32]struct{}, err
 }
 
 // GetMasterStatus gets status from master.
+// When the returned error is nil, the gtid.Set must be not nil.
 func GetMasterStatus(ctx context.Context, db *sql.DB, flavor string) (gmysql.Position, gtid.Set, error) {
 	var (
 		binlogPos gmysql.Position
@@ -195,31 +196,54 @@ func GetMasterStatus(ctx context.Context, db *sql.DB, flavor string) (gmysql.Pos
 		| ON.000001 |     4822 |              |                  | 85ab69d1-b21f-11e6-9c5e-64006a8978d2:1-46
 		+-----------+----------+--------------+------------------+--------------------------------------------+
 	*/
+	/*
+		For MariaDB,SHOW MASTER STATUS:
+		+--------------------+----------+--------------+------------------+
+		| File               | Position | Binlog_Do_DB | Binlog_Ignore_DB |
+		+--------------------+----------+--------------+------------------+
+		| mariadb-bin.000016 |      475 |              |                  |
+		+--------------------+----------+--------------+------------------+
+		SELECT @@global.gtid_binlog_pos;
+		+--------------------------+
+		| @@global.gtid_binlog_pos |
+		+--------------------------+
+		| 0-1-2                    |
+		+--------------------------+
+	*/
 	var (
 		gtidStr    string
 		binlogName string
 		pos        uint32
 		nullPtr    interface{}
 	)
-	for rows.Next() {
-		if len(rowColumns) == 5 {
-			err = rows.Scan(&binlogName, &pos, &nullPtr, &nullPtr, &gtidStr)
-		} else {
-			err = rows.Scan(&binlogName, &pos, &nullPtr, &nullPtr)
-		}
-		if err != nil {
-			return binlogPos, gs, terror.DBErrorAdapt(err, terror.ErrDBDriverError)
-		}
+	if !rows.Next() {
+		return binlogPos, gs, terror.ErrNoMasterStatus.Generate()
+	}
 
-		binlogPos = gmysql.Position{
-			Name: binlogName,
-			Pos:  pos,
-		}
+	if len(rowColumns) == 5 {
+		err = rows.Scan(&binlogName, &pos, &nullPtr, &nullPtr, &gtidStr)
+	} else {
+		err = rows.Scan(&binlogName, &pos, &nullPtr, &nullPtr)
+	}
+	if err != nil {
+		return binlogPos, gs, terror.DBErrorAdapt(err, terror.ErrDBDriverError)
+	}
 
-		gs, err = gtid.ParserGTID(flavor, gtidStr)
-		if err != nil {
-			return binlogPos, gs, err
-		}
+	binlogPos = gmysql.Position{
+		Name: binlogName,
+		Pos:  pos,
+	}
+
+	gs, err = gtid.ParserGTID(flavor, gtidStr)
+	if err != nil {
+		return binlogPos, gs, err
+	}
+	if rows.Next() {
+		log.L().Warn("SHOW MASTER STATUS returns more than one row, will only use first row")
+	}
+
+	if rows.Close() != nil {
+		return binlogPos, gs, terror.DBErrorAdapt(rows.Err(), terror.ErrDBDriverError)
 	}
 	if rows.Err() != nil {
 		return binlogPos, gs, terror.DBErrorAdapt(rows.Err(), terror.ErrDBDriverError)

pkg/utils/db_test.go

@@ -125,6 +125,14 @@ func (t *testDBSuite) TestGetMasterStatus(c *C) {
 	})
 	c.Assert(gs.String(), Equals, "1-2-100")
 	c.Assert(mock.ExpectationsWereMet(), IsNil)
+
+	// some upstream (maybe a polarDB secondary node)
+	rows = mock.NewRows([]string{"File", "Position", "Binlog_Do_DB", "Binlog_Ignore_DB"})
+	mock.ExpectQuery(`SHOW MASTER STATUS`).WillReturnRows(rows)
+
+	_, gs, err = GetMasterStatus(ctx, db, "mysql")
+	c.Assert(gs, IsNil)
+	c.Assert(err, NotNil)
 }
 
 func (t *testDBSuite) TestGetMariaDBGtidDomainID(c *C) {