From f253e078118fd77abf44f12d1813099c805ee15d Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 8 Nov 2024 00:05:48 +0500 Subject: [PATCH 1/4] Added overflow_safe_multiply() --- include/gen_utils.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/gen_utils.h b/include/gen_utils.h index 33f5afc34..11a58026e 100644 --- a/include/gen_utils.h +++ b/include/gen_utils.h @@ -314,6 +314,18 @@ inline unsigned long long realtime_time() { return (((unsigned long long) ts.tv_sec) * 1000000) + (ts.tv_nsec / 1000); } +template +inline T overflow_safe_multiply(T val) { + static_assert(std::is_integral::value, "T must be an integer type."); + static_assert(std::is_unsigned_v, "T must be an unsigned integer type."); + static_assert(FACTOR > 0, "Negative factors are not supported."); + + if constexpr (FACTOR == 0) return 0; + if (val == 0) return 0; + if (val > std::numeric_limits::max() / FACTOR) return std::numeric_limits::max(); + return (val * FACTOR); +} + #endif /* __GEN_FUNCTIONS */ bool Proxy_file_exists(const char *); From 3b5671267fa7249ba3e320b7d62acb0d245528c4 Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 8 Nov 2024 00:50:11 +0500 Subject: [PATCH 2/4] Implemented overflow-safe multiplication for mysql_thread___threshold_resultset_size and pgsql_thread___threshold_resultset_size --- lib/Base_Thread.cpp | 4 ++-- lib/PgSQL_Connection.cpp | 6 +++--- lib/mysql_connection.cpp | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/Base_Thread.cpp b/lib/Base_Thread.cpp index 50cbb0c18..f30ca6af8 100644 --- a/lib/Base_Thread.cpp +++ b/lib/Base_Thread.cpp @@ -380,7 +380,7 @@ bool Base_Thread::set_backend_to_be_skipped_if_frontend_is_slow(DS * myds, unsig unsigned int buffered_data = 0; buffered_data = myds->sess->client_myds->PSarrayOUT->len * PGSQL_RESULTSET_BUFLEN; buffered_data += myds->sess->client_myds->resultset->len * PGSQL_RESULTSET_BUFLEN; - if (buffered_data > (unsigned int)pgsql_thread___threshold_resultset_size * 4) { + if (buffered_data > overflow_safe_multiply<4,unsigned int>(pgsql_thread___threshold_resultset_size)) { thr->mypolls.fds[n].events = 0; return true; } @@ -388,7 +388,7 @@ bool Base_Thread::set_backend_to_be_skipped_if_frontend_is_slow(DS * myds, unsig unsigned int buffered_data = 0; buffered_data = myds->sess->client_myds->PSarrayOUT->len * RESULTSET_BUFLEN; buffered_data += myds->sess->client_myds->resultset->len * RESULTSET_BUFLEN; - if (buffered_data > (unsigned int)mysql_thread___threshold_resultset_size * 4) { + if (buffered_data > overflow_safe_multiply<4,unsigned int>(mysql_thread___threshold_resultset_size)) { thr->mypolls.fds[n].events = 0; return true; } diff --git a/lib/PgSQL_Connection.cpp b/lib/PgSQL_Connection.cpp index fa665e312..1e887612f 100644 --- a/lib/PgSQL_Connection.cpp +++ b/lib/PgSQL_Connection.cpp @@ -1760,7 +1760,7 @@ PG_ASYNC_ST PgSQL_Connection::handler(short event) { unsigned int buffered_data = 0; buffered_data = myds->sess->client_myds->PSarrayOUT->len * PGSQL_RESULTSET_BUFLEN; buffered_data += myds->sess->client_myds->resultset->len * PGSQL_RESULTSET_BUFLEN; - if (buffered_data > (unsigned int)pgsql_thread___threshold_resultset_size * 8) { + if (buffered_data > overflow_safe_multiply<8,unsigned int>(pgsql_thread___threshold_resultset_size)) { next_event(ASYNC_USE_RESULT_CONT); // we temporarily pause . See #1232 break; } @@ -1858,7 +1858,7 @@ PG_ASYNC_ST PgSQL_Connection::handler(short event) { update_bytes_recv(bytes_recv); processed_bytes += bytes_recv; // issue #527 : this variable will store the amount of bytes processed during this event if ( - (processed_bytes > (unsigned int)pgsql_thread___threshold_resultset_size * 8) + (processed_bytes > overflow_safe_multiply<8,unsigned int>(pgsql_thread___threshold_resultset_size)) || (pgsql_thread___throttle_ratio_server_to_client && pgsql_thread___throttle_max_bytes_per_second_to_client && (processed_bytes > (unsigned long long)pgsql_thread___throttle_max_bytes_per_second_to_client / 10 * (unsigned long long)pgsql_thread___throttle_ratio_server_to_client)) ) { @@ -1880,7 +1880,7 @@ PG_ASYNC_ST PgSQL_Connection::handler(short event) { processed_bytes += bytes_recv; // issue #527 : this variable will store the amount of bytes processed during this event if ( - (processed_bytes > (unsigned int)pgsql_thread___threshold_resultset_size * 8) + (processed_bytes > overflow_safe_multiply<8,unsigned int>(pgsql_thread___threshold_resultset_size)) || (pgsql_thread___throttle_ratio_server_to_client && pgsql_thread___throttle_max_bytes_per_second_to_client && (processed_bytes > (unsigned long long)pgsql_thread___throttle_max_bytes_per_second_to_client / 10 * (unsigned long long)pgsql_thread___throttle_ratio_server_to_client)) ) { diff --git a/lib/mysql_connection.cpp b/lib/mysql_connection.cpp index 42013c1d5..50c0ff321 100644 --- a/lib/mysql_connection.cpp +++ b/lib/mysql_connection.cpp @@ -1506,7 +1506,7 @@ MDB_ASYNC_ST MySQL_Connection::handler(short event) { unsigned int buffered_data=0; buffered_data = myds->sess->client_myds->PSarrayOUT->len * RESULTSET_BUFLEN; buffered_data += myds->sess->client_myds->resultset->len * RESULTSET_BUFLEN; - if (buffered_data > (unsigned int)mysql_thread___threshold_resultset_size*8) { + if (buffered_data > overflow_safe_multiply<8,unsigned int>(mysql_thread___threshold_resultset_size)) { next_event(ASYNC_STMT_EXECUTE_STORE_RESULT_CONT); // we temporarily pause . See #1232 break; } @@ -1532,7 +1532,7 @@ MDB_ASYNC_ST MySQL_Connection::handler(short event) { if (rows_read_inner > 1) { process_rows_in_ASYNC_STMT_EXECUTE_STORE_RESULT_CONT(processed_bytes); if ( - (processed_bytes > (unsigned int)mysql_thread___threshold_resultset_size*8) + (processed_bytes > overflow_safe_multiply<8,unsigned int>(mysql_thread___threshold_resultset_size)) || ( mysql_thread___throttle_ratio_server_to_client && mysql_thread___throttle_max_bytes_per_second_to_client && (processed_bytes > (unsigned long long)mysql_thread___throttle_max_bytes_per_second_to_client/10*(unsigned long long)mysql_thread___throttle_ratio_server_to_client) ) ) { @@ -1685,7 +1685,7 @@ MDB_ASYNC_ST MySQL_Connection::handler(short event) { unsigned int buffered_data=0; buffered_data = myds->sess->client_myds->PSarrayOUT->len * RESULTSET_BUFLEN; buffered_data += myds->sess->client_myds->resultset->len * RESULTSET_BUFLEN; - if (buffered_data > (unsigned int)mysql_thread___threshold_resultset_size*8) { + if (buffered_data > overflow_safe_multiply<8,unsigned int>(mysql_thread___threshold_resultset_size)) { next_event(ASYNC_USE_RESULT_CONT); // we temporarily pause . See #1232 break; } @@ -1739,7 +1739,7 @@ MDB_ASYNC_ST MySQL_Connection::handler(short event) { bytes_info.bytes_recv += br; processed_bytes+=br; // issue #527 : this variable will store the amount of bytes processed during this event if ( - (processed_bytes > (unsigned int)mysql_thread___threshold_resultset_size*8) + (processed_bytes > overflow_safe_multiply<8,unsigned int>(mysql_thread___threshold_resultset_size)) || ( mysql_thread___throttle_ratio_server_to_client && mysql_thread___throttle_max_bytes_per_second_to_client && (processed_bytes > (unsigned long long)mysql_thread___throttle_max_bytes_per_second_to_client/10*(unsigned long long)mysql_thread___throttle_ratio_server_to_client) ) ) { From 23d2f6cb995b609c9adadfbb1f6ef0c9c4bcd997 Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 8 Nov 2024 00:50:40 +0500 Subject: [PATCH 3/4] Added TAP test for MySQL --- ...g_test_4707_threshold_resultset_size-t.cpp | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 test/tap/tests/mysql-reg_test_4707_threshold_resultset_size-t.cpp diff --git a/test/tap/tests/mysql-reg_test_4707_threshold_resultset_size-t.cpp b/test/tap/tests/mysql-reg_test_4707_threshold_resultset_size-t.cpp new file mode 100644 index 000000000..3308936b4 --- /dev/null +++ b/test/tap/tests/mysql-reg_test_4707_threshold_resultset_size-t.cpp @@ -0,0 +1,113 @@ + /** + * @file mysql-reg_test_4707_threshold_resultset_size-t.cpp + * @brief The test specifically examines the impact of different mysql-threshold_resultset_size threshold values on query response times + * and addresses an identified issue caused by variable overflow, which results in slow performance. + */ + +#include +#include +#include +#include "mysql.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +CommandLine cl; + +int main(int argc, char** argv) { + + plan(6); // Total number of tests planned + + if (cl.getEnv()) + return exit_status(); + + // Initialize Admin connection + MYSQL* proxysql_admin = mysql_init(NULL); + if (!proxysql_admin) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_admin)); + return -1; + } + + // Connnect to ProxySQL Admin + if (!mysql_real_connect(proxysql_admin, cl.admin_host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_admin)); + return -1; + } + + // Initialize Backend connection + MYSQL* proxysql_backend = mysql_init(NULL); + if (!proxysql_backend) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_backend)); + return -1; + } + + // Connnect to ProxySQL Backend + if (!mysql_real_connect(proxysql_backend, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_backend)); + return -1; + } + MYSQL_QUERY(proxysql_admin, "DELETE FROM mysql_query_rules"); + MYSQL_QUERY(proxysql_admin, "LOAD MYSQL QUERY RULES TO RUNTIME"); + MYSQL_QUERY(proxysql_admin, "SET mysql-poll_timeout=2000"); + MYSQL_QUERY(proxysql_admin, "SET mysql-threshold_resultset_size=8000"); + MYSQL_QUERY(proxysql_admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + + int rc; + + auto start = std::chrono::high_resolution_clock::now(); + rc = mysql_query(proxysql_backend, "SELECT 1"); + auto end = std::chrono::high_resolution_clock::now(); + + if (rc == 0) { + MYSQL_RES* res = mysql_store_result(proxysql_backend); + ok(res != nullptr, "Query executed successfully. %s", mysql_error(proxysql_backend)); + mysql_free_result(res); + } + else { + ok(false, "Error executing query. %s", mysql_error(proxysql_admin)); + } + + std::chrono::duration duration = end - start; + ok(duration.count() < 10.00, "Execution time should be less than 10 ms. Actual: %f ms", duration.count()); + + MYSQL_QUERY(proxysql_admin, "SET mysql-threshold_resultset_size=536870912"); + MYSQL_QUERY(proxysql_admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + + start = std::chrono::high_resolution_clock::now(); + rc = mysql_query(proxysql_backend, "SELECT 1"); + end = std::chrono::high_resolution_clock::now(); + + if (rc == 0) { + MYSQL_RES* res = mysql_store_result(proxysql_backend); + ok(res != nullptr, "Query executed successfully. %s", mysql_error(proxysql_backend)); + mysql_free_result(res); + } + else { + ok(false, "Error executing query. %s", mysql_error(proxysql_admin)); + } + duration = end - start; + ok(duration.count() < 10.00, "Execution time should be less than 10 ms. Actual: %f ms", duration.count()); + + MYSQL_QUERY(proxysql_admin, "SET mysql-threshold_resultset_size=1073741824"); + MYSQL_QUERY(proxysql_admin, "LOAD MYSQL VARIABLES TO RUNTIME"); + + start = std::chrono::high_resolution_clock::now(); + rc = mysql_query(proxysql_backend, "SELECT 1"); + end = std::chrono::high_resolution_clock::now(); + + if (rc == 0) { + MYSQL_RES* res = mysql_store_result(proxysql_backend); + ok(res != nullptr, "Query executed successfully. %s", mysql_error(proxysql_backend)); + mysql_free_result(res); + } + else { + ok(false, "Error executing query. %s", mysql_error(proxysql_admin)); + } + duration = end - start; + ok(duration.count() < 10.00, "Execution time should be less than 10 ms. Actual: %f ms", duration.count()); + + mysql_close(proxysql_backend); + mysql_close(proxysql_admin); + + return exit_status(); +} From 0b09b4c5975b262628a424a02a6dae7ce746568d Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 8 Nov 2024 12:13:49 +0500 Subject: [PATCH 4/4] Added TAP test for PgSQL --- ...g_test_4707_threshold_resultset_size-t.cpp | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 test/tap/tests/pgsql-reg_test_4707_threshold_resultset_size-t.cpp diff --git a/test/tap/tests/pgsql-reg_test_4707_threshold_resultset_size-t.cpp b/test/tap/tests/pgsql-reg_test_4707_threshold_resultset_size-t.cpp new file mode 100644 index 000000000..dc6fb21ed --- /dev/null +++ b/test/tap/tests/pgsql-reg_test_4707_threshold_resultset_size-t.cpp @@ -0,0 +1,134 @@ + /** + * @file pgsql-reg_test_4707_threshold_resultset_size-t.cpp + * @brief The test specifically examines the impact of different pgsql-threshold_resultset_size threshold values on query response times + * and addresses an identified issue caused by variable overflow, which results in slow performance. + */ + +#include +#include +#include +#include "libpq-fe.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +CommandLine cl; + +using PGConnPtr = std::unique_ptr; + +enum ConnType { + ADMIN, + BACKEND +}; + +PGConnPtr createNewConnection(ConnType conn_type, bool with_ssl) { + + const char* host = (conn_type == BACKEND) ? cl.pgsql_host : cl.pgsql_admin_host; + int port = (conn_type == BACKEND) ? cl.pgsql_port : cl.pgsql_admin_port; + const char* username = (conn_type == BACKEND) ? cl.pgsql_username : cl.admin_username; + const char* password = (conn_type == BACKEND) ? cl.pgsql_password : cl.admin_password; + + std::stringstream ss; + + ss << "host=" << host << " port=" << port; + ss << " user=" << username << " password=" << password; + ss << (with_ssl ? " sslmode=require" : " sslmode=disable"); + + PGconn* conn = PQconnectdb(ss.str().c_str()); + if (PQstatus(conn) != CONNECTION_OK) { + fprintf(stderr, "Connection failed to '%s': %s", (conn_type == BACKEND ? "Backend" : "Admin"), PQerrorMessage(conn)); + PQfinish(conn); + return PGConnPtr(nullptr, &PQfinish); + } + return PGConnPtr(conn, &PQfinish); +} + +bool executeQueries(PGconn* conn, const std::vector& queries) { + for (const auto& query : queries) { + diag("Running: %s", query.c_str()); + PGresult* res = PQexec(conn, query.c_str()); + bool success = PQresultStatus(res) == PGRES_TUPLES_OK || + PQresultStatus(res) == PGRES_COMMAND_OK; + if (!success) { + fprintf(stderr, "Failed to execute query '%s': %s", + query.c_str(), PQerrorMessage(conn)); + PQclear(res); + return false; + } + PQclear(res); + } + return true; +} + +int main(int argc, char** argv) { + + plan(6); // Total number of tests planned + + if (cl.getEnv()) + return exit_status(); + + // Connnect to ProxySQL Admin + PGConnPtr admin_conn = createNewConnection(ConnType::ADMIN, false); + + if (!admin_conn) { + BAIL_OUT("Error: failed to connect to the database in file %s, line %d\n", __FILE__, __LINE__); + return exit_status(); + } + + // Connnect to ProxySQL Backend + PGConnPtr backend_conn = createNewConnection(ConnType::BACKEND, false); + + if (!backend_conn) { + BAIL_OUT("Error: failed to connect to the database in file %s, line %d\n", __FILE__, __LINE__); + return exit_status(); + } + + if (!executeQueries(admin_conn.get(), { + "DELETE FROM pgsql_query_rules", + "LOAD PGSQL QUERY RULES TO RUNTIME", + "SET pgsql-poll_timeout=2000", + "SET pgsql-threshold_resultset_size=8000", + "LOAD PGSQL VARIABLES TO RUNTIME" })) + return exit_status(); + + bool success; + + auto start = std::chrono::high_resolution_clock::now(); + success = executeQueries(backend_conn.get(), { "SELECT 1" }); + auto end = std::chrono::high_resolution_clock::now(); + + ok(success, "Query executed successfully. %s", PQerrorMessage(backend_conn.get())); + + std::chrono::duration duration = end - start; + ok(duration.count() < 10.00, "Execution time should be less than 10 ms. Actual: %f ms", duration.count()); + + if (!executeQueries(admin_conn.get(), { + "SET pgsql-threshold_resultset_size=536870912", + "LOAD PGSQL VARIABLES TO RUNTIME" })) + return exit_status(); + + start = std::chrono::high_resolution_clock::now(); + success = executeQueries(backend_conn.get(), { "SELECT 1" }); + end = std::chrono::high_resolution_clock::now(); + + ok(success, "Query executed successfully. %s", PQerrorMessage(backend_conn.get())); + + duration = end - start; + ok(duration.count() < 10.00, "Execution time should be less than 10 ms. Actual: %f ms", duration.count()); + + if (!executeQueries(admin_conn.get(), { + "SET pgsql-threshold_resultset_size=1073741824", + "LOAD PGSQL VARIABLES TO RUNTIME" })) + return exit_status(); + + start = std::chrono::high_resolution_clock::now(); + success = executeQueries(backend_conn.get(), { "SELECT 1" }); + end = std::chrono::high_resolution_clock::now(); + + ok(success, "Query executed successfully. %s", PQerrorMessage(backend_conn.get())); + + duration = end - start; + ok(duration.count() < 10.00, "Execution time should be less than 10 ms. Actual: %f ms", duration.count()); + + return exit_status(); +}