Skip to content

Commit

Permalink
Merge pull request #4744 from sysown/v3.0_issue_4707_threshold_result…
Browse files Browse the repository at this point in the history
…set_size

Fix Overflow in *-threshold_resultset_size during Multiplication - v3.0
  • Loading branch information
renecannao authored Nov 11, 2024
2 parents b3ae0df + 0b09b4c commit ce4394e
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 9 deletions.
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();
}

0 comments on commit ce4394e

Please sign in to comment.