Skip to content

Support pipelined forwarding of client requests to leader#98

Merged
al13n321 merged 3 commits into
masterfrom
stream
May 6, 2026
Merged

Support pipelined forwarding of client requests to leader#98
al13n321 merged 3 commits into
masterfrom
stream

Conversation

@al13n321
Copy link
Copy Markdown
Member

@al13n321 al13n321 commented Mar 31, 2026

Used by ClickHouse/ClickHouse#101757

Most of the code is from #104 by @filimonov

@al13n321 al13n321 marked this pull request as ready for review April 7, 2026 22:49
@al13n321 al13n321 requested a review from antonio2368 April 8, 2026 04:04
@antonio2368 antonio2368 self-assigned this Apr 8, 2026
Comment thread src/asio_service.cxx Outdated
Comment thread src/asio_service.cxx Outdated
Copy link
Copy Markdown

@filimonov filimonov left a comment

Choose a reason for hiding this comment

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

👍

Comment thread src/handle_client_request.cxx Outdated
Comment thread src/handle_user_cmd.cxx Outdated
Comment thread src/handle_user_cmd.cxx Outdated
Comment thread src/client_req_stream.cxx
|| !resp->get_accepted()
|| resp->get_result_code() != cmd_result_code::OK;
if (non_ok) state->abandoned_.store(true);
size_t prev_in_flight = state->in_flight_.fetch_sub(1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[[maybe_unused]]?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

IIRC nuraft uses (void) instead of [[maybe_unused]], probably it targets an older C++ version that doesn't have it.

Comment thread include/libnuraft/client_req_stream.hxx Outdated
Comment thread include/libnuraft/client_req_stream.hxx Outdated
Comment thread include/libnuraft/client_req_stream.hxx
Comment thread src/client_req_stream.cxx

void client_req_stream::append(std::vector< ptr<buffer> > logs)
{
ptr<req_msg> req = cs_new<req_msg>(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if logs are empty do we need to call send?
I guess it should happen from the callers side but just in case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't call it with empty logs, so we're free to pick any reasonable behavior for this case here. Going through the normal code path seems good: it's less code and seems more likely to be useful to hypothetical imaginary future users (e.g. for some kind of heartbeats). Or do you mean it currently breaks because the server rejects such message or something?

@al13n321 al13n321 merged commit fd87197 into master May 6, 2026
1 check passed
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.

3 participants