From 6ed82ef8cbe22192a268fb416c46ed08fe442301 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Tue, 25 Nov 2025 13:11:26 +0000 Subject: [PATCH] Fix crash in TCP keepalive warnings for issue #5212 - Fix crash by using get_variable_int() instead of get_variable_string() for boolean use_tcp_keepalive variable - use_tcp_keepalive is a boolean variable, not a string, so get_variable_int() returns 0/1 instead of a string - Fix syntax errors by removing duplicate code and fixing brace structure - Add comprehensive Doxygen documentation for both MySQL and PostgreSQL warnings Resolves assertion failure: "Not existing variable: use_tcp_keepalive" Resolves: #5212 --- lib/Admin_FlushVariables.cpp | 128 +++++++++-------- ...reg_test_5212_tcp_keepalive_warnings-t.cpp | 129 ++++++++++++++++++ 2 files changed, 191 insertions(+), 66 deletions(-) create mode 100644 test/tap/tests/mysql-reg_test_5212_tcp_keepalive_warnings-t.cpp diff --git a/lib/Admin_FlushVariables.cpp b/lib/Admin_FlushVariables.cpp index 3b4a8359d..79019cb81 100644 --- a/lib/Admin_FlushVariables.cpp +++ b/lib/Admin_FlushVariables.cpp @@ -549,38 +549,36 @@ void ProxySQL_Admin::flush_mysql_variables___database_to_runtime(SQLite3DB *db, flush_GENERIC_variables__checksum__database_to_runtime("mysql", checksum, epoch); pthread_mutex_unlock(&GloVars.checksum_mutex); } + /** - * @brief Check and warn if TCP keepalive is disabled for MySQL connections. - * - * This safety check warns users when mysql-use_tcp_keepalive is set to false, - * which can cause connection instability in certain deployment scenarios. - * - * @warning Disabling TCP keepalive is unsafe when ProxySQL is deployed behind: - * - Network load balancers with idle connection timeouts - * - NAT firewalls with connection state timeout - * - Cloud environments with connection pooling - * - Any intermediate network device that drops idle connections - * - * @why_unsafe TCP keepalive sends periodic keep-alive packets on idle connections. - * When disabled: - * - Load balancers may drop connections from their connection pools - * - NAT devices may remove connection state from their tables - * - Cloud load balancers (AWS ELB, GCP Load Balancer, etc.) may terminate - * connections during idle periods - * - Results in sudden connection failures and "connection reset" errors - * - Can cause application downtime and poor user experience - * - * @recommendation Always set mysql-use_tcp_keepalive=true when deploying - * behind load balancers or in cloud environments. - */ - // Check for TCP keepalive setting and warn if disabled - char *mysql_use_tcp_keepalive = GloMTH->get_variable_string((char *)"use_tcp_keepalive"); - if (mysql_use_tcp_keepalive && strcmp(mysql_use_tcp_keepalive, "false") == 0) { - proxy_warning("mysql-use_tcp_keepalive is set to false. This may cause connection drops when ProxySQL is behind a network load balancer. Consider setting this to true.\n"); - } - if (mysql_use_tcp_keepalive) { - free(mysql_use_tcp_keepalive); - } + * @brief Check and warn if TCP keepalive is disabled for MySQL connections. + * + * This safety check warns users when mysql-use_tcp_keepalive is set to false, + * which can cause connection instability in certain deployment scenarios. + * + * @warning Disabling TCP keepalive is unsafe when ProxySQL is deployed behind: + * - Network load balancers with idle connection timeouts + * - NAT firewalls with connection state timeout + * - Cloud environments with connection pooling + * - Any intermediate network device that drops idle connections + * + * @why_unsafe TCP keepalive sends periodic keep-alive packets on idle connections. + * When disabled: + * - Load balancers may drop connections from their connection pools + * - NAT devices may remove connection state from their tables + * - Cloud load balancers (AWS ELB, GCP Load Balancer, etc.) may terminate + * connections during idle periods + * - Results in sudden connection failures and "connection reset" errors + * - Can cause application downtime and poor user experience + * + * @recommendation Always set mysql-use_tcp_keepalive=true when deploying + * behind load balancers or in cloud environments. + */ + // Check for TCP keepalive setting and warn if disabled + int mysql_use_tcp_keepalive = GloMTH->get_variable_int((char *)"use_tcp_keepalive"); + if (mysql_use_tcp_keepalive == 0) { + proxy_warning("mysql-use_tcp_keepalive is set to false. This may cause connection drops when ProxySQL is behind a network load balancer. Consider setting this to true.\n"); + } } if (resultset) delete resultset; } @@ -917,42 +915,40 @@ void ProxySQL_Admin::flush_pgsql_variables___database_to_runtime(SQLite3DB* db, GloVars.checksums_values.mysql_variables.checksum, GloVars.checksums_values.mysql_variables.epoch ); */ + /** - * @brief Check and warn if TCP keepalive is disabled for PostgreSQL connections. - * - * This safety check warns users when pgsql-use_tcp_keepalive is set to false, - * which can cause connection instability in certain deployment scenarios. - * - * @warning Disabling TCP keepalive is unsafe when ProxySQL is deployed behind: - * - Network load balancers with idle connection timeouts - * - NAT firewalls with connection state timeout - * - Cloud environments with connection pooling - * - Any intermediate network device that drops idle connections - * - * @why_unsafe TCP keepalive sends periodic keep-alive packets on idle connections. - * When disabled for PostgreSQL: - * - Load balancers may drop connections from their connection pools - * - NAT devices may remove connection state from their tables - * - Cloud load balancers (AWS ELB, GCP Load Balancer, etc.) may terminate - * connections during idle periods - * - PostgreSQL connections may appear "stale" to the database server - * - Results in sudden connection failures and "connection reset" errors - * - Can cause application downtime and poor user experience - * - * @note PostgreSQL connections are often long-lived and benefit greatly from - * TCP keepalive, especially in connection-pooled environments. - * - * @recommendation Always set pgsql-use_tcp_keepalive=true when deploying - * behind load balancers or in cloud environments. - */ - // Check for TCP keepalive setting and warn if disabled - char *pgsql_use_tcp_keepalive = GloPTH->get_variable_string((char *)"use_tcp_keepalive"); - if (pgsql_use_tcp_keepalive && strcmp(pgsql_use_tcp_keepalive, "false") == 0) { - proxy_warning("pgsql-use_tcp_keepalive is set to false. This may cause connection drops when ProxySQL is behind a network load balancer. Consider setting this to true.\n"); - } - if (pgsql_use_tcp_keepalive) { - free(pgsql_use_tcp_keepalive); - } + * @brief Check and warn if TCP keepalive is disabled for PostgreSQL connections. + * + * This safety check warns users when pgsql-use_tcp_keepalive is set to false, + * which can cause connection instability in certain deployment scenarios. + * + * @warning Disabling TCP keepalive is unsafe when ProxySQL is deployed behind: + * - Network load balancers with idle connection timeouts + * - NAT firewalls with connection state timeout + * - Cloud environments with connection pooling + * - Any intermediate network device that drops idle connections + * + * @why_unsafe TCP keepalive sends periodic keep-alive packets on idle connections. + * When disabled for PostgreSQL: + * - Load balancers may drop connections from their connection pools + * - NAT devices may remove connection state from their tables + * - Cloud load balancers (AWS ELB, GCP Load Balancer, etc.) may terminate + * connections during idle periods + * - PostgreSQL connections may appear "stale" to the database server + * - Results in sudden connection failures and "connection reset" errors + * - Can cause application downtime and poor user experience + * + * @note PostgreSQL connections are often long-lived and benefit greatly from + * TCP keepalive, especially in connection-pooled environments. + * + * @recommendation Always set pgsql-use_tcp_keepalive=true when deploying + * behind load balancers or in cloud environments. + */ + // Check for TCP keepalive setting and warn if disabled + int pgsql_use_tcp_keepalive = GloPTH->get_variable_int((char *)"use_tcp_keepalive"); + if (pgsql_use_tcp_keepalive == 0) { + proxy_warning("pgsql-use_tcp_keepalive is set to false. This may cause connection drops when ProxySQL is behind a network load balancer. Consider setting this to true.\n"); + } } if (resultset) delete resultset; } diff --git a/test/tap/tests/mysql-reg_test_5212_tcp_keepalive_warnings-t.cpp b/test/tap/tests/mysql-reg_test_5212_tcp_keepalive_warnings-t.cpp new file mode 100644 index 000000000..3910c5d60 --- /dev/null +++ b/test/tap/tests/mysql-reg_test_5212_tcp_keepalive_warnings-t.cpp @@ -0,0 +1,129 @@ +#include +#include +#include +#include + +#include +#include +#include + +#include "mysql.h" +#include "tap.h" +#include "command_line.h" +#include "utils.h" + +using namespace std; + +int main(int argc, char** argv) { + CommandLine cl; + + // Plan for 4 tests + plan(4); + + // Get connections + MYSQL* admin = mysql_init(NULL); + if (!admin) { + diag("Failed to initialize admin connection"); + return exit_status(); + } + + if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) { + diag("Failed to connect to ProxySQL admin: %s", mysql_error(admin)); + mysql_close(admin); + return exit_status(); + } + + // Get the log file path + const string log_path { get_env("REGULAR_INFRA_DATADIR") + "/proxysql.log" }; + + // Test 1: MySQL TCP keepalive warning + diag("Testing MySQL TCP keepalive warning when set to false"); + { + fstream logfile {}; + int open_err = open_file_and_seek_end(log_path, logfile); + if (open_err != EXIT_SUCCESS) { + diag("Failed to open log file: %s", log_path.c_str()); + mysql_close(admin); + return exit_status(); + } + + // Set MySQL TCP keepalive to false + int query_err = mysql_query(admin, "SET mysql-use_tcp_keepalive='false'"); + ok(query_err == 0, "SET mysql-use_tcp_keepalive='false' should succeed"); + if (query_err != 0) { + diag("Error setting mysql-use_tcp_keepalive: %s", mysql_error(admin)); + mysql_close(admin); + return exit_status(); + } + + // Load MySQL variables to runtime to trigger warning + query_err = mysql_query(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + ok(query_err == 0, "LOAD MYSQL VARIABLES TO RUNTIME should succeed"); + if (query_err != 0) { + diag("Error loading MySQL variables: %s", mysql_error(admin)); + mysql_close(admin); + return exit_status(); + } + + // Wait a bit for the warning to be written to log + usleep(100000); // 100ms + + // Check for the warning in the log + const string warning_regex { ".*\\[WARNING\\].*mysql-use_tcp_keepalive is set to false.*" }; + const auto& [match_count, warning_lines] = get_matching_lines(logfile, warning_regex, true); + + ok(match_count > 0, "MySQL TCP keepalive warning should appear in log when set to false"); + if (match_count == 0) { + diag("Expected warning not found in log file"); + } else { + diag("Found MySQL TCP keepalive warning in log: %s", std::get(warning_lines[0]).c_str()); + } + } + + // Test 2: PostgreSQL TCP keepalive warning + diag("Testing PostgreSQL TCP keepalive warning when set to false"); + { + fstream logfile {}; + int open_err = open_file_and_seek_end(log_path, logfile); + if (open_err != EXIT_SUCCESS) { + diag("Failed to open log file: %s", log_path.c_str()); + mysql_close(admin); + return exit_status(); + } + + // Set PostgreSQL TCP keepalive to false + int query_err = mysql_query(admin, "SET pgsql-use_tcp_keepalive='false'"); + ok(query_err == 0, "SET pgsql-use_tcp_keepalive='false' should succeed"); + if (query_err != 0) { + diag("Error setting pgsql-use_tcp_keepalive: %s", mysql_error(admin)); + mysql_close(admin); + return exit_status(); + } + + // Load PgSQL variables to runtime to trigger warning + query_err = mysql_query(admin, "LOAD PGSQL VARIABLES TO RUNTIME"); + ok(query_err == 0, "LOAD PGSQL VARIABLES TO RUNTIME should succeed"); + if (query_err != 0) { + diag("Error loading PgSQL variables: %s", mysql_error(admin)); + mysql_close(admin); + return exit_status(); + } + + // Wait a bit for the warning to be written to log + usleep(100000); // 100ms + + // Check for the warning in the log + const string warning_regex { ".*\\[WARNING\\].*pgsql-use_tcp_keepalive is set to false.*" }; + const auto& [match_count, warning_lines] = get_matching_lines(logfile, warning_regex, true); + + ok(match_count > 0, "PostgreSQL TCP keepalive warning should appear in log when set to false"); + if (match_count == 0) { + diag("Expected warning not found in log file"); + } else { + diag("Found PostgreSQL TCP keepalive warning in log: %s", std::get(warning_lines[0]).c_str()); + } + } + + mysql_close(admin); + return exit_status(); +} \ No newline at end of file