Removed StateTracker complexity #123
Conversation
shuhaowu
changed the title
shuhaowu
force-pushed
the
state-tracker-complexity-removal
branch
from
68fbf33
to
4f17d4d
Compare
shuhaowu
changed the title
state_tracker.go
Outdated
| } | ||
| } | ||
| lastWrittenBinlogPosition mysql.Position | ||
| lastStoredBinlogPositionForInlineVerifier mysql.Position |
There was a problem hiding this comment.
nit: lastVerifiedBinlogPosition is simpler? Do we need to introduce this in this PR? Seems to me like a followup to #122
There was a problem hiding this comment.
It's not quite the last verified binlog position. As soon as the binlog event is added to the reverify store, we record this position. The reverify store will be dumped anyway so we won't lose anything.
Either way, I'll remove it for now and send a follow up PR after #122.
state_tracker.go
Outdated
| return s.CopyStage.LastProcessedBinlogPosition | ||
| nilPosition := mysql.Position{} | ||
| if s.LastWrittenBinlogPosition == nilPosition { | ||
| return s.LastStoredBinlogPositionForInlineVerifier |
There was a problem hiding this comment.
If there was no LastWrittenBinlogPosition (nothing was written by the binlog writer), how could we have a LastStoredBinlogPositionForInlineVerifier?
There was a problem hiding this comment.
Both the LastWrittenBinlogPosition and the LastStoredBinlogPositionForInlineVerifier are initialized during Ferry.Start, right after the binlog streamer is connected. This is done to address a bug where if the application database got not writes to it while Ghostferry is running, Ghostferry could not resume.
| // The binlog writer and the verify binlog positions are different because the | ||
| // binlog writer is buffered in a background go routine. Its position can race | ||
| // with respect to the verifier binlog position. The minimum position between | ||
| // the two are always safe to resume from. | ||
| func (s *SerializableState) MinBinlogPosition() mysql.Position { |
There was a problem hiding this comment.
Why is this method defined on this struct? Seems like a consumer concern
There was a problem hiding this comment.
I was more thinking of this as an "attribute" of the SerializableState and hence put it here.
This commit can be reverted in a later PR.
The
StateTrackercode was complex as it was split between theCopyStateTrackerand theVerifierStateTracker. Since we are moving towards theInlineVerifier, which requires very little state, we don't need this complexity anymore.This PR removes the extra complexity and only retains a single
StateTrackerandSerializableState.