-
Notifications
You must be signed in to change notification settings - Fork 1k
do not sync persistent store when get a fake position #397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@siddontang PTAL |
PTAL @GregoryIan |
canal/sync.go
Outdated
@@ -65,6 +65,10 @@ func (c *Canal) runSyncBinlog() error { | |||
// TODO: If we meet any DDL query, we must save too. | |||
switch e := ev.Event.(type) { | |||
case *replication.RotateEvent: | |||
// if event pos equal to local pos.maybe be fake rotate event,do nothing. | |||
if string(e.NextLogName) == pos.Name && uint32(e.Position) == pos.Pos { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in MySQL, a fake rotate event is an event with timestamp equals to 0 (see https://github.com/mysql/mysql-server/blob/8cc757da3d87bf4a1f07dcfb2d3c96fed3806870/sql/rpl_binlog_sender.cc#L899), and also with log pos equals to 0 in the implementation (see https://github.com/mysql/mysql-server/blob/8cc757da3d87bf4a1f07dcfb2d3c96fed3806870/sql/rpl_binlog_sender.cc#L906).
I don't know whether some corner cases exist if checking by string(e.NextLogName) == pos.Name && uint32(e.Position) == pos.Pos
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Friendly ping @lintanghui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in MySQL, a fake rotate event is an event with timestamp equals to 0 (see https://github.com/mysql/mysql-server/blob/8cc757da3d87bf4a1f07dcfb2d3c96fed3806870/sql/rpl_binlog_sender.cc#L899), and also with log pos equals to 0 in the implementation (see https://github.com/mysql/mysql-server/blob/8cc757da3d87bf4a1f07dcfb2d3c96fed3806870/sql/rpl_binlog_sender.cc#L906).
I don't know whether some corner cases exist if checking by
string(e.NextLogName) == pos.Name && uint32(e.Position) == pos.Pos
.
yes, timestamp 0 and pos 0 is set in event header, but in event body, pos and file name is also be set, https://github.com/mysql/mysql-server/blob/8cc757da3d87bf4a1f07dcfb2d3c96fed3806870/sql/rpl_binlog_sender.cc#L909 . in sync.go we just get event body and compare it. we can also do such below
ev.Header.Pos==0 {continue }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lintanghui I know, in this special case string(e.NextLogName) == pos.Name && uint32(e.Position) == pos.Pos
is right, but I don't know whether some corner cases exist.
BTW, will you update it to ev.Header.Pos==0 {continue }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine, it seem better to compare header pos.
PTAL @GregoryIan . it's that ok to be merge? |
fixed #396