-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
parseComBinlogDumpGTID
: GTID payload is always 5.6 flavor
#17605
parseComBinlogDumpGTID
: GTID payload is always 5.6 flavor
#17605
Conversation
Signed-off-by: Shlomi Noach <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
go/mysql/replication_test.go
Outdated
gtidSet, err := replication.ParseMysql56GTIDSet("16b1039f-22b6-11ed-b765-0a43f95f28a3:1-243") | ||
require.NoError(t, err) | ||
sidBlock := gtidSet.SIDBlock() | ||
assert.Len(t, sidBlock, 0x30) // 48 bytes |
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.
Nit, but no reason not to use decimal here (can then remove the comment).
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.
done.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17605 +/- ##
==========================================
- Coverage 67.68% 67.66% -0.03%
==========================================
Files 1586 1586
Lines 255170 255407 +237
==========================================
+ Hits 172717 172825 +108
- Misses 82453 82582 +129 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Description
Quick followup/fix to #17580. #17580 wasn't well tested, and although nothing in Vitess is actually calling
ComBinlogDumpGTID
with a GTID payload -- such calls fail. This is because the function attempts to run:However, the encoded GTID does not have a
MySQL56/
prefix. Rightly so, because that's not in the MySQL protocol. The commandComBinlogDumpGTID
is MySQL-specific (ie not MariaDB or any other flavor) and thus the GTID payload is necessarily aMySQL56
flavor.In addition, the GTID is written in
ComBinlogDumpGTID
as a SID block. This PR now also parses it back from a SID block form.This PR fixes the issue and adds appropriate testing.
Related Issue(s)
#17579
Checklist
Deployment Notes