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.

The patch makes the client retry the failed (as described above) request
once over a fresh new connection, provided it was done over a connection
got from pool.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
  • Loading branch information
xemul committed Nov 21, 2023
1 parent 04716f1 commit 085d805
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
3 changes: 3 additions & 0 deletions include/seastar/http/client.hh
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ public:
* \param handle -- the response handler
* \param expected -- the expected reply status code
*
* 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, reply::status_type expected = reply::status_type::ok);

Expand Down
16 changes: 16 additions & 0 deletions src/http/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,22 @@ future<> client::make_request(request req, reply_handler handle, reply::status_t
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) {
return do_make_request(con, req, handle, expected);
}).handle_exception_type([this, &req, &handle, expected] (const transport_error& ex) {
if (ex.requests_submitted == 1) {
// It wasn't connection from pool, no need in making new one again
return make_exception_future<>(ex.cause);
}
// This creates one more connection without checking the limit, but
// since the old connection is closing it compensates the increase.
//
// However, in case new make_request() manages to creep in between
// it will temporarily create one extra connection, but it will not
// be held in pool anyway. Still, this is unlikely, _nr_connections
// decrease will happen in a waiting task and this place is chained
// directly after the put_connection()
return with_new_connection([this, &req, &handle, expected] (connection& con) {
return do_make_request(con, req, handle, expected);
});
});
});
}
Expand Down

0 comments on commit 085d805

Please sign in to comment.