[k2] rpc client boost by T-y-c-o-o-n · Pull Request #1635 · VKCOM/kphp

T-y-c-o-o-n · 2026-06-15T10:37:24Z

No description provided.

T-y-c-o-o-n · 2026-06-17T16:41:42Z

-    // 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) {


В новом коде нет вызова regularize_extra_headers(...)

T-y-c-o-o-n · 2026-06-17T16:44:59Z

-  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)};


кажется, это не будет работать для ignore_answer запросов (если для них, вообще, вызывается rpc_queue_push)

apolyakov · 2026-06-18T10:32:05Z

 }

-/** @kphp-extern-func-info interruptible */
+/** @kphp-extern-func-info */


We can completely remove this now

apolyakov · 2026-06-18T10:33:07Z


+/**
+ * Try to send rpc request. If case of success write descriptor of rpc request to `rpc_d`, otherwise return `errno` != 0.
+ * which should be later used to call `k2_rpc_fetch_response`.


nit: fix grammar

apolyakov · 2026-06-18T10:35:07Z

+ * `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_request(const char* actor_name, size_t actor_name_len, const void* request_ptr, size_t request_size, uint64_t* rpc_d);


Maybe rename to k2_rpc_send?

Do we really need actor_name? Can't we just use something like int64_t actor_id?

Shoudn't we consider multiple RPC implementations here, e.g. TL RPC, gRPC etc?

apolyakov · 2026-06-18T10:36:39Z

+
+/**
+ * Get response size for corresponding request of this `rpc_d`. Write 0 to `response_size` if response is not ready.
+ * Write response size value to `response_size` when response is ready.


when response is ready => if response is ready

apolyakov · 2026-06-18T10:37:40Z

+ *
+ * @return return `0` on success. libc-like `errno` otherwise
+ *
+ * `EINVAL` => invalid `rpc_d` descriptor, for example, it is unknown descriptor, or not rpc descriptor.


Let's add something like * Possible errno values:

apolyakov · 2026-06-18T10:38:58Z

+int32_t k2_rpc_get_response_size(uint64_t rpc_d, size_t* response_size);
+
+/**
+ * Write response for corresponding request of this `rpc_d` to `buf`. Does nothing if response is not ready.


IMO it's a bad design choice to have this function do nothing in case a response is not ready yet. We need to indicate this case explicitly

apolyakov · 2026-06-18T10:42:10Z


+namespace {
+
+static std::expected<string, std::pair<int, string>> get_rpc_response(int64_t query_id, k2::descriptor rpc_d, bool collect_responses_extra_info) {


We use sized integral types in runtime-light

We definitely need an RAII-like type for the RPC descriptor to prevent leaks that cause the instance to hang.

No need for static in anonymous namespace inside a cpp file

apolyakov · 2026-06-18T10:44:13Z

+  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)};


Looks like we can have a single buffer for RPC response to reuse

apolyakov · 2026-06-18T10:48:24Z

+  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)) {


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

apolyakov · 2026-06-18T10:49:49Z

+  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};


What will happen if nobody will fetch response? I'm afraid we might hang in this case

Nikita Siniachenko added 9 commits April 29, 2026 17:04

first impl

c3f9787

added kphp rpc client bench

14a08be

break in switch + different debug exit codes

b0d876a

null timeout in bench

257c0d3

rpc send functions now common functions instead of coroutines

beb7bb0

remove one coroutine on rpc response fetch

67e44cf

removed test trash

b9e7a73

error handling

12bdbbb

rpc_queue_push

222adae

T-y-c-o-o-n force-pushed the n.siniachenko/k2/rpc-client-boost branch from 156e9ec to 222adae Compare June 15, 2026 11:57

Nikita Siniachenko added 2 commits June 15, 2026 16:42

kphp::rpc::request_info

401624f

fmt

1d8072a

T-y-c-o-o-n commented Jun 17, 2026

View reviewed changes

apolyakov requested changes Jun 18, 2026

View reviewed changes

Nikita Siniachenko added 4 commits June 18, 2026 22:53

added RpcKind + better docs

3dbcdad

query_handle

c47e90c

send_and_get_handle

d07e265

refactored query_handle a bit

e1a60df

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[k2] rpc client boost#1635

[k2] rpc client boost#1635
T-y-c-o-o-n wants to merge 15 commits into
masterfrom
n.siniachenko/k2/rpc-client-boost

T-y-c-o-o-n commented Jun 15, 2026

Uh oh!

T-y-c-o-o-n Jun 17, 2026

Uh oh!

T-y-c-o-o-n Jun 17, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026 •

edited

Loading

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

apolyakov Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		namespace {

		static std::expected<string, std::pair<int, string>> get_rpc_response(int64_t query_id, k2::descriptor rpc_d, bool collect_responses_extra_info) {

Conversation

T-y-c-o-o-n commented Jun 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apolyakov Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

apolyakov Jun 18, 2026 •

edited

Loading