Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Overflow in *-threshold_resultset_size during Multiplication - v3.0 #4744

Merged
merged 4 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions include/gen_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,18 @@ inline unsigned long long realtime_time() {
return (((unsigned long long) ts.tv_sec) * 1000000) + (ts.tv_nsec / 1000);
}

template<int FACTOR, typename T>
inline T overflow_safe_multiply(T val) {
static_assert(std::is_integral<T>::value, "T must be an integer type.");
static_assert(std::is_unsigned_v<T>, "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<T>::max() / FACTOR) return std::numeric_limits<T>::max();
return (val * FACTOR);
}

#endif /* __GEN_FUNCTIONS */

bool Proxy_file_exists(const char *);
Expand Down
4 changes: 2 additions & 2 deletions lib/Base_Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -380,15 +380,15 @@ 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;
}
} else if constexpr (std::is_same_v<T, MySQL_Thread>) {
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;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/PgSQL_Connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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))
) {
Expand All @@ -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))
) {
Expand Down
8 changes: 4 additions & 4 deletions lib/mysql_connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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) )
) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) )
) {
Expand Down
113 changes: 113 additions & 0 deletions test/tap/tests/mysql-reg_test_4707_threshold_resultset_size-t.cpp
Original file line number Diff line number Diff line change
@@ -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 <string>
#include <sstream>
#include <chrono>
#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<double, std::milli> 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();
}
134 changes: 134 additions & 0 deletions test/tap/tests/pgsql-reg_test_4707_threshold_resultset_size-t.cpp
Original file line number Diff line number Diff line change
@@ -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 <string>
#include <sstream>
#include <chrono>
#include "libpq-fe.h"
#include "command_line.h"
#include "tap.h"
#include "utils.h"

CommandLine cl;

using PGConnPtr = std::unique_ptr<PGconn, decltype(&PQfinish)>;

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<std::string>& 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<double, std::milli> 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();
}