From 3329a671e51a266d6fa7f9193f611e2e5e8fd13a Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Nov 2025 19:13:40 +0100 Subject: [PATCH] Add extensive documentation for fast forward grace close feature --- lib/MySQL_Session.cpp | 15 ++++++++--- lib/mysql_data_stream.cpp | 25 ++++++++++++++++++ test/tap/tests/fast_forward_grace_close.cpp | 28 ++++++++++++++++----- 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp index c42333cbe..434872c96 100644 --- a/lib/MySQL_Session.cpp +++ b/lib/MySQL_Session.cpp @@ -3782,9 +3782,18 @@ int MySQL_Session::GPFC_Statuses2(bool& wrong_pass, PtrSize_t& pkt) { break; case FAST_FORWARD: mybe->server_myds->PSarrayOUT->add(pkt.ptr, pkt.size); - // Fast Forward Grace Close Logic: - // If the backend closed during fast forward mode, we defer session closure to allow - // pending client output buffers to drain, preventing data loss. + /* + * Fast Forward Grace Close Logic: + * In fast forward mode, ProxySQL forwards packets without buffering them. + * If the backend connection closes unexpectedly while client data is still + * being sent, we risk data loss. To mitigate this, we implement a grace + * period where the session remains open until all pending client output + * buffers are drained or a timeout (mysql_thread___fast_forward_grace_close_ms) + * is reached. + * + * This logic detects backend closure, starts the grace timer, and waits + * for buffers to empty before closing the session. + */ // Detect if backend closed during fast forward if (mybe->server_myds->status == MYSQL_SERVER_STATUS_OFFLINE_HARD || mybe->server_myds->fd == -1) { if (!backend_closed_in_fast_forward) { diff --git a/lib/mysql_data_stream.cpp b/lib/mysql_data_stream.cpp index 8616746fc..5672aa862 100644 --- a/lib/mysql_data_stream.cpp +++ b/lib/mysql_data_stream.cpp @@ -576,6 +576,16 @@ int MySQL_Data_Stream::read_from_net() { } else { // Shutdown if we either received the EOF, or operation failed with non-retryable error. if (ssl_recv_bytes==0 || (ssl_recv_bytes==-1 && errno != EINTR && errno != EAGAIN)) { + /* + * Fast Forward Grace Close Logic: + * When the backend connection closes unexpectedly (EOF) during fast forward mode, + * instead of immediately closing the session, we check if there are pending + * client output buffers. If so, we initiate a grace period to allow the + * buffers to drain before closing the session. + * + * This prevents data loss in fast forward scenarios where ProxySQL forwards + * packets without buffering, and the backend closes before all data is sent. + */ if (myds_type == MYDS_BACKEND && sess && sess->session_fast_forward && ssl_recv_bytes==0) { if (PSarrayIN->len > 0 || sess->client_myds->PSarrayOUT->len > 0 || queue_data(sess->client_myds->queueOUT) > 0) { if (sess->backend_closed_in_fast_forward == false) { @@ -601,6 +611,10 @@ int MySQL_Data_Stream::read_from_net() { if (encrypted==false) { int myds_errno=errno; if (r==0 || (r==-1 && myds_errno != EINTR && myds_errno != EAGAIN)) { + /* + * Fast Forward Grace Close Logic: + * Similar check for non-encrypted connections when backend closes with EOF. + */ if (myds_type == MYDS_BACKEND && sess && sess->session_fast_forward && r==0) { if (PSarrayIN->len > 0 || sess->client_myds->PSarrayOUT->len > 0 || queue_data(sess->client_myds->queueOUT) > 0) { if (sess->backend_closed_in_fast_forward == false) { @@ -644,6 +658,10 @@ int MySQL_Data_Stream::read_from_net() { if ( (revents & POLLHUP) ) { // this is a final check // Only if the amount of data read is 0 or less, then we check POLLHUP + /* + * Fast Forward Grace Close Logic: + * Handle POLLHUP event similarly, initiating grace close if buffers are pending. + */ if (myds_type == MYDS_BACKEND && sess && sess->session_fast_forward) { if (PSarrayIN->len > 0 || sess->client_myds->PSarrayOUT->len > 0 || queue_data(sess->client_myds->queueOUT) > 0) { if (sess->backend_closed_in_fast_forward == false) { @@ -799,6 +817,13 @@ void MySQL_Data_Stream::set_pollout() { } else { _pollfd->events = POLLIN; if (myds_type == MYDS_BACKEND && sess && sess->session_fast_forward && sess->backend_closed_in_fast_forward == true) { + /* + * Fast Forward Grace Close Logic: + * During the grace period after backend closure, we manage polling to avoid busy-waiting. + * If POLLIN is set, poll() will return immediately since the socket is closed, + * causing the thread to spin. To prevent this, we clear POLLIN during the grace period + * and rely on timeouts to eventually close the session. + */ // this is a fast forward session where the backend connection was already closed // if we set POLLIN : the thread will spin on poll() until the socket is closed // if we do not set POLLIN : we won't be able to timeout diff --git a/test/tap/tests/fast_forward_grace_close.cpp b/test/tap/tests/fast_forward_grace_close.cpp index 50bb6c938..7afa73a8c 100644 --- a/test/tap/tests/fast_forward_grace_close.cpp +++ b/test/tap/tests/fast_forward_grace_close.cpp @@ -1,4 +1,20 @@ #include +/* + * Fast Forward Grace Close Test + * + * This test validates the fast forward grace close feature in ProxySQL. + * The feature prevents data loss by allowing pending client output buffers + * to drain before closing sessions when the backend closes unexpectedly + * in fast forward mode. + * + * Test Strategy: + * - Generate a large binlog on the backend. + * - Connect via ProxySQL and read the binlog in a throttled manner to trigger + * fast forward mode closure. + * - Verify that the grace close logic allows buffers to drain without data loss. + * - Really slow connection should fail + */ + #include #include #include @@ -36,7 +52,7 @@ int main() { // 0 means no limit // we skip 8 because or the edge std::vector target_times = {0, 1, 2, 3, 4, 5, 6, 7, /* 8, */ 20, 30, 60}; - plan(9 + target_times.size()); + plan(8 + target_times.size()); CommandLine cl; if (cl.getEnv()) { @@ -52,9 +68,9 @@ int main() { } ok(1, "Connected to backend server"); MYSQL_QUERY(backend_conn, "CREATE TABLE IF NOT EXISTS dummy_log_table (id INT PRIMARY KEY AUTO_INCREMENT, data LONGTEXT)"); -// MYSQL_QUERY(backend_conn, "INSERT INTO dummy_log_table (data) VALUES (REPEAT('a', 1024*50))"); -// MYSQL_QUERY(backend_conn, "INSERT INTO dummy_log_table (data) VALUES (REPEAT('a', 1024*50))"); -// MYSQL_QUERY(backend_conn, "INSERT INTO dummy_log_table (data) VALUES (REPEAT('a', 1024*50))"); + MYSQL_QUERY(backend_conn, "INSERT INTO dummy_log_table (data) VALUES (REPEAT('a', 1024*50))"); + MYSQL_QUERY(backend_conn, "INSERT INTO dummy_log_table (data) VALUES (REPEAT('a', 1024*50))"); + MYSQL_QUERY(backend_conn, "INSERT INTO dummy_log_table (data) VALUES (REPEAT('a', 1024*50))"); int rc = mysql_query(backend_conn, "FLUSH LOGS"); ok(rc == 0, "Generated data and flushed logs on backend"); @@ -69,8 +85,8 @@ int main() { rc = mysql_query(proxysql_admin, "UPDATE global_variables SET variable_value='8000' WHERE variable_name='mysql-fast_forward_grace_close_ms'"); ok(rc == 0, "Set mysql-fast_forward_grace_close_ms=8000"); - rc = mysql_query(proxysql_admin, "SET mysql-have_ssl=0"); - ok(rc == 0, "Set mysql-have_ssl=0"); +// rc = mysql_query(proxysql_admin, "SET mysql-have_ssl=0"); +// ok(rc == 0, "Set mysql-have_ssl=0"); rc = mysql_query(proxysql_admin, "LOAD MYSQL VARIABLES TO RUNTIME"); ok(rc == 0, "Loaded MYSQL variables to runtime");