Skip to content

[WIP] Setting client side wait_timeout#4901

Merged
renecannao merged 16 commits intov3.0from
v3.0_wait_timeout
Dec 6, 2025
Merged

[WIP] Setting client side wait_timeout#4901
renecannao merged 16 commits intov3.0from
v3.0_wait_timeout

Conversation

@yashwantsahu20
Copy link
Contributor

Setting client side wait_timeout – current ProxySQL ignores wait_timeout set by the client. If a client specifies a value for wait_timeout , ProxySQL should terminate the client connection (but not the backend connection) when said timeout is reached instead of ProxySQL’s global mysql-wait_timeout . As a further enhancement, the wait timeout specified by the client shouldn’t exceed ProxySQL’s wait_timeout . The implementation also requires adequate TAP test to verify the functionality.

@renecannao
Copy link
Contributor

Brainstorming on implementation details.
Should we perform range validation of wait_timeout specified by the client and return an error if out of range? Or maybe it is easier to silently ignore out of range values? (or maybe write an error in error log without notifying the client).

The previous point is also related to: what happens when we change mysql-wait_timeout at runtime?
Currently, the current mysql-wait_timeout is what is used to determine if the client needs to be disconnected, not the value of mysql-wait_timeout when the client connected. This is used during failover for example, to terminate all clients quickly.
So perhaps we should use the smaller of the two , avoiding out of range validation.

Thoughts?

@yashwantsahu20
Copy link
Contributor Author

yashwantsahu20 commented Apr 2, 2025

Thanks René for very nice suggestions.

Please let me know your thought about this approach.

We can do as following

  1. Let's do a basic range validation as done by MySQL to satisfy third-party tools, tests, etc., and stay aligned with MySQL limits.
    Only minimum and maximum values will be validated. This will return an error to the client.
    Example:
      min - 1
      max - one year

https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_wait_timeout

  1. Add an error message to the error log if the session's wait_timeout value is greater than the global one, but still accept and store the value, as the global timeout may be increased in the future. We need to log a message as client may expect longer session timeout while terminating it.

No need to return an error to the client, since from the client’s perspective, there is no visible failure.

  1. While terminating the connection, we will always pick the smaller of the two values (session wait_timeout and global mysql-wait_timeout).

@renecannao
Copy link
Contributor

SGTM

@renecannao
Copy link
Contributor

Can one of the admins verify this patch?

Signed-off-by: Wazir Ahmed <wazir@proxysql.com>
- effective_timeout = min (mysql-wait_timeout, session-wait_timeout)

Signed-off-by: Wazir Ahmed <wazir@proxysql.com>
@wazir-ahmed wazir-ahmed self-assigned this Oct 2, 2025
@renecannao
Copy link
Contributor

retest this please

Signed-off-by: René Cannaò <rene@proxysql.com>
@renecannao
Copy link
Contributor

retest this please

- Add wait_timeout member variable declaration to Base_Session class
- Fix constructor initialization to use this->wait_timeout
- Fix assignment in handler to properly scope member variable
- Resolves compilation error for wait_timeout functionality
…mping

- Add range validation for client SET wait_timeout commands
- Implement clamping between 1 second (1000ms) and 20 days (1,728,000,000ms)
- Add warning messages when values are clamped due to ProxySQL limits
- Maintain MySQL compatibility by accepting larger values than global config
- Fix signed/unsigned comparison warning in wait_timeout assignment
- Ensures client applications don't break while enforcing safety limits
- Include wait_timeout value in session JSON output for monitoring/debugging
- Provides visibility into client-configured timeout values
- Add JSON parsing helper for PROXYSQL INTERNAL SESSION responses
- Add test_wait_timeout_json_values() function with 8 test cases:
  * Zero value (should clamp to 1 second)
  * Valid values (1s, 10s, 5min, 1hr, 24hr, 20 days)
  * Value exceeding 20 days (should clamp to maximum)
- Validate that wait_timeout appears in session JSON output
- Test plan increased from 4 to 12 tests total
- Provides comprehensive validation of wait_timeout clamping behavior
- Replace <json/json.h> include with <json.hpp> and proper nlohmann namespace
- Replace manual JSON parsing with fetch_internal_session() utility function
- Replace fail() calls with ok(false, ...) to follow TAP framework standards
- Add comprehensive JSON validation test for various wait_timeout values
- Include edge case testing: zero values, maximum values, and clamping behavior

Resolves compilation issues and enables proper testing of wait_timeout JSON output.
Key improvements:
- Fix timeout comparison in MySQL_Thread::idle_thread_to_kill_idle_sessions() to prevent underflow
- Use effective wait_timeout (minimum of global and session values) for idle timeout calculations
- Add proper newline characters to proxy_warning messages for consistent log formatting
- Increase test sleep times to account for global timeout enforcement
- Fix session timeout test durations to properly test timeout behavior

Technical changes:
- Replace broken min_idle calculation with proper effective wait_timeout logic
- Add std::min() usage to determine effective timeout from global and session values
- Ensure warning messages end with newline characters for proper log formatting
- Update test sleep durations to ensure proper timeout testing

Resolves potential timeout calculation bugs and ensures consistent timeout enforcement behavior.
Code improvements:
- Extract SESS_TO_SCAN_idle_thread constant to header file for better maintainability
- Replace magic number 128 with named constant in idle_thread_to_kill_idle_sessions()
- Improve code readability and consistency in session scanning logic

Test enhancements:
- Add mysql-poll_timeout configuration for more precise timeout testing
- Reduce test sleep times to 13 seconds for faster test execution
- Add diagnostic messages to clearly show timeout configurations in test output
- Ensure tests properly validate timeout enforcement with precise timing

The changes improve code maintainability and make tests more reliable and faster
while maintaining accurate timeout validation.
…ation

Enhance logging clarity:
- Replace generic IP address with detailed connection info including IP and port
- Use client_myds->addr.addr and client_myds->addr.port for precise identification
- Improve debuggability of timeout clamping and enforcement warnings

The warning messages now provide complete connection details, making it easier
to identify and troubleshoot timeout-related issues in ProxySQL logs.
The get_status_variable() function was only scanning worker threads
but ignoring auxiliary threads (idle threads) where timeout
terminations are detected. This caused the timeout termination
counter to show incorrect/zero values.

- Added idle thread scanning to both overloaded versions of
  get_status_variable() function
- Now properly collects metrics from both worker and idle threads
- Fixes the issue where proxysql_mysql_timeout_terminated_connections_total
  showed zero despite actual timeout terminations

Resolves the metrics reading issue identified in the previous commits.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 6, 2025

@renecannao renecannao merged commit b73160e into v3.0 Dec 6, 2025
61 of 65 checks passed
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.

3 participants