-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor: Extract GTID interval logic into Gtid_Interval class
#5224
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
base: feature/gtid-range-update
Are you sure you want to change the base?
refactor: Extract GTID interval logic into Gtid_Interval class
#5224
Conversation
Summary of ChangesHello @proton-lisandro-pin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces foundational changes to ProxySQL's GTID management system, specifically to enable support for ranged GTID updates. It redefines how GTID intervals are represented and manipulated internally, moving from basic pairs to a more robust Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Question: none of the files in I'll be happy to add tests for |
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.
Code Review
This pull request introduces significant changes to support ranged GTID updates by refactoring gtid_interval_t from a std::pair into a dedicated Gtid_Interval class and reworking the addGtid logic. The changes are well-structured and lay a good foundation for parsing GTID ranges. My review focuses on improving the C++ implementation of the new Gtid_Interval class and the associated functions, with suggestions for enhancing const correctness, readability, and adherence to C++ best practices. These improvements will make the new code more robust and easier to maintain.
dd126a2 to
b1a781c
Compare
c8ac908 to
93dfdf8
Compare
fda6306 to
893fc40
Compare
wazir-ahmed
left a 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.
Looks good overall. Just a few minor comments to address.
893fc40 to
979dfeb
Compare
|
@proton-lisandro-pin please resolve the conflicts. |
Gtid_Interval class
Sorry, i didn't realize we changed the destination branch 🤦 Change is now rebased. |
979dfeb to
23c153f
Compare
|
@proton-lisandro-pin I have added documentation for unit tests here. You can add the unit test for |
e0d3cae to
1b27fe6
Compare
Done, PTAL. |
1b27fe6 to
9b68a7c
Compare
7022d5b to
a638889
Compare
a638889 to
e76d093
Compare
|



Initial work to support ranged GTID updates from proxysql_mysqlbinlog.
This PR reworks
gtid_interval_tso GTID intervals can be compared/merged, and modifies the logic to add new GTIDs togtid_set_tso it understand range updates. Notably, it will automatically de-dupe and compact overlapping ranges,when possible.
A PR to properly parse ranges on
I1=andI2=GTID update messages will follow; until then, this PR changesno functionality within ProxySQL.
See sysown/proxysql_mysqlbinlog#33 for details.