Skip to content

Commit 71cbe2e

Browse files
qicosmosqicosmos
authored
[coro_rpc_client][fix]Add request timeout confg (#844)
1 parent e10d8c7 commit 71cbe2e

File tree

2 files changed

+116
-55
lines changed

2 files changed

+116
-55
lines changed

‎include/ylt/coro_rpc/impl/coro_rpc_client.hpp

+47-48
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,14 @@ class coro_rpc_client {
161161
"client has been closed"};
162162
struct config {
163163
uint64_t client_id = get_global_client_id();
164-
std::chrono::milliseconds timeout_duration =
165-
std::chrono::milliseconds{30000};
166-
std::string host;
167-
std::string port;
164+
std::optional<std::chrono::milliseconds> connect_timeout_duration;
165+
std::optional<std::chrono::milliseconds> request_timeout_duration;
166+
std::string host{};
167+
std::string port{};
168168
bool enable_tcp_no_delay = true;
169169
#ifdef YLT_ENABLE_SSL
170-
std::filesystem::path ssl_cert_path;
171-
std::string ssl_domain;
170+
std::filesystem::path ssl_cert_path{};
171+
std::string ssl_domain{};
172172
#endif
173173
};
174174

@@ -203,6 +203,8 @@ class coro_rpc_client {
203203

204204
const config &get_config() const { return config_; }
205205

206+
config &get_config() { return config_; }
207+
206208
[[nodiscard]] bool init_config(const config &conf) {
207209
config_ = conf;
208210
#ifdef YLT_ENABLE_SSL
@@ -228,57 +230,45 @@ class coro_rpc_client {
228230
*
229231
* @param host server address
230232
* @param port server port
231-
* @param timeout_duration RPC call timeout
233+
* @param connect_timeout_duration RPC call timeout seconds
232234
* @return error code
233235
*/
234236
[[nodiscard]] async_simple::coro::Lazy<coro_rpc::err_code> connect(
235237
std::string host, std::string port,
236-
std::chrono::steady_clock::duration timeout_duration =
238+
std::chrono::steady_clock::duration connect_timeout_duration =
237239
std::chrono::seconds(30)) {
238240
auto lock_ok = connect_mutex_.tryLock();
239241
if (!lock_ok) {
240242
co_await connect_mutex_.coScopedLock();
241243
co_return err_code{};
242244
// do nothing, someone has reconnect the client
243245
}
244-
config_.host = std::move(host);
245-
config_.port = std::move(port);
246-
config_.timeout_duration =
247-
std::chrono::duration_cast<std::chrono::milliseconds>(timeout_duration);
246+
247+
if (config_.host.empty()) {
248+
config_.host = std::move(host);
249+
}
250+
if (config_.port.empty()) {
251+
config_.port = std::move(port);
252+
}
253+
if (!config_.connect_timeout_duration) {
254+
config_.connect_timeout_duration =
255+
std::chrono::duration_cast<std::chrono::milliseconds>(
256+
connect_timeout_duration);
257+
}
258+
248259
auto ret = co_await connect_impl();
249260
connect_mutex_.unlock();
250261
co_return std::move(ret);
251262
}
252263
[[nodiscard]] async_simple::coro::Lazy<coro_rpc::err_code> connect(
253264
std::string_view endpoint,
254-
std::chrono::steady_clock::duration timeout_duration =
265+
std::chrono::steady_clock::duration connect_timeout_duration =
255266
std::chrono::seconds(30)) {
256267
auto pos = endpoint.find(':');
257-
auto lock_ok = connect_mutex_.tryLock();
258-
if (!lock_ok) {
259-
co_await connect_mutex_.coScopedLock();
260-
co_return err_code{};
261-
// do nothing, someone has reconnect the client
262-
}
263-
config_.host = endpoint.substr(0, pos);
264-
config_.port = endpoint.substr(pos + 1);
265-
config_.timeout_duration =
266-
std::chrono::duration_cast<std::chrono::milliseconds>(timeout_duration);
267-
auto ret = co_await connect_impl();
268-
connect_mutex_.unlock();
269-
co_return std::move(ret);
270-
}
268+
std::string host(endpoint.substr(0, pos));
269+
std::string port(endpoint.substr(pos + 1));
271270

272-
[[nodiscard]] async_simple::coro::Lazy<coro_rpc::err_code> connect() {
273-
auto lock_ok = connect_mutex_.tryLock();
274-
if (!lock_ok) {
275-
co_await connect_mutex_.coScopedLock();
276-
co_return err_code{};
277-
// do nothing, someone has reconnect the client
278-
}
279-
auto ret = co_await connect_impl();
280-
connect_mutex_.unlock();
281-
co_return std::move(ret);
271+
return connect(std::move(host), std::move(port), connect_timeout_duration);
282272
}
283273

284274
#ifdef YLT_ENABLE_SSL
@@ -323,11 +313,12 @@ class coro_rpc_client {
323313
*/
324314
template <auto func, typename... Args>
325315
async_simple::coro::Lazy<rpc_result<decltype(get_return_type<func>())>>
326-
call_for(auto duration, Args &&...args) {
316+
call_for(auto request_timeout_duration, Args &&...args) {
327317
using return_type = decltype(get_return_type<func>());
328318
auto async_result =
329319
co_await co_await send_request_for_with_attachment<func, Args...>(
330-
duration, req_attachment_, std::forward<Args>(args)...);
320+
request_timeout_duration, req_attachment_,
321+
std::forward<Args>(args)...);
331322
req_attachment_ = {};
332323
if (async_result) {
333324
control_->resp_buffer_ = async_result->release_buffer();
@@ -410,9 +401,12 @@ class coro_rpc_client {
410401

411402
ELOGV(INFO, "client_id %d begin to connect %s", config_.client_id,
412403
config_.port.data());
413-
timeout(*this->timer_, config_.timeout_duration, "connect timer canceled")
414-
.start([](auto &&) {
415-
});
404+
auto conn_timeout_dur = *config_.connect_timeout_duration;
405+
if (conn_timeout_dur.count() >= 0) {
406+
timeout(*this->timer_, conn_timeout_dur, "connect timer canceled")
407+
.start([](auto &&) {
408+
});
409+
}
416410

417411
std::error_code ec = co_await coro_io::async_connect(
418412
&control_->executor_, control_->socket_, config_.host, config_.port);
@@ -747,7 +741,7 @@ class coro_rpc_client {
747741
private:
748742
template <auto func, typename... Args>
749743
async_simple::coro::Lazy<rpc_error> send_request_for_impl(
750-
auto duration, uint32_t &id, coro_io::period_timer &timer,
744+
auto request_timeout_duration, uint32_t &id, coro_io::period_timer &timer,
751745
std::string_view attachment, Args &&...args) {
752746
using R = decltype(get_return_type<func>());
753747

@@ -766,9 +760,10 @@ class coro_rpc_client {
766760

767761
static_check<func, Args...>();
768762

769-
if (duration.count() > 0) {
770-
timeout(timer, duration, "rpc call timer canceled").start([](auto &&) {
771-
});
763+
if (request_timeout_duration.count() >= 0) {
764+
timeout(timer, request_timeout_duration, "rpc call timer canceled")
765+
.start([](auto &&) {
766+
});
772767
}
773768

774769
#ifdef YLT_ENABLE_SSL
@@ -965,16 +960,20 @@ class coro_rpc_client {
965960
template <auto func, typename... Args>
966961
async_simple::coro::Lazy<async_simple::coro::Lazy<
967962
async_rpc_result<decltype(get_return_type<func>())>>>
968-
send_request_for_with_attachment(auto time_out_duration,
963+
send_request_for_with_attachment(auto request_timeout_duration,
969964
std::string_view request_attachment,
970965
Args &&...args) {
971966
using rpc_return_t = decltype(get_return_type<func>());
972967
recving_guard guard(control_.get());
973968
uint32_t id;
969+
if (!config_.request_timeout_duration) {
970+
config_.request_timeout_duration = request_timeout_duration;
971+
}
972+
974973
auto timer = std::make_unique<coro_io::period_timer>(
975974
control_->executor_.get_asio_executor());
976975
auto result = co_await send_request_for_impl<func>(
977-
time_out_duration, id, *timer, request_attachment,
976+
*config_.request_timeout_duration, id, *timer, request_attachment,
978977
std::forward<Args>(args)...);
979978
auto &control = *control_;
980979
if (!result) {

‎src/coro_rpc/tests/test_coro_rpc_client.cpp

+69-7
Original file line numberDiff line numberDiff line change
@@ -480,14 +480,76 @@ TEST_CASE("testing client with shutdown") {
480480
g_action = {};
481481
}
482482
TEST_CASE("testing client timeout") {
483-
// SUBCASE("connect, 0ms timeout") {
484-
// coro_rpc_client client;
485-
// auto ret = client.connect("127.0.0.1", "8801", 0ms);
486-
// auto val = syncAwait(ret);
483+
coro_rpc_server server(2, 8801);
484+
server.register_handler<hello, client_hello>();
485+
auto res = server.async_start();
486+
CHECK_MESSAGE(!res.hasResult(), "server start timeout");
487487

488-
// CHECK_MESSAGE(val == std::errc::timed_out,
489-
// make_error_code(val).message());
490-
// }
488+
SUBCASE("connect, 0ms connect timeout") {
489+
coro_rpc_client client;
490+
coro_rpc_client::config config{};
491+
config.connect_timeout_duration = 0ms;
492+
bool r = client.init_config(config);
493+
CHECK(r);
494+
auto ret = client.connect(
495+
"127.0.0.1", "8801",
496+
1000ms); // this arg won't update config connect timeout duration.
497+
auto val = syncAwait(ret);
498+
499+
if (val) {
500+
CHECK_MESSAGE(val == coro_rpc::errc::timed_out, val.message());
501+
}
502+
}
503+
SUBCASE("connect, -1ms never timeout") {
504+
coro_rpc_client client;
505+
coro_rpc_client::config config{};
506+
config.connect_timeout_duration = -1ms; // less than 0, no timeout
507+
// checking.
508+
bool r = client.init_config(config);
509+
CHECK(r);
510+
auto ret = client.connect(
511+
"127.0.0.1", "8801",
512+
1000ms); // this arg won't update config connect timeout duration.
513+
auto val = syncAwait(ret);
514+
515+
CHECK(!val);
516+
}
517+
518+
SUBCASE("connect, 0ms request timeout") {
519+
coro_rpc_client client;
520+
coro_rpc_client::config config{};
521+
config.connect_timeout_duration = 1000ms;
522+
config.request_timeout_duration = 0ms;
523+
bool r = client.init_config(config);
524+
CHECK(r);
525+
auto ret = client.connect(
526+
"127.0.0.1", "8801",
527+
0ms); // 0ms won't cover config connect timeout duration.
528+
auto val = syncAwait(ret);
529+
530+
CHECK(!val);
531+
auto result = syncAwait(client.call<hello>());
532+
533+
if (result.has_value()) {
534+
std::cout << result.value() << std::endl;
535+
}
536+
else {
537+
CHECK_MESSAGE(result.error().code == coro_rpc::errc::timed_out,
538+
result.error().msg);
539+
}
540+
}
541+
SUBCASE("connect, -1ms never request timeout") {
542+
coro_rpc_client client;
543+
client.get_config().request_timeout_duration =
544+
-1ms; // less than 0, never timeout.
545+
auto ret = client.connect("127.0.0.1", "8801");
546+
auto val = syncAwait(ret);
547+
548+
CHECK(!val);
549+
auto result = syncAwait(client.call<hello>());
550+
551+
CHECK(result.has_value());
552+
}
491553
SUBCASE("connect, ip timeout") {
492554
g_action = {};
493555
// https://stackoverflow.com/questions/100841/artificially-create-a-connection-timeout-error

0 commit comments

Comments
 (0)