Skip to content

Conversation

@luke36
Copy link

@luke36 luke36 commented Aug 28, 2024

In cluster mode, a client can open several connections. If one of them handles a response before the connection of another, m_reqs_processed is positive at line 556 so the timed event is not created. Since the bufferevent is already enabled, the process would hang forever. This might fix #94, might not. This is a quick fix. There are definitely better ways to repair it.


Note

Medium Risk
Touches connection-establishment and request-rate timer setup logic; small but can affect cluster-mode pacing and reconnection behavior if the timer is mismanaged.

Overview
Prevents a hang when running in cluster mode with --rate enabled by making rate-limit timer creation depend on whether the connection’s m_event_timer is set, rather than m_conns_manager->get_reqs_processed().

Initializes m_event_timer to NULL in shard_connection so each new shard connection reliably sets up its own timer and starts sending the first request on connect.

Written by Cursor Bugbot for commit 058d904. This will update automatically on new commits. Configure here.

@fcostaoliveira fcostaoliveira self-requested a review November 17, 2025 10:01
@fcostaoliveira fcostaoliveira self-assigned this Jan 28, 2026
@fcostaoliveira
Copy link
Collaborator

cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

bufferevent_enable(m_bev, EV_READ|EV_WRITE);

if (!m_conns_manager->get_reqs_processed()) {
if (m_event_timer == NULL) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconnection calls wrong function when rate limiting disabled

Medium Severity

When request_rate is 0 (rate limiting disabled), m_event_timer is never assigned, so the condition m_event_timer == NULL remains true even on reconnection. This causes process_first_request() to be called instead of fill_pipeline(), incorrectly resetting the benchmark start time via set_start_time(). The original condition !get_reqs_processed() correctly distinguished first connection from reconnection regardless of rate limiting configuration.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

memtier_benchmark doest not exit as expected

2 participants