Skip to content

Commit bba7359

Browse files
authored
Fix lexographical comparison of binlog filenames (#1604)
* Parse binlog file numbers numerically instead of lexicographically to correctly order files like binlog.999999 < binlog.1000000. Would cause the stream to ignore all incoming events and render the gh-ost process stuck: https://github.com/github/gh-ost/blob/48b34bcbfde730b2548d598dee98e9c1f0d2fcce/go/binlog/gomysql_reader.go#L85-L88 Possibly remediated by 005043d too, which drops the SmallerThanOrEqual check from GoMySqlReader.handleRowsEvent * Remove unused fn FileBinlogCoordinates.FileSmallerThan
1 parent 1557a95 commit bba7359

File tree

2 files changed

+21
-15
lines changed

2 files changed

+21
-15
lines changed

go/mysql/binlog_file.go

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,12 @@ func (this *FileBinlogCoordinates) SmallerThan(other BinlogCoordinates) bool {
7878
if !ok || other == nil {
7979
return false
8080
}
81-
if this.LogFile < coord.LogFile {
82-
return true
83-
}
84-
if this.LogFile == coord.LogFile && this.LogPos < coord.LogPos {
85-
return true
81+
82+
fileNumberDist := this.FileNumberDistance(coord)
83+
if fileNumberDist == 0 {
84+
return this.LogPos < coord.LogPos
8685
}
87-
return false
86+
return fileNumberDist > 0
8887
}
8988

9089
// SmallerThanOrEquals returns true if this coordinate is the same or equal to the other one.
@@ -100,15 +99,6 @@ func (this *FileBinlogCoordinates) SmallerThanOrEquals(other BinlogCoordinates)
10099
return this.LogFile == coord.LogFile && this.LogPos == coord.LogPos // No Type comparison
101100
}
102101

103-
// FileSmallerThan returns true if this coordinate's file is strictly smaller than the other's.
104-
func (this *FileBinlogCoordinates) FileSmallerThan(other BinlogCoordinates) bool {
105-
coord, ok := other.(*FileBinlogCoordinates)
106-
if !ok || other == nil {
107-
return false
108-
}
109-
return this.LogFile < coord.LogFile
110-
}
111-
112102
// FileNumberDistance returns the numeric distance between this coordinate's file number and the other's.
113103
// Effectively it means "how many rotates/FLUSHes would make these coordinates's file reach the other's"
114104
func (this *FileBinlogCoordinates) FileNumberDistance(other *FileBinlogCoordinates) int {

go/mysql/binlog_file_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,19 @@ func TestIsLogPosOverflowBeyond4Bytes(t *testing.T) {
134134
require.True(t, curCoordinates.IsLogPosOverflowBeyond4Bytes(preCoordinates))
135135
}
136136
}
137+
138+
func TestBinlogCoordinates_LogFileZeroPaddedTransition(t *testing.T) {
139+
c1 := FileBinlogCoordinates{LogFile: "mysql-bin.999999", LogPos: 100}
140+
c2 := FileBinlogCoordinates{LogFile: "mysql-bin.1000000", LogPos: 100}
141+
142+
require.True(t, c1.SmallerThan(&c2))
143+
}
144+
145+
func TestBinlogCoordinates_SameLogFileDifferentPosition(t *testing.T) {
146+
c1 := FileBinlogCoordinates{LogFile: "binlog.000001", LogPos: 100}
147+
c2 := FileBinlogCoordinates{LogFile: "binlog.000001", LogPos: 200}
148+
149+
require.True(t, c1.SmallerThan(&c2))
150+
require.False(t, c2.SmallerThan(&c1))
151+
require.False(t, c1.SmallerThan(&c1))
152+
}

0 commit comments

Comments
 (0)