[k2] rpc client boost#1635
Open
T-y-c-o-o-n wants to merge 20 commits into
Open
Conversation
T-y-c-o-o-n
force-pushed
the
n.siniachenko/k2/rpc-client-boost
branch
from
156e9ec to
222adae
Compare
T-y-c-o-o-n
commented
Jun 17, 2026
| // prepare and send RPC request | ||
| // 'request_buf' will look like this: | ||
| // [ RpcExtraHeaders (optional) ] [ payload ] | ||
| if (const auto& [opt_new_extra_header, cur_extra_header_size]{kphp::rpc::regularize_extra_headers(tl_storer.view(), ignore_answer)}; opt_new_extra_header) { |
Contributor
Author
There was a problem hiding this comment.
В новом коде нет вызова regularize_extra_headers(...)
| const auto it_awaiter_task{rpc_client_instance_st.response_awaiter_tasks.find(request_id)}; | ||
| if (it_awaiter_task == rpc_client_instance_st.response_awaiter_tasks.end()) [[unlikely]] { | ||
| kphp::log::warning("could not find rpc query with id {} in pending queries", queue_id); | ||
| const auto it_rpc_request_info{rpc_client_instance_st.rpc_requests_infos.find(request_id)}; |
Contributor
Author
There was a problem hiding this comment.
кажется, это не будет работать для ignore_answer запросов (если для них, вообще, вызывается rpc_queue_push)
apolyakov
requested changes
Jun 18, 2026
| if (!first_response_size) { | ||
| return std::unexpected{std::make_pair(TL_ERROR_INTERNAL, string{"error fetching rpc response"})}; | ||
| } | ||
| string response{reinterpret_cast<char*>(k2::alloc(*first_response_size)), static_cast<string::size_type>(*first_response_size)}; |
Contributor
There was a problem hiding this comment.
Looks like we can have a single buffer for RPC response to reuse
| std::chrono::nanoseconds now_ns{now_instant.time_point_ns}; | ||
| std::chrono::nanoseconds timeout{rpc_request_info.deadline - now_ns}; | ||
| kphp::coro::io_scheduler& m_scheduler{kphp::coro::io_scheduler::get()}; | ||
| switch (co_await m_scheduler.poll(rpc_request_info.rpc_d, kphp::coro::poll_op::read, timeout)) { |
Contributor
There was a problem hiding this comment.
It's quite hard to understand what's going on here. Can you please make it more human readable? For example, I think we don't need to directly use the scheduler's API here
Contributor
Author
There was a problem hiding this comment.
Tried to refactor and improve code readability:
- k2-rpc-api logic refactored to separate
kphp::rpc::query_handleclass - time juggling moved to util functions
| k2::TimePoint now_instant{}; | ||
| k2::instant(std::addressof(now_instant)); | ||
| std::chrono::nanoseconds now_ns{now_instant.time_point_ns}; | ||
| std::chrono::nanoseconds timeout{rpc_request_info.deadline - now_ns}; |
Contributor
There was a problem hiding this comment.
What will happen if nobody will fetch response? I'm afraid we might hang in this case
added 17 commits
June 21, 2026 22:16
…GAIN` errno if response is not ready
T-y-c-o-o-n
force-pushed
the
n.siniachenko/k2/rpc-client-boost
branch
from
a342aa9 to
e77636c
Compare
apolyakov
reviewed
Jun 29, 2026
| * `EAI_MEMORY` => max descriptors count achieved | ||
| * `EINVAL` => invalid `actor_name` or request, or connection pool is empty for this actor. | ||
| */ | ||
| int32_t k2_rpc_send(const char* actor_name, size_t actor_name_len, const void* request_ptr, size_t request_size, enum RpcKind rpc_kind, uint64_t* rpc_d); |
Contributor
There was a problem hiding this comment.
Let's do either:
- rename
k2_rpc_send->k2_rpc_send_request - rename
k2_rpc_fetch_response->k2_rpc_fetch,k2_rpc_get_response_size->k2_rpc_response_size
| return k2_component_access(component_name.size(), component_name.data()); | ||
| } | ||
|
|
||
| inline std::expected<uint64_t, int32_t> rpc_send_request(std::string_view actor_name, std::span<const std::byte> request_buffer) noexcept { |
Contributor
There was a problem hiding this comment.
- we have
k2::descriptortype that should be used here instead ofuint64_t - shouldn't we add
rpc_kindparameter?
Contributor
There was a problem hiding this comment.
Let's put these functions to kphp::time namespace and name them like kphp::time::remaining and kphp::time::expires_at.
P.S. we try to not introduce util as it leads to a bunch of disparate functionality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.