Skip to content

Add mysql-resolution_family to control IPv4/IPv6 hostname resolution#5554

Draft
Copilot wants to merge 6 commits intov3.0from
copilot/add-flag-choose-ipv4-or-ipv6
Draft

Add mysql-resolution_family to control IPv4/IPv6 hostname resolution#5554
Copilot wants to merge 6 commits intov3.0from
copilot/add-flag-choose-ipv4-or-ipv6

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 31, 2026

ProxySQL currently defers backend hostname resolution entirely to the system resolver, which can cause mysql_servers.hostname entries to resolve to IPv4 or IPv6 unpredictably depending on host policy and DNS responses. This change adds a global MySQL setting to make address-family selection deterministic when needed, while preserving current behavior by default.

  • New global variable

    • Adds mysql-resolution_family
    • Supported values:
      • system — use OS/default resolver behavior (AF_UNSPEC)
      • ipv4 — resolve only IPv4 addresses (AF_INET)
      • ipv6 — resolve only IPv6 addresses (AF_INET6)
  • Runtime variable handling

    • Extends MySQL runtime variable storage to include resolution_family
    • Defaults to system
    • Validates input and rejects unsupported values
    • Normalizes accepted values to lowercase for consistent persistence/runtime reads
  • DNS resolution path

    • Updates the MySQL DNS resolver thread to read mysql-resolution_family
    • Maps the configured value directly into getaddrinfo() hints.ai_family
    • Preserves existing AI_ADDRCONFIG behavior
  • Focused unit coverage

    • Adds standalone unit coverage for:
      • accepted/rejected values
      • normalization behavior
      • mapping to AF_UNSPEC / AF_INET / AF_INET6

Known limitations

  • DNS cache bypass: When the DNS cache does not contain an entry for a hostname (cache miss), the hostname is passed directly to mysql_real_connect which resolves it via the MariaDB client library using hardcoded AF_UNSPEC. The mysql-resolution_family setting does not apply in this path. To ensure the setting is effective, the DNS cache must be enabled and populated (which is the default behavior when monitor is active). A future enhancement may address this by patching the client library.
  • PgSQL: This PR only covers MySQL. A separate PR will add the equivalent pgsql-resolution_family variable.

Example usage

UPDATE global_variables
SET variable_value = 'ipv4'
WHERE variable_name = 'mysql-resolution_family';

LOAD MYSQL VARIABLES TO RUNTIME;
SAVE MYSQL VARIABLES TO DISK;

Copilot AI and others added 2 commits March 31, 2026 15:27
Agent-Logs-Url: https://github.com/sysown/proxysql/sessions/7d0ebed6-886c-4e06-add5-d74b44eced4a

Co-authored-by: renecannao <3645227+renecannao@users.noreply.github.com>
Agent-Logs-Url: https://github.com/sysown/proxysql/sessions/7d0ebed6-886c-4e06-add5-d74b44eced4a

Co-authored-by: renecannao <3645227+renecannao@users.noreply.github.com>
Copilot AI changed the title [WIP] Add flag to select IPv4 or IPv6 for hostname resolution Add mysql-resolution_family to control IPv4/IPv6 hostname resolution Mar 31, 2026
Copilot AI requested a review from renecannao March 31, 2026 15:31
When the DNS cache misses, the hostname is passed directly to
mysql_real_connect which uses the MariaDB client library's own
getaddrinfo with AF_UNSPEC. The mysql-resolution_family setting
only applies to the DNS cache resolver thread, not this fallback
path. Add a comment documenting this known limitation.
@renecannao renecannao marked this pull request as draft April 1, 2026 09:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new MySQL global variable (mysql-resolution_family) to make backend hostname resolution deterministic with respect to IPv4 vs IPv6 by mapping the configured value into getaddrinfo() hints in the MySQL DNS resolver thread, while keeping default behavior (system/AF_UNSPEC) unchanged.

Changes:

  • Introduces include/MySQL_Resolution.h with validation, normalization, and AF_* mapping helpers.
  • Extends MySQL runtime variable storage with resolution_family and uses it in monitor_dns_resolver_thread() to set hints.ai_family.
  • Adds a focused standalone unit test and wires it into the unit test build.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
include/MySQL_Resolution.h Header-only helpers for validating/normalizing the setting and mapping to AF_UNSPEC/AF_INET/AF_INET6.
include/MySQL_Thread.h Adds resolution_family to MySQL thread variables struct.
lib/MySQL_Thread.cpp Registers, defaults, exposes, validates, and persists resolution_family in MySQL runtime variables.
lib/MySQL_Monitor.cpp Applies resolution_family to getaddrinfo() hints in the DNS resolver thread.
lib/mysql_connection.cpp Documents the known limitation for DNS-cache-miss path using mysql_real_connect.
test/tap/tests/unit/mysql_resolution_unit-t.cpp Unit coverage for validation/normalization/mapping behavior.
test/tap/tests/unit/Makefile Adds the new unit test target and build rule.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +4686 to +4691
hints.ai_flags = AI_ADDRCONFIG;
char* resolution_family = GloMTH ? GloMTH->get_variable_string((char *)"resolution_family") : NULL;
hints.ai_family = mysql_resolution_family_to_ai_family(resolution_family);
if (resolution_family) {
free(resolution_family);
}
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

GloMTH->get_variable_string("resolution_family") is called without holding the MySQL threads handler lock. set_variable() updates string variables by free() + strdup(), and get_variable_string() does a strdup() of the underlying pointer without internal locking, so this read can race with a concurrent variable update and lead to use-after-free/crash. Acquire GloMTH->wrlock() (there is no rdlock) around reading resolution_family (and release it before getaddrinfo()), or otherwise fetch the configured family in a thread-safe/cached way.

Copilot uses AI. Check for mistakes.
@renecannao
Copy link
Copy Markdown
Contributor

Review: Thread storage variable missing for resolution_family

The PR reads the variable value directly via GloMTH->get_variable_string() in monitor_dns_resolver_thread():

char* resolution_family = GloMTH ? GloMTH->get_variable_string((char *)"resolution_family") : NULL;

This is incorrect. ProxySQL uses thread-local storage variables (mysql_thread___ prefix) that get populated during MySQL_Thread::refresh_variables(). The monitor threads read these thread-local copies — never calling GloMTH->get_variable_string() directly from hot paths. Calling get_variable_string() directly requires taking a lock on the handler's variables, which is both a performance concern and inconsistent with the established pattern.

What needs to be added:

  1. include/proxysql_structs.h — Add the thread-local variable in both blocks:

    • Definition block (~line 1404, near the other monitor_local_dns_* vars):
      __thread char * mysql_thread___resolution_family;
    • Extern block (~line 1736):
      extern __thread char * mysql_thread___resolution_family;
  2. lib/MySQL_Thread.cpprefresh_variables() (~line 4660, after monitor_local_dns_resolver_queue_maxsize):

    REFRESH_VARIABLE_CHAR(resolution_family);
  3. lib/MySQL_Thread.cpp — destructor (~line 3252, with the other char* cleanups):

    if (mysql_thread___resolution_family) { free(mysql_thread___resolution_family); mysql_thread___resolution_family=NULL; }
  4. lib/MySQL_Monitor.cppmonitor_dns_resolver_thread() — Replace the direct call:

    // Before (wrong):
    char* resolution_family = GloMTH ? GloMTH->get_variable_string((char *)"resolution_family") : NULL;
    hints.ai_family = mysql_resolution_family_to_ai_family(resolution_family);
    if (resolution_family) { free(resolution_family); }
    
    // After (correct):
    hints.ai_family = mysql_resolution_family_to_ai_family(mysql_thread___resolution_family);

This follows the exact same pattern used by all other monitor variables (monitor_local_dns_cache_ttl, monitor_username, etc.).

Replace direct GloMTH->get_variable_string() call in
monitor_dns_resolver_thread() with the proper mysql_thread___
thread-local variable pattern:

- Add __thread char* mysql_thread___resolution_family in proxysql_structs.h
- Add REFRESH_VARIABLE_CHAR(resolution_family) in refresh_variables()
- Free the thread-local variable in MySQL_Thread destructor
- Read mysql_thread___resolution_family directly in the monitor thread
  instead of taking a lock via get_variable_string()
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 1, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: add flag/param to choose between IPv4 or IPv6 only when resolving hostnames

3 participants