Skip to content

Commit dfcd45c

Browse files
committed
client: redesign waiting methods
Currently, call of any waiting method (`wait`, `waitAll` and so on) with zero timeout blocks execution until all required responses are received. That's not great because in some scenarios users want to check if there are any responses ready without blocking execution. Of course, one can use `wait(1)` which sleeps for only a millisecond, but anyway it's often unexpected that `wait(0)` sleeps forever. Let's stick to `epoll_wait` interface - `wait(0)` is non-blocking and `wait(-1)` blocks forever. Note that before the commit, passing `-1` would lead to an assertion failure. Closes #111
1 parent f9dae01 commit dfcd45c

File tree

6 files changed

+72
-5
lines changed

6 files changed

+72
-5
lines changed

src/Client/EpollNetProvider.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,8 @@ template<class BUFFER, class Stream>
258258
int
259259
EpollNetProvider<BUFFER, Stream>::wait(int timeout)
260260
{
261-
assert(timeout >= 0);
262-
if (timeout == 0)
261+
assert(timeout >= -1);
262+
if (timeout == -1)
263263
timeout = TIMEOUT_INFINITY;
264264
LOG_DEBUG("Network engine wait for ", timeout, " milliseconds");
265265
/* Send pending requests. */

src/Client/LibevNetProvider.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ template<class BUFFER, class Stream>
363363
int
364364
LibevNetProvider<BUFFER, Stream>::wait(int timeout)
365365
{
366-
assert(timeout >= 0);
366+
assert(timeout >= -1);
367367
if (timeout > 0) {
368368
ev_timer_init(&m_TimeoutWatcher, &timeout_cb, timeout / MILLISECONDS, 0 /* repeat */);
369369
ev_timer_start(m_Loop, &m_TimeoutWatcher);

src/Utils/Timer.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class Timer {
4040
}
4141
bool isExpired() const
4242
{
43-
if (m_Timeout == std::chrono::milliseconds{0})
43+
if (m_Timeout == std::chrono::milliseconds{-1})
4444
return false;
4545
std::chrono::time_point<std::chrono::steady_clock> end =
4646
std::chrono::steady_clock::now();
@@ -50,7 +50,7 @@ class Timer {
5050
}
5151
int elapsed() const
5252
{
53-
if (m_Timeout == std::chrono::milliseconds{0})
53+
if (m_Timeout == std::chrono::milliseconds{-1})
5454
return 0;
5555
std::chrono::time_point<std::chrono::steady_clock> end =
5656
std::chrono::steady_clock::now();

test/ClientTest.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,60 @@ response_decoding(Connector<BUFFER, NetProvider> &client)
11371137
fail_unless(response->body.data->decode(mpp::as_arr(arr_of_num)));
11381138
fail_unless(std::get<0>(arr_of_num) == 666);
11391139

1140+
}
1141+
1142+
/** Checks all available `wait` methods of connector. */
1143+
void
1144+
test_wait(void)
1145+
{
1146+
TEST_INIT(0);
1147+
static constexpr double SLEEP_TIME = 0.1;
1148+
1149+
/* Use own client - the test must be isolated. */
1150+
Connector<Buf_t, NetProvider> client;
1151+
Connection<Buf_t, NetProvider> conn(client);
1152+
int rc = test_connect(client, conn, localhost, port);
1153+
fail_unless(rc == 0);
1154+
1155+
TEST_CASE("wait(0) and wait(-1)");
1156+
rid_t f = conn.call("remote_sleep", std::forward_as_tuple(SLEEP_TIME));
1157+
fail_unless(!conn.futureIsReady(f));
1158+
client.wait(conn, f, 0);
1159+
fail_unless(!conn.futureIsReady(f));
1160+
client.wait(conn, f, -1);
1161+
fail_unless(conn.futureIsReady(f));
1162+
std::optional<Response<Buf_t>> response = conn.getResponse(f);
1163+
fail_unless(response.has_value());
1164+
1165+
TEST_CASE("waitAny(0) and waitAny(-1)");
1166+
f = conn.call("remote_sleep", std::forward_as_tuple(SLEEP_TIME));
1167+
fail_unless(!client.waitAny(0).has_value());
1168+
fail_unless(client.waitAny(-1).has_value());
1169+
response = conn.getResponse(f);
1170+
fail_unless(response.has_value());
1171+
1172+
TEST_CASE("waitAll(0) and waitAll(-1)");
1173+
std::vector<rid_t> fs;
1174+
fs.push_back(conn.ping());
1175+
fs.push_back(conn.ping());
1176+
fail_unless(client.waitAll(conn, fs, 0) == -1);
1177+
fail_unless(client.waitAll(conn, fs, -1) == 0);
1178+
response = conn.getResponse(fs[0]);
1179+
fail_unless(response.has_value());
1180+
response = conn.getResponse(fs[1]);
1181+
fail_unless(response.has_value());
1182+
1183+
TEST_CASE("waitCount(0) and waitCount(-1)");
1184+
fs.clear();
1185+
fs.push_back(conn.ping());
1186+
fs.push_back(conn.ping());
1187+
fail_unless(client.waitCount(conn, 2, 0) == -1);
1188+
fail_unless(client.waitCount(conn, 2, -1) == 0);
1189+
response = conn.getResponse(fs[0]);
1190+
fail_unless(response.has_value());
1191+
response = conn.getResponse(fs[1]);
1192+
fail_unless(response.has_value());
1193+
11401194
client.close(conn);
11411195
}
11421196

@@ -1189,5 +1243,6 @@ int main()
11891243
#endif
11901244
::test_dead_connection_wait(client);
11911245
response_decoding(client);
1246+
test_wait();
11921247
return 0;
11931248
}

test/cfg.lua

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ function remote_echo(...)
3434
return {...}
3535
end
3636

37+
function remote_sleep(timeout)
38+
local fiber = require('fiber')
39+
fiber.sleep(timeout)
40+
return nil
41+
end
42+
3743
function get_rps()
3844
return box.stat.net().REQUESTS.rps
3945
end

test/cfg_ssl.lua

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ function remote_echo(...)
3232
return {...}
3333
end
3434

35+
function remote_sleep(timeout)
36+
local fiber = require('fiber')
37+
fiber.sleep(timeout)
38+
return nil
39+
end
40+
3541
function get_rps()
3642
return box.stat.net().REQUESTS.rps
3743
end

0 commit comments

Comments
 (0)