fix: show idle MySQL sessions with textual processlist boolean - #6031
Conversation
📝 WalkthroughWalkthroughThe change updates MySQL idle-session processlist boolean parsing and adds TAP coverage for disabled and enabled visibility. The test is registered across supported MySQL-related groups. PostgreSQL handling remains unchanged. ChangesMySQL idle processlist visibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant TAPTest
participant ProxySQLAdmin
participant ProxySQLFrontend
participant Processlist
TAPTest->>ProxySQLAdmin: Set idle-session visibility to false
TAPTest->>ProxySQLFrontend: Create an idle session
TAPTest->>Processlist: Verify the session is hidden
TAPTest->>ProxySQLAdmin: Set idle-session visibility to true
TAPTest->>Processlist: Verify the session is shown
TAPTest->>ProxySQLAdmin: Restore defaults
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Code Review ✅ ApprovedFixes MySQL processlist idle boolean synchronization by correctly parsing textual true/false values instead of relying on atoi, and adds a TAP regression test. No issues found. OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07d5fd2cc3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| if (!processlist_count(admin, session_id, &count)) { |
There was a problem hiding this comment.
Under scheduler contention, the preceding wait for a zero count can finish while the session is still being transferred through idle_mysql_sessions or myexchange.idle_mysql_sessions, before idle_thread_gets_sessions_from_worker_thread() registers it with an idle thread; those transfer queues are not enumerated by SQL3_Processlist(). The single query here can consequently still return zero after the flag is correctly enabled, making this newly registered CI test flaky. Use wait_for_processlist_count(..., 1, ...) for the enabled state as well.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3.0 #6031 +/- ##
==========================================
- Coverage 53.40% 53.37% -0.04%
==========================================
Files 487 488 +1
Lines 145258 145358 +100
Branches 36704 36729 +25
==========================================
+ Hits 77579 77582 +3
- Misses 50732 50747 +15
- Partials 16947 17029 +82
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Summary
true/falsevalues correctly when synchronizingmysql-session_idle_show_processlistinto the admin processlist configuration.falseand visible idle sessions with textualtrue.Root cause
The MySQL processlist callback used
atoi(varvalue). Although the MySQL variable accepted the textual valuetrue,atoi("true")evaluates to zero. The separate admin-side processlist configuration therefore continued hiding sessions moved to idle maintenance threads.Validation
Passing:
make -j4 build_tap_test_debug(exit 0; includes the documented dependency, library, ProxySQL, and TAP build chain)reg_test_processlist_idle_boolean-tpassed 2/2 assertionsfalse: idle session hidden, count 0true: idle session visible, count 1python3 test/tap/groups/lint_groups_json.pypython3 test/tap/groups/check_groups.py --sourcegit diff --check origin/v3.0...HEADThe earlier direct component make invocations omitted the repository's dependency bootstrap chain and were not valid clean-checkout validation; the resulting "missing mysql.h/curl" note was incorrect.
Summary by CodeRabbit
Bug Fixes
true/falseand1/0values.Tests
Documentation