From e9c9838cdc86d4e141435a2a8620c0d955cdf818 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 11 Jan 2019 23:26:08 +0900 Subject: [PATCH] nghttpx: Pool h1 backend connection per address Pool HTTP/1.1 backend connection per address and reuse it only when the next round robin index refers to this address. Previously if there is a pooled connection, there is no round robin selection. --- src/shrpx_client_handler.cc | 69 +++++++++++++------------ src/shrpx_http_downstream_connection.cc | 10 ---- src/shrpx_worker.cc | 12 +---- src/shrpx_worker.h | 1 - 4 files changed, 38 insertions(+), 54 deletions(-) diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index cdb20146..71e522a0 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -658,30 +658,11 @@ void ClientHandler::pool_downstream_connection( << " in group " << group; } - auto &shared_addr = group->shared_addr; - - if (shared_addr->affinity.type == SessionAffinity::NONE) { - auto &dconn_pool = group->shared_addr->dconn_pool; - dconn_pool.add_downstream_connection(std::move(dconn)); - - return; - } - auto addr = dconn->get_addr(); auto &dconn_pool = addr->dconn_pool; dconn_pool->add_downstream_connection(std::move(dconn)); } -void ClientHandler::remove_downstream_connection(DownstreamConnection *dconn) { - if (LOG_ENABLED(INFO)) { - CLOG(INFO, this) << "Removing downstream connection DCONN:" << dconn - << " from pool"; - } - auto &dconn_pool = - dconn->get_downstream_addr_group()->shared_addr->dconn_pool; - dconn_pool.remove_downstream_connection(dconn); -} - namespace { // Computes 32bits hash for session affinity for IP address |ip|. uint32_t compute_affinity_from_ip(const StringRef &ip) { @@ -1138,26 +1119,48 @@ ClientHandler::get_downstream_connection(int &err, Downstream *downstream, return std::move(dconn); } - auto &dconn_pool = shared_addr->dconn_pool; + auto end = shared_addr->next; + for (;;) { + auto addr = &shared_addr->addrs[shared_addr->next]; - // pool connection must be HTTP/1.1 connection - auto dconn = dconn_pool.pop_downstream_connection(); + if (addr->proto != Proto::HTTP1) { + if (++shared_addr->next >= shared_addr->addrs.size()) { + shared_addr->next = 0; + } - if (dconn) { - if (LOG_ENABLED(INFO)) { - CLOG(INFO, this) << "Reuse downstream connection DCONN:" << dconn.get() - << " from pool"; - } - } else { - if (LOG_ENABLED(INFO)) { - CLOG(INFO, this) << "Downstream connection pool is empty." - << " Create new one"; + assert(end != shared_addr->next); + + continue; } - dconn = std::make_unique(group, 0, conn_.loop, - worker_); + // pool connection must be HTTP/1.1 connection + auto dconn = addr->dconn_pool->pop_downstream_connection(); + if (dconn) { + if (++shared_addr->next >= shared_addr->addrs.size()) { + shared_addr->next = 0; + } + + if (LOG_ENABLED(INFO)) { + CLOG(INFO, this) << "Reuse downstream connection DCONN:" << dconn.get() + << " from pool"; + } + + dconn->set_client_handler(this); + + return dconn; + } + + break; } + if (LOG_ENABLED(INFO)) { + CLOG(INFO, this) << "Downstream connection pool is empty." + << " Create new one"; + } + + auto dconn = + std::make_unique(group, 0, conn_.loop, worker_); + dconn->set_client_handler(this); return dconn; diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 6b4c28eb..1fea9b14 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -807,16 +807,6 @@ int HttpDownstreamConnection::end_upload_data() { namespace { void remove_from_pool(HttpDownstreamConnection *dconn) { - auto &group = dconn->get_downstream_addr_group(); - auto &shared_addr = group->shared_addr; - - if (shared_addr->affinity.type == SessionAffinity::NONE) { - auto &dconn_pool = - dconn->get_downstream_addr_group()->shared_addr->dconn_pool; - dconn_pool.remove_downstream_connection(dconn); - return; - } - auto addr = dconn->get_addr(); auto &dconn_pool = addr->dconn_pool; dconn_pool->remove_downstream_connection(dconn); diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index 8ce9869c..83b6fef2 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -164,12 +164,6 @@ void Worker::replace_downstream_config( g->retired = true; auto &shared_addr = g->shared_addr; - - if (shared_addr->affinity.type == SessionAffinity::NONE) { - shared_addr->dconn_pool.remove_all(); - continue; - } - for (auto &addr : shared_addr->addrs) { addr.dconn_pool->remove_all(); } @@ -307,10 +301,8 @@ void Worker::replace_downstream_config( std::shuffle(std::begin(shared_addr->addrs), std::end(shared_addr->addrs), randgen_); - if (shared_addr->affinity.type != SessionAffinity::NONE) { - for (auto &addr : shared_addr->addrs) { - addr.dconn_pool = std::make_unique(); - } + for (auto &addr : shared_addr->addrs) { + addr.dconn_pool = std::make_unique(); } dst->shared_addr = shared_addr; diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index 3705b6fe..14e2e2de 100644 --- a/src/shrpx_worker.h +++ b/src/shrpx_worker.h @@ -164,7 +164,6 @@ struct SharedDownstreamAddr { // TODO Verify that this approach performs better in performance // wise. DList http2_avail_freelist; - DownstreamConnectionPool dconn_pool; // Configuration for session affinity AffinityConfig affinity; // Next http/1.1 downstream address index in addrs.