Skip to content

Commit

Permalink
dm: reduce gSet.String() usage by using zeroGSet for checking empty (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ti-chi-bot authored Dec 11, 2023
1 parent 37ac92a commit 2cfc63e
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 9 deletions.
3 changes: 2 additions & 1 deletion dm/pkg/binlog/event/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

gmysql "github.com/go-mysql-org/go-mysql/mysql"
"github.com/go-mysql-org/go-mysql/replication"
"github.com/pingcap/tiflow/dm/pkg/gtid"
"github.com/pingcap/tiflow/dm/pkg/terror"
)

Expand Down Expand Up @@ -172,7 +173,7 @@ func GTIDIncrease(flavor string, gSet gmysql.GTIDSet) (gmysql.GTIDSet, error) {

// verifySingleGTID verifies gSet whether only containing a single valid GTID.
func verifySingleGTID(flavor string, gSet gmysql.GTIDSet) (interface{}, error) {
if gSet == nil || len(gSet.String()) == 0 {
if gtid.IsZeroGTIDSet(gSet) {
return nil, terror.ErrBinlogEmptyGTID.Generate()
}

Expand Down
3 changes: 2 additions & 1 deletion dm/pkg/binlog/event/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

gmysql "github.com/go-mysql-org/go-mysql/mysql"
"github.com/go-mysql-org/go-mysql/replication"
"github.com/pingcap/tiflow/dm/pkg/gtid"
"github.com/pingcap/tiflow/dm/pkg/terror"
)

Expand Down Expand Up @@ -717,7 +718,7 @@ func GenXIDEvent(header *replication.EventHeader, latestPos uint32, xid uint64)
// GenMariaDBGTIDListEvent generates a MariadbGTIDListEvent.
// ref: https://mariadb.com/kb/en/library/gtid_list_event/
func GenMariaDBGTIDListEvent(header *replication.EventHeader, latestPos uint32, gSet gmysql.GTIDSet) (*replication.BinlogEvent, error) {
if gSet == nil || len(gSet.String()) == 0 {
if gtid.IsZeroGTIDSet(gSet) {
return nil, terror.ErrBinlogEmptyGTID.Generate()
}

Expand Down
4 changes: 2 additions & 2 deletions dm/pkg/binlog/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,8 @@ func IsFreshPosition(location Location, flavor string, cmpGTID bool) bool {
//
// but if can't compare gSet1 and gSet2, will returns 0, false.
func CompareGTID(gSet1, gSet2 gmysql.GTIDSet) (int, bool) {
gSetIsEmpty1 := gSet1 == nil || len(gSet1.String()) == 0
gSetIsEmpty2 := gSet2 == nil || len(gSet2.String()) == 0
gSetIsEmpty1 := gtid.IsZeroGTIDSet(gSet1)
gSetIsEmpty2 := gtid.IsZeroGTIDSet(gSet2)

switch {
case gSetIsEmpty1 && gSetIsEmpty2:
Expand Down
39 changes: 35 additions & 4 deletions dm/pkg/gtid/gtid.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import (
"github.com/pingcap/tiflow/dm/pkg/terror"
)

var (
zeroMySQLGTIDSet = MustZeroGTIDSet(mysql.MySQLFlavor)
zeroMariaDBGTIDSet = MustZeroGTIDSet(mysql.MariaDBFlavor)
)

// ParserGTID parses GTID from string. If the flavor is not specified, it will
// try mysql GTID first and then MariaDB GTID.
func ParserGTID(flavor, gtidStr string) (mysql.GTIDSet, error) {
Expand All @@ -42,7 +47,11 @@ func ParserGTID(flavor, gtidStr string) (mysql.GTIDSet, error) {
gtid, err = mysql.ParseGTIDSet(fla, gtidStr)
}
case mysql.MariaDBFlavor:
gtid, err = mysql.ParseGTIDSet(fla, gtidStr)
if IsZeroMariaDBGTIDSet(gtidStr) {
gtid, err = mysql.ParseGTIDSet(fla, "")
} else {
gtid, err = mysql.ParseGTIDSet(fla, gtidStr)
}
case "":
fla = mysql.MySQLFlavor
gtid, err = mysql.ParseGTIDSet(fla, gtidStr)
Expand Down Expand Up @@ -74,9 +83,6 @@ func MustZeroGTIDSet(flavor string) mysql.GTIDSet {
// IsZeroMySQLGTIDSet is used to meet this usage: when user wants to start binlog
// replication from scratch, a "uuid:0" (MySQL flavor) or "0-0-0" (mariaDB) GTID
// set must be written, in order to distinguish that user forgets to write it.
//
// For above two flavor, only "uuid:0" is illegal, so we use IsZeroMySQLGTIDSet
// to handle it.
func IsZeroMySQLGTIDSet(gStr string) bool {
sp := strings.Split(gStr, ",")
if len(sp) != 1 {
Expand All @@ -90,3 +96,28 @@ func IsZeroMySQLGTIDSet(gStr string) bool {
interval := strings.TrimSpace(sep[1])
return interval == "0"
}

// IsZeroMariaDBGTIDSet is used to meet this usage: when user wants to start binlog
// replication from scratch, a "uuid:0" (MySQL flavor) or "0-0-0" (mariaDB) GTID
// set must be written, in order to distinguish that user forgets to write it.
//
// For MariaDB, the GTID set like "0-0-0" will confuse IsZeroGTIDSet function,
// so we also need to check the interval part.
func IsZeroMariaDBGTIDSet(gStr string) bool {
sp := strings.Split(gStr, ",")
if len(sp) != 1 {
return false
}

sep := strings.Split(sp[0], "-")
if len(sep) != 3 {
return false
}
interval := strings.TrimSpace(sep[2])
return interval == "0"
}

// IsZeroGTIDSet is used to check whether a GTID set is zero.
func IsZeroGTIDSet(gSet mysql.GTIDSet) bool {
return gSet == nil || zeroMySQLGTIDSet.Equal(gSet) || zeroMariaDBGTIDSet.Equal(gSet)
}
60 changes: 59 additions & 1 deletion dm/pkg/gtid/gtid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestParseGTIDNoFlavor(t *testing.T) {
require.Error(t, err)
}

func TestIsNilGTIDSet(t *testing.T) {
func TestIsNilMySQLGTIDSet(t *testing.T) {
t.Parallel()

require.False(t, IsZeroMySQLGTIDSet(""))
Expand All @@ -79,6 +79,17 @@ func TestIsNilGTIDSet(t *testing.T) {
require.True(t, IsZeroMySQLGTIDSet(" xxxxx:0 "))
}

func TestIsNilMariaDBGTIDSet(t *testing.T) {
t.Parallel()

require.False(t, IsZeroMariaDBGTIDSet(""))
require.False(t, IsZeroMariaDBGTIDSet("xxxxx"))
require.False(t, IsZeroMariaDBGTIDSet("a-b-0,c-d:0"))
require.False(t, IsZeroMariaDBGTIDSet("xxxxx:1"))
require.True(t, IsZeroMariaDBGTIDSet("x-y-0"))
require.True(t, IsZeroMariaDBGTIDSet(" x-y-0 "))
}

func TestParseZeroAsEmptyGTIDSet(t *testing.T) {
t.Parallel()

Expand All @@ -94,3 +105,50 @@ func TestParseZeroAsEmptyGTIDSet(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "", gset.String())
}

func TestIsZeroGTIDSet(t *testing.T) {
t.Parallel()

testCases := []struct {
gsetStr string
isZero bool
flavor string
}{
{
"",
true,
mysql.MySQLFlavor,
},
{
"",
true,
mysql.MariaDBFlavor,
},
{
"3ccc475b-2343-11e7-be21-6c0b84d59f30:0",
true,
mysql.MySQLFlavor,
},
{
"0-0-0",
true,
mysql.MariaDBFlavor,
},
{
"3ccc475b-2343-11e7-be21-6c0b84d59f30:1-14",
false,
mysql.MySQLFlavor,
},
{
"1-2-3",
false,
mysql.MariaDBFlavor,
},
}
for i, testCase := range testCases {
t.Logf("test case %d", i)
gset, err := ParserGTID(testCase.flavor, testCase.gsetStr)
require.NoError(t, err)
require.Equal(t, testCase.isZero, IsZeroGTIDSet(gset))
}
}

0 comments on commit 2cfc63e

Please sign in to comment.