From f984ed1b9689fb227ac63160880055bf8f74909b Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 09:52:39 -0700 Subject: [PATCH 1/9] avoid crashing when requests per interval goes to 0 --- shard_connection.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shard_connection.cpp b/shard_connection.cpp index e873308..160ef6c 100644 --- a/shard_connection.cpp +++ b/shard_connection.cpp @@ -364,7 +364,10 @@ void shard_connection::push_req(request* req) { m_pipeline->push(req); m_pending_resp++; if (m_config->request_rate) { - assert(m_request_per_cur_interval > 0); + if (m_request_per_cur_interval == 0) { + // not ideal for accuracy but will prevent crashes in very low-rate cases + return; + } m_request_per_cur_interval--; } } From 25ac503c6fc1b3c9dbfd83a325a59ee7966e1326 Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 10:25:53 -0700 Subject: [PATCH 2/9] AI suggested fix seems more complete than mine --- shard_connection.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/shard_connection.cpp b/shard_connection.cpp index 160ef6c..cd311ad 100644 --- a/shard_connection.cpp +++ b/shard_connection.cpp @@ -364,11 +364,9 @@ void shard_connection::push_req(request* req) { m_pipeline->push(req); m_pending_resp++; if (m_config->request_rate) { - if (m_request_per_cur_interval == 0) { - // not ideal for accuracy but will prevent crashes in very low-rate cases - return; + if (m_request_per_cur_interval > 0) { + m_request_per_cur_interval--; } - m_request_per_cur_interval--; } } @@ -535,7 +533,7 @@ void shard_connection::fill_pipeline(void) // that's enough, we reached the rate limit if (m_config->request_rate && m_request_per_cur_interval == 0) { - // return and skip on update events + // Keep the connection enabled but don't send more requests this interval return; } @@ -549,9 +547,7 @@ void shard_connection::fill_pipeline(void) if ((m_pending_resp == 0) && (evbuffer_get_length(bufferevent_get_output(m_bev)) == 0)) { benchmark_debug_log("%s Done, no requests to send no response to wait for\n", get_readable_id()); bufferevent_disable(m_bev, EV_WRITE|EV_READ); - if (m_config->request_rate) { - event_del(m_event_timer); - } + // Don't delete the timer event - we need it for the next interval } } } From ebe105c535a3d13492f59e3510ae25ecabc014d5 Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 10:46:46 -0700 Subject: [PATCH 3/9] more comprehensive fix --- shard_connection.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/shard_connection.cpp b/shard_connection.cpp index cd311ad..0ce0dcf 100644 --- a/shard_connection.cpp +++ b/shard_connection.cpp @@ -454,6 +454,11 @@ void shard_connection::process_response(void) m_cluster_slots = setup_done; benchmark_debug_log("cluster slot command successful\n"); + + // Re-enable the connection after cluster slots update + if (m_bev != NULL && m_connection_state == conn_connected) { + bufferevent_enable(m_bev, EV_READ|EV_WRITE); + } } break; case rt_hello: @@ -546,8 +551,10 @@ void shard_connection::fill_pipeline(void) // no pending response (nothing to read) and output buffer empty (nothing to write) if ((m_pending_resp == 0) && (evbuffer_get_length(bufferevent_get_output(m_bev)) == 0)) { benchmark_debug_log("%s Done, no requests to send no response to wait for\n", get_readable_id()); - bufferevent_disable(m_bev, EV_WRITE|EV_READ); - // Don't delete the timer event - we need it for the next interval + // Only disable the connection if we're not waiting for cluster slots + if (m_cluster_slots == setup_done) { + bufferevent_disable(m_bev, EV_WRITE|EV_READ); + } } } } @@ -608,6 +615,12 @@ void shard_connection::handle_event(short events) void shard_connection::handle_timer_event() { m_request_per_cur_interval = m_config->request_per_interval; + + // Re-enable the connection if it was disabled due to rate limiting + if (m_bev != NULL && m_connection_state == conn_connected) { + bufferevent_enable(m_bev, EV_READ|EV_WRITE); + } + fill_pipeline(); } From d381eec2303331135a96a9784d5ddd0a7d815c21 Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 10:53:48 -0700 Subject: [PATCH 4/9] revert last update --- shard_connection.cpp | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/shard_connection.cpp b/shard_connection.cpp index 0ce0dcf..cd311ad 100644 --- a/shard_connection.cpp +++ b/shard_connection.cpp @@ -454,11 +454,6 @@ void shard_connection::process_response(void) m_cluster_slots = setup_done; benchmark_debug_log("cluster slot command successful\n"); - - // Re-enable the connection after cluster slots update - if (m_bev != NULL && m_connection_state == conn_connected) { - bufferevent_enable(m_bev, EV_READ|EV_WRITE); - } } break; case rt_hello: @@ -551,10 +546,8 @@ void shard_connection::fill_pipeline(void) // no pending response (nothing to read) and output buffer empty (nothing to write) if ((m_pending_resp == 0) && (evbuffer_get_length(bufferevent_get_output(m_bev)) == 0)) { benchmark_debug_log("%s Done, no requests to send no response to wait for\n", get_readable_id()); - // Only disable the connection if we're not waiting for cluster slots - if (m_cluster_slots == setup_done) { - bufferevent_disable(m_bev, EV_WRITE|EV_READ); - } + bufferevent_disable(m_bev, EV_WRITE|EV_READ); + // Don't delete the timer event - we need it for the next interval } } } @@ -615,12 +608,6 @@ void shard_connection::handle_event(short events) void shard_connection::handle_timer_event() { m_request_per_cur_interval = m_config->request_per_interval; - - // Re-enable the connection if it was disabled due to rate limiting - if (m_bev != NULL && m_connection_state == conn_connected) { - bufferevent_enable(m_bev, EV_READ|EV_WRITE); - } - fill_pipeline(); } From 4f6b057f35bfddc8b6c52e3d15e99495d77b0bb2 Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 11:39:05 -0700 Subject: [PATCH 5/9] fix end of the run stall with rate-limiting --- shard_connection.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/shard_connection.cpp b/shard_connection.cpp index cd311ad..2991e6b 100644 --- a/shard_connection.cpp +++ b/shard_connection.cpp @@ -547,7 +547,10 @@ void shard_connection::fill_pipeline(void) if ((m_pending_resp == 0) && (evbuffer_get_length(bufferevent_get_output(m_bev)) == 0)) { benchmark_debug_log("%s Done, no requests to send no response to wait for\n", get_readable_id()); bufferevent_disable(m_bev, EV_WRITE|EV_READ); - // Don't delete the timer event - we need it for the next interval + if (m_config->request_rate && m_conns_manager->finished()) { + // Only delete the timer when we're actually done with the benchmark + event_del(m_event_timer); + } } } } @@ -607,6 +610,11 @@ void shard_connection::handle_event(short events) } void shard_connection::handle_timer_event() { + if (m_conns_manager->finished()) { + // If we're done with the benchmark, stop the timer + event_del(m_event_timer); + return; + } m_request_per_cur_interval = m_config->request_per_interval; fill_pipeline(); } From 1f85029130d496df4d9c50ea16d39298a4e6a704 Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 11:43:21 -0700 Subject: [PATCH 6/9] check finished --- shard_connection.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shard_connection.cpp b/shard_connection.cpp index 2991e6b..66faf04 100644 --- a/shard_connection.cpp +++ b/shard_connection.cpp @@ -547,8 +547,8 @@ void shard_connection::fill_pipeline(void) if ((m_pending_resp == 0) && (evbuffer_get_length(bufferevent_get_output(m_bev)) == 0)) { benchmark_debug_log("%s Done, no requests to send no response to wait for\n", get_readable_id()); bufferevent_disable(m_bev, EV_WRITE|EV_READ); - if (m_config->request_rate && m_conns_manager->finished()) { - // Only delete the timer when we're actually done with the benchmark + if (m_conns_manager->finished()) { + // If we're done with the benchmark, stop the timer event_del(m_event_timer); } } From fb8e7efb2589f854549c3eed6fb36df14b50ffae Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 11:46:05 -0700 Subject: [PATCH 7/9] break loop --- client.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client.cpp b/client.cpp index a088d47..f513d06 100755 --- a/client.cpp +++ b/client.cpp @@ -229,6 +229,11 @@ void client::set_end_time() { m_stats.set_end_time(NULL); m_end_set = true; + + // Break out of the event loop when we're done + if (m_event_base != NULL) { + event_base_loopbreak(m_event_base); + } } } From c30331af71156afd9b062884bb54b4ae0b47c390 Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 11:50:04 -0700 Subject: [PATCH 8/9] make sure stats are collected --- client.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client.cpp b/client.cpp index f513d06..56e59e6 100755 --- a/client.cpp +++ b/client.cpp @@ -227,12 +227,17 @@ void client::set_end_time() { if (!m_end_set) { benchmark_debug_log("nothing else to do, test is finished.\n"); + // First update the stats m_stats.set_end_time(NULL); m_end_set = true; - // Break out of the event loop when we're done + // Then break out of the event loop if (m_event_base != NULL) { - event_base_loopbreak(m_event_base); + // Give a small delay to ensure stats are collected + struct timeval tv; + tv.tv_sec = 0; + tv.tv_usec = 100000; // 100ms delay + event_base_loopexit(m_event_base, &tv); } } } From 96dba0ebb6c4a5ff34dd8ea333fc6c53d3a03f4f Mon Sep 17 00:00:00 2001 From: Kurt Moeller Date: Thu, 15 May 2025 11:56:33 -0700 Subject: [PATCH 9/9] clean connections - so stats will be properly collected --- shard_connection.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/shard_connection.cpp b/shard_connection.cpp index 66faf04..f52ab0e 100644 --- a/shard_connection.cpp +++ b/shard_connection.cpp @@ -546,10 +546,13 @@ void shard_connection::fill_pipeline(void) // no pending response (nothing to read) and output buffer empty (nothing to write) if ((m_pending_resp == 0) && (evbuffer_get_length(bufferevent_get_output(m_bev)) == 0)) { benchmark_debug_log("%s Done, no requests to send no response to wait for\n", get_readable_id()); - bufferevent_disable(m_bev, EV_WRITE|EV_READ); - if (m_conns_manager->finished()) { - // If we're done with the benchmark, stop the timer - event_del(m_event_timer); + // Only disable the connection if we're not in the process of receiving responses + if (evbuffer_get_length(bufferevent_get_input(m_bev)) == 0) { + bufferevent_disable(m_bev, EV_WRITE|EV_READ); + if (m_conns_manager->finished()) { + // If we're done with the benchmark, stop the timer + event_del(m_event_timer); + } } } }