Skip to content

Implement a ignore_min_gtid_annotation global variable, fix memory leak/overflow in min_gtid annotation handler.#5150

Merged
renecannao merged 2 commits intosysown:v3.0from
proton-lisandro-pin:ignore_min_gtid
Nov 4, 2025
Merged

Implement a ignore_min_gtid_annotation global variable, fix memory leak/overflow in min_gtid annotation handler.#5150
renecannao merged 2 commits intosysown:v3.0from
proton-lisandro-pin:ignore_min_gtid

Conversation

@proton-lisandro-pin
Copy link
Contributor

@proton-lisandro-pin proton-lisandro-pin commented Oct 8, 2025 •
edited
Loading

When true, all min_gtid query annotations are ignored; see https://proxysql.com/documentation/query-annotations/ for details.

This is useful on ProxySQL setups with multiple layers, where some layers mandate GTID-based routing
while others don't.

Also fixes a buffer overflow bug and memory leak in the min_gtid annotation handler.

@gemini-code-assist
Copy link

gemini-code-assist bot commented Oct 8, 2025

Summary of Changes

Hello @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 PR adds a new global variable ignore_min_gtid_annotations to ProxySQL. When set to true, ProxySQL will disregard min_gtid query annotations, which is beneficial for multi-layered ProxySQL setups where GTID routing requirements vary across layers. The implementation involves defining the variable, modifying the query processor to check its value, and integrating it into ProxySQL's configuration and refresh mechanisms.

Highlights

  • Intent: This pull request introduces a new global variable, ignore_min_gtid_annotations, to ProxySQL. The primary goal is to provide a mechanism to selectively ignore min_gtid query annotations, which are used for GTID-based routing. This feature is particularly useful in complex ProxySQL deployments with multiple layers, where GTID-based routing might be required in some layers but not others.
  • Changes: The changes involve defining a new boolean variable ignore_min_gtid_annotations within the ProxySQL thread variables. The MySQL_Query_Processor has been updated to check this variable: if true, any min_gtid query annotations will be ignored, and a debug message will be logged. Otherwise, the min_gtid annotation will be processed as usual. The variable is made configurable and refreshable through the standard ProxySQL variable management mechanisms.
  • Files Changed: The following files were modified:
  • include/MySQL_Query_Processor.h: Modified query_parser_first_comment_extended to conditionally ignore min_gtid annotations based on the new variable.
  • include/MySQL_Thread.h: Added ignore_min_gtid_annotations to the MySQL_Threads_Handler::variables struct.
  • include/proxysql_structs.h: Declared the thread-local mysql_thread___ignore_min_gtid_annotations variable.
  • lib/MySQL_Session.cpp: Minor indentation change (unrelated to the main feature).
  • lib/MySQL_Thread.cpp: Integrated the new variable into the variable management system, including adding it to the list of configurable variables, initializing it, and ensuring it can be refreshed.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]
gemini-code-assist bot reviewed Oct 8, 2025
View reviewed changes
Copy link

@gemini-code-assist gemini-code-assist bot left a 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 a new global variable ignore_min_gtid_annotation to control GTID-based routing, which is a useful feature for multi-layered ProxySQL setups. The implementation is mostly correct, adding the new variable across different parts of the codebase consistently.

However, I've found a critical buffer overflow vulnerability and a high-severity issue related to type safety in the modified code. Please address these issues to ensure the stability and security of the application.

@renecannao
Copy link
Contributor

renecannao commented Oct 8, 2025

Can one of the admins verify this patch?

@proton-lisandro-pin proton-lisandro-pin changed the title Implement a ignore_min_gtid_annotation global variable for ProxySQL. Implement a ignore_min_gtid_annotation global variable, fix memory leak/overflow in min_gtid_annotation handler. Oct 8, 2025
@proton-lisandro-pin proton-lisandro-pin changed the title Implement a ignore_min_gtid_annotation global variable, fix memory leak/overflow in min_gtid_annotation handler. Implement a ignore_min_gtid_annotation global variable, fix memory leak/overflow in min_gtid annotation handler. Oct 8, 2025
@noizu
Copy link
Contributor

noizu commented Oct 14, 2025

@wazir-ahmed please review.

@proton-lisandro-pin
Copy link
Contributor Author

proton-lisandro-pin commented Oct 20, 2025

Friendly ping?

wazir-ahmed
wazir-ahmed reviewed Oct 29, 2025
View reviewed changes
Copy link
Member

@wazir-ahmed wazir-ahmed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@proton-lisandro-pin Thanks for the contribution. The changes look good to me. However I believe we should add a TAP test for this change. You can refer the existing TAP tests under test/tap/tests directory and add a similar test for this new configuration variable.

Generally, we aim to keep TAP tests simple and straightforward. The following steps outline one approach we could use to easily verify this patch.

1. Admin Session

  • Update all mysql_servers with gtid_port=0
  • Set ignore_min_gtid_annotations mysql variable to true
  • load mysql variable to runtime; load mysql servers to runtime

2. Proxy Session

  • Run queries with the comment /* min_gtid=<random_uuid> */
  • Expected: Queries should execute successfully on one of the backend connections without failure.

@proton-lisandro-pin
Copy link
Contributor Author

proton-lisandro-pin commented Oct 30, 2025

@proton-lisandro-pin Thanks for the contribution. The changes look good to me. However I believe we should add a TAP test for this change. You can refer the existing TAP tests under test/tap/tests directory and add a similar test for this new configuration variable.

Done, PTAL.

As a side note, is the ProxySQL documentation on a public repository? I wanted to include an entry for ignore_min_gtid_annotations, but couldn't find where.

@wazir-ahmed
Copy link
Member

wazir-ahmed commented Oct 30, 2025 •
edited
Loading

@proton-lisandro-pin I have rebased the commits with v3.0 branch and updated the TAP test (we can further simplify the steps I suggested earlier). CI workflows have been triggered. I'll check the results tomorrow.

@wazir-ahmed
Copy link
Member

wazir-ahmed commented Oct 30, 2025

@proton-lisandro-pin Regarding documentation, the user docs available at proxysql.com/documentation is not open for external contributions at this moment. As per the release plan, we'll handle documenting this new configuration.

proton-lisandro-pin and others added 2 commits October 31, 2025 10:01
@proton-lisandro-pin @wazir-ahmed
Implement a ignore_min_gtid_annotation global variable for ProxySQL
6cb5222 
When true, all `min_gtid` query annotations are ignored; see
https://proxysql.com/documentation/query-annotations/ for details.

This is useful on ProxySQL setups with multiple layers, where some
layers mandate GTID-based routing while others don't.
@proton-lisandro-pin @wazir-ahmed
Add TAP test for mysql-ignore_min_gtid_annotations
c23930b 
Co-authored-by: Wazir Ahmed <wazir@proxysql.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 31, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@renecannao
Copy link
Contributor

renecannao commented Oct 31, 2025

Add to whitelist

@wazir-ahmed wazir-ahmed added this to the Release 3.0.3 milestone Nov 4, 2025
@renecannao renecannao merged commit dfd46fe into sysown:v3.0 Nov 4, 2025
146 of 154 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wazir-ahmed wazir-ahmed wazir-ahmed left review comments

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@wazir-ahmed wazir-ahmed

Labels

None yet

Projects

None yet

Milestone

Release 3.0.3

Development

Successfully merging this pull request may close these issues.

4 participants

@proton-lisandro-pin @renecannao @noizu @wazir-ahmed