Skip to content

Commit

Permalink
http/client: Retry request over fresh connection in case old one failed
Browse files Browse the repository at this point in the history
Currently if client::make_request() fails, the error is propagated back
to the caller whatever the error is. Sometimes the request fails because
of connection glitch, which can happen and in that case it's common to
try the same request again after a while in a hope that it was indeed
some teporary glitch.

Signed-off-by: Pavel Emelyanov <[email protected]>
  • Loading branch information
xemul committed Jun 4, 2024
1 parent 5cc634b commit 1c20f8d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
7 changes: 6 additions & 1 deletion include/seastar/http/client.hh
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ public:
class client {
public:
using reply_handler = noncopyable_function<future<>(const reply&, input_stream<char>&& body)>;
using retry_requests = bool_class<struct retry_requests_tag>;

private:
friend class http::internal::client_ref;
Expand All @@ -178,6 +179,7 @@ private:
unsigned _nr_connections = 0;
unsigned _max_connections;
unsigned long _total_new_connections = 0;
const retry_requests _retry;
condition_variable _wait_con;
connections_list_t _pool;

Expand Down Expand Up @@ -232,7 +234,7 @@ public:
* \param f -- the factory pointer
*
*/
explicit client(std::unique_ptr<connection_factory> f, unsigned max_connections = default_max_connections);
explicit client(std::unique_ptr<connection_factory> f, unsigned max_connections = default_max_connections, retry_requests retry = retry_requests::no);

/**
* \brief Send the request and handle the response
Expand All @@ -246,6 +248,9 @@ public:
* \param handle -- the response handler
* \param expected -- the optional expected reply status code, default is std::nullopt
*
* Note that the handle callback should be prepared to be called more than once, because
* client may restart the whole request processing in case server closes the connection
* in the middle of operation
*/
future<> make_request(request req, reply_handler handle, std::optional<reply::status_type> expected = std::nullopt);

Expand Down
23 changes: 21 additions & 2 deletions src/http/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,10 @@ client::client(socket_address addr, shared_ptr<tls::certificate_credentials> cre
{
}

client::client(std::unique_ptr<connection_factory> f, unsigned max_connections)
client::client(std::unique_ptr<connection_factory> f, unsigned max_connections, retry_requests retry)
: _new_connections(std::move(f))
, _max_connections(max_connections)
, _retry(retry)
{
}

Expand Down Expand Up @@ -330,9 +331,27 @@ auto client::with_new_connection(Fn&& fn) {

future<> client::make_request(request req, reply_handler handle, std::optional<reply::status_type> expected) {
return do_with(std::move(req), std::move(handle), [this, expected] (request& req, reply_handler& handle) mutable {
return with_connection([this, &req, &handle, expected] (connection& con) {
auto f = with_connection([this, &req, &handle, expected] (connection& con) {
return do_make_request(con, req, handle, expected);
});

if (_retry) {
f = f.handle_exception_type([this, &req, &handle, expected] (const std::system_error& ex) {
auto code = ex.code().value();
if ((code != EPIPE) && (code != ECONNABORTED)) {
return make_exception_future<>(ex);
}

// The 'con' connection may not yet be freed, so the total connection
// count still account for it and with_new_connection() may temporarily
// break the limit. That's OK, the 'con' will be closed really soon
return with_new_connection([this, &req, &handle, expected] (connection& con) {
return do_make_request(con, req, handle, expected);
});
});
}

return f;
});
}

Expand Down

0 comments on commit 1c20f8d

Please sign in to comment.