Document DNS cache bypass limitation for resolution_family by renecannao · Pull Request #5558 · sysown/proxysql

renecannao · 2026-04-01T09:44:53Z

Summary

Add code comment in mysql_connection.cpp documenting that mysql-resolution_family does not apply when the DNS cache misses and the hostname falls through to mysql_real_connect (which uses the MariaDB client library's hardcoded AF_UNSPEC)

Follow-up to #5554. Tracked in #5555 for future enhancement.

Summary by CodeRabbit

Documentation
- Clarified DNS resolution behavior in connection handling to improve code maintainability.

Note: This release contains no user-visible changes or functional updates.

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.

coderabbitai · 2026-04-01T09:45:14Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 35e17935-c075-4480-9606-44aee2beea6b

📥 Commits

Reviewing files that changed from the base of the PR and between d0625dc and 5b12c7c.

📒 Files selected for processing (1)

lib/mysql_connection.cpp

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{cpp,h,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{cpp,h,hpp}: Class names must use PascalCase with protocol prefixes (MySQL_, PgSQL_, ProxySQL_)
Member variables must use snake_case
Constants and macros must use UPPER_SNAKE_CASE
C++17 is required; use conditional compilation via #ifdef PROXYSQLGENAI, #ifdef PROXYSQL31, etc. for feature flags
Use pthread mutexes for synchronization and std::atomic<> for counters

Files:

lib/mysql_connection.cpp

**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

Use RAII for resource management and jemalloc for memory allocation

Files:

lib/mysql_connection.cpp

lib/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

One class per file is the typical convention

Files:

lib/mysql_connection.cpp

🧠 Learnings (2)

📓 Common learnings

Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5548
File: lib/mysql_connection.cpp:1837-1843
Timestamp: 2026-03-26T16:38:58.553Z
Learning: In ProxySQL's lib/mysql_connection.cpp, `SHOW WARNINGS` detection for both `update_warning_count_from_connection()` and the `add_eof()` call in `ASYNC_USE_RESULT_CONT` intentionally uses `myds->sess->CurrentQuery.QueryParserArgs.digest_text` (comment-stripped digest text). This means the fix/feature does not work when `mysql-query_digests_keep_comment=1` (digest_text contains comments) or `mysql-query_digests=0` (digest_text is unavailable) — these configurations are explicitly excluded from the regression test for `reg_test_5306-show_warnings_with_comment-t`. This design is consistent across the codebase and is an accepted, documented limitation.

📚 Learning: 2026-03-26T16:38:58.553Z

Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5548
File: lib/mysql_connection.cpp:1837-1843
Timestamp: 2026-03-26T16:38:58.553Z
Learning: In ProxySQL's lib/mysql_connection.cpp, `SHOW WARNINGS` detection for both `update_warning_count_from_connection()` and the `add_eof()` call in `ASYNC_USE_RESULT_CONT` intentionally uses `myds->sess->CurrentQuery.QueryParserArgs.digest_text` (comment-stripped digest text). This means the fix/feature does not work when `mysql-query_digests_keep_comment=1` (digest_text contains comments) or `mysql-query_digests=0` (digest_text is unavailable) — these configurations are explicitly excluded from the regression test for `reg_test_5306-show_warnings_with_comment-t`. This design is consistent across the codebase and is an accepted, documented limitation.

Applied to files:

lib/mysql_connection.cpp

🔇 Additional comments (1)

lib/mysql_connection.cpp (1)

962-965: Good clarification of cache-miss behavior and config scope.

This comment precisely documents the fallback resolution path and prevents misinterpretation of mysql-resolution_family coverage.

Based on learnings: accepted, documented limitations in lib/mysql_connection.cpp are an established pattern in this codebase.

📝 Walkthrough

Walkthrough

A clarifying comment is added to the MySQL_Connection::connect_start_DNS_lookup() function explaining that when a DNS cache lookup results in a cache miss, the MariaDB client library handles hostname resolution and the mysql-resolution_family setting does not apply to the cache-miss resolver thread.

Changes

Cohort / File(s)	Summary
DNS Cache-Miss Documentation `lib/mysql_connection.cpp`	Adds explanatory comment in `connect_start_DNS_lookup()` clarifying that cache-miss path delegates hostname resolution to the MariaDB client library via `AF_UNSPEC` and that `mysql-resolution_family` setting does not apply.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related issues

resolution_family: extend to DNS cache bypass path and PgSQL #5555: Directly related—addresses the same DNS cache-miss path and function in MySQL_Connection::connect_start_DNS_lookup().

Poem

🐰 A comment, small but oh so bright,
Makes the cache-miss path crystal-light!
When DNS whispers secrets untold,
Our rabbit knows: MariaDB's got the hold! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Document DNS cache bypass limitation for resolution_family' accurately and specifically describes the main change: adding documentation via a code comment about a DNS cache limitation related to the resolution_family setting.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/5554-resolution-family-limitation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

gemini-code-assist

Code Review

This pull request adds a documentation comment to lib/mysql_connection.cpp to clarify DNS resolution behavior during cache misses. However, the feedback points out that the added comment is misleading because it incorrectly attributes the use of the mysql-resolution_family setting to the DNS cache resolver thread, which actually uses a hardcoded AF_UNSPEC value.

gemini-code-assist · 2026-04-01T09:48:48Z

+		// NOTE: DNS cache miss — hostname is passed directly to
+		// mysql_real_connect which resolves via the MariaDB client library
+		// using AF_UNSPEC. The mysql-resolution_family setting does NOT
+		// apply here; it only governs the DNS cache resolver thread.


The comment states that mysql-resolution_family governs the DNS cache resolver thread. However, the implementation of monitor_dns_resolver_thread in lib/MySQL_Monitor.cpp appears to use a hardcoded AF_UNSPEC for the address family:

// lib/MySQL_Monitor.cpp:4675 hints.ai_family = AF_UNSPEC;

This makes the comment potentially misleading as it suggests mysql-resolution_family is being used by the resolver thread when it is not. To avoid confusion, the comment should be updated to accurately reflect the current implementation.

Copilot

Pull request overview

Adds an inline code comment in the MySQL backend connection path to document a limitation around hostname resolution behavior when the DNS cache does not return an IP (fallback to the MariaDB client library resolver).

Changes:

Document the DNS-cache-miss fallback behavior in MySQL_Connection::connect_start_DNS_lookup().
Clarify (via comment) when AF_UNSPEC is used for resolution in the fallback path.

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

Copilot · 2026-04-01T09:50:28Z

+		// NOTE: DNS cache miss — hostname is passed directly to
+		// mysql_real_connect which resolves via the MariaDB client library
+		// using AF_UNSPEC. The mysql-resolution_family setting does NOT
+		// apply here; it only governs the DNS cache resolver thread.


The new comment refers to a mysql-resolution_family setting and says it “only governs the DNS cache resolver thread”, but in this branch the DNS resolver thread still hardcodes hints.ai_family = AF_UNSPEC (see lib/MySQL_Monitor.cpp:4676) and there are no other references to mysql-resolution_family in the repo. Either stack this PR on top of the change that introduces that setting, or reword this note to avoid documenting a config option that doesn’t exist here. Also consider naming the actual async API used here (mysql_real_connect_start) to match the code path.

sonarqubecloud · 2026-04-01T09:54:15Z

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

Copilot AI review requested due to automatic review settings April 1, 2026 09:44

Copilot started reviewing on behalf of renecannao April 1, 2026 09:46 View session

gemini-code-assist bot reviewed Apr 1, 2026

View reviewed changes

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document DNS cache bypass limitation for resolution_family#5558

Document DNS cache bypass limitation for resolution_family#5558
renecannao wants to merge 1 commit intov3.0from
fix/5554-resolution-family-limitation

renecannao commented Apr 1, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Apr 1, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Apr 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

sonarqubecloud bot commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

renecannao commented Apr 1, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 1, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

renecannao commented Apr 1, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 1, 2026 •

edited

Loading