-
Notifications
You must be signed in to change notification settings - Fork 987
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
replication: Support GTID tag in PreviousGTIDsEvent #952
base: master
Are you sure you want to change the base?
Conversation
Issue: ref go-mysql-org#845 The `PreviousGTIDsEvent` / `PREVIOUS_GTIDS_LOG_EVENT` has changed to work with tagged GTIDs. First the `uuidCount` has changed, it encodes the GTID format. Here format 1 is tagged and format 0 is untagged. Then each entry may have a tag. If there is a tag then the uuid itself isn't printed but the tag is appended to the last entry. Examples: `896e7882-18fe-11ef-ab88-22222d34d411:1-3` regular format, compatible with both formats `896e7882-18fe-11ef-ab88-22222d34d411:1-4:aaaa:1` tagged format. Combination of - `896e7882-18fe-11ef-ab88-22222d34d411:1-4` - `896e7882-18fe-11ef-ab88-22222d34d411:aaaa:1` `896e7882-18fe-11ef-ab88-22222d34d411:1-4:aaaa:1:abc:1-3:bbbbb:1:bbbbbb:1:x:1,896e7882-18fe-11ef-ab88-22222d34d412:1-2` Combination of: ``` 896e7882-18fe-11ef-ab88-22222d34d411:1-4 :aaaa:1 🔤1-3 :bbbbb:1 :bbbbbb:1 ❌1, 896e7882-18fe-11ef-ab88-22222d34d412:1-2 ``` Please also see: `mysqlbinlog --read-from-remote-server --hexdump $binlogfile` to see how MySQL encodes/decodes this. See also: - https://dev.mysql.com/doc/refman/8.4/en/replication-gtids-concepts.html
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.
I'm not sure Example 3 in PR description has the same format as MySQL. In https://dev.mysql.com/doc/refman/8.4/en/replication-gtids-concepts.html , there's
GTIDs originating from the same server but having different tags are treated in a manner similar to those originating from different servers, like this:
3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_1:1-3:15-21, 3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_2:8-52
rather than 3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_1:1-3:15-21:Domain_2:8-52
. I guess the reason is if Domain_2
is replaced by tag 23
, we will see 3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_1:1-3:15-21:23:8-52
, which is ambiguous.
GtidFormatTagged | ||
) | ||
|
||
func decodeSid(data []byte) (format GtidFormat, sidnr uint64) { |
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.
can you put the referenced MySQL source code or document in comment, so other reviewers can double check the implementation?
It looks like my example might be correct and the MySQL docs might not be in sync with the actual behavior. Note that From https://dev.mysql.com/doc/dev/mysql-server/latest/classmysql_1_1gtid_1_1Tag.html
|
And with the exact GTIDs from the docs:
|
I've filed https://bugs.mysql.com/bug.php?id=116789 for this |
I also filed https://bugs.mysql.com/bug.php?id=116747 some days ago |
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.
Just a pure code-related review. I'll check GTID encoding and decoding soon
uuidCount := binary.LittleEndian.Uint16(data[pos : pos+8]) | ||
|
||
gtidinfo := make([]byte, 8) | ||
copy(gtidinfo, data[:8]) |
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.
I see that the new copy is caused by decodeSid
will modify data in-place. How about let decodeSid
copy the needed data? So there's no risk that in future we need to change the code and caller forget to copy it.
if format == GtidFormatTagged { | ||
tagLength := int(data[pos]) / 2 | ||
pos += 1 | ||
if tagLength > 0 { |
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.
can the tag have zero length? please add comments
@@ -254,9 +298,14 @@ func (e *PreviousGTIDsEvent) Decode(data []byte) error { | |||
} | |||
intervals[i] = interval | |||
} | |||
previousGTIDSets[i] = fmt.Sprintf("%s:%s", uuid, strings.Join(intervals, ":")) | |||
if isTag { | |||
previousGTIDSets[currentSetnr-1] += fmt.Sprintf(":%s:%s", tag, strings.Join(intervals, ":")) |
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.
Because string is immutable, string +=
will create a new string object each time. If there are too many tags the performance will drop. Maybe store every "tag+intervals" and join them once at all
Co-authored-by: lance6716 <[email protected]>
Issue: ref #845
The
PreviousGTIDsEvent
/PREVIOUS_GTIDS_LOG_EVENT
has changed to work with tagged GTIDs.First the
uuidCount
has changed, it encodes the GTID format. Here format 1 is tagged and format 0 is untagged.Then each entry may have a tag. If there is a tag then the uuid itself isn't printed but the tag is appended to the last entry.
Example 1
896e7882-18fe-11ef-ab88-22222d34d411:1-3
regular format, compatible with both formats
Example 2
896e7882-18fe-11ef-ab88-22222d34d411:1-4:aaaa:1
tagged format.
Combination of
896e7882-18fe-11ef-ab88-22222d34d411:1-4
896e7882-18fe-11ef-ab88-22222d34d411:aaaa:1
Example 3
896e7882-18fe-11ef-ab88-22222d34d411:1-4:aaaa:1:abc:1-3:bbbbb:1:bbbbbb:1:x:1,896e7882-18fe-11ef-ab88-22222d34d412:1-2
Combination of:
Please also see:
mysqlbinlog --read-from-remote-server --hexdump $binlogfile
to see how MySQL encodes/decodes this.See also: