[WIP] Setting client side wait_timeout#4901
Conversation
|
Brainstorming on implementation details. The previous point is also related to: what happens when we change Thoughts? |
|
Thanks René for very nice suggestions. Please let me know your thought about this approach. We can do as following
https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_wait_timeout
No need to return an error to the client, since from the client’s perspective, there is no visible failure.
|
|
SGTM |
|
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>
|
retest this please |
Signed-off-by: René Cannaò <rene@proxysql.com>
|
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.
|



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.