Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
nghttpx: Use an existing h2 backend connection as much as possible
h2load measurement reveals that this strategy is 3 times faster than
the previous implementations.
  • Loading branch information
tatsuhiro-t committed Oct 19, 2017
1 parent aaa0b85 commit f507b5e
Showing 1 changed file with 30 additions and 46 deletions.
76 changes: 30 additions & 46 deletions src/shrpx_client_handler.cc
Expand Up @@ -804,61 +804,40 @@ bool load_lighter(const DownstreamAddr *lhs, const DownstreamAddr *rhs) {
Http2Session *ClientHandler::select_http2_session(
const std::shared_ptr<DownstreamAddrGroup> &group) {
auto &shared_addr = group->shared_addr;

// First count the working backend addresses.
size_t min = 0;
for (const auto &addr : shared_addr->addrs) {
if (addr.proto != PROTO_HTTP2 || addr.connect_blocker->blocked()) {
continue;
}

++min;
}

if (min == 0) {
if (LOG_ENABLED(INFO)) {
CLOG(INFO, this) << "No working backend address found";
}

return nullptr;
}

auto &http2_avail_freelist = shared_addr->http2_avail_freelist;

if (http2_avail_freelist.size() >= min) {
for (auto session = http2_avail_freelist.head; session;) {
auto next = session->dlnext;
for (auto session = http2_avail_freelist.head; session;) {
auto next = session->dlnext;

session->remove_from_freelist();
session->remove_from_freelist();

// session may be in graceful shutdown period now.
if (session->max_concurrency_reached(0)) {
if (LOG_ENABLED(INFO)) {
CLOG(INFO, this)
<< "Maximum streams have been reached for Http2Session("
<< session << "). Skip it";
}
// session may be in graceful shutdown period now.
if (session->max_concurrency_reached(0)) {
if (LOG_ENABLED(INFO)) {
CLOG(INFO, this)
<< "Maximum streams have been reached for Http2Session(" << session
<< "). Skip it";
}

session = next;
session = next;

continue;
}
continue;
}

if (LOG_ENABLED(INFO)) {
CLOG(INFO, this) << "Use Http2Session " << session
<< " from http2_avail_freelist";
}
if (LOG_ENABLED(INFO)) {
CLOG(INFO, this) << "Use Http2Session " << session
<< " from http2_avail_freelist";
}

if (session->max_concurrency_reached(1)) {
if (LOG_ENABLED(INFO)) {
CLOG(INFO, this) << "Maximum streams are reached for Http2Session("
<< session << ").";
}
} else {
session->add_to_avail_freelist();
if (session->max_concurrency_reached(1)) {
if (LOG_ENABLED(INFO)) {
CLOG(INFO, this) << "Maximum streams are reached for Http2Session("
<< session << ").";
}
return session;
} else {
session->add_to_avail_freelist();
}
return session;
}

DownstreamAddr *selected_addr = nullptr;
Expand Down Expand Up @@ -901,7 +880,12 @@ Http2Session *ClientHandler::select_http2_session(
}
}

assert(selected_addr);
if (selected_addr == nullptr) {
if (LOG_ENABLED(INFO)) {
CLOG(INFO, this) << "No working backend address found";
}
return nullptr;
}

if (LOG_ENABLED(INFO)) {
CLOG(INFO, this) << "Selected DownstreamAddr=" << selected_addr
Expand Down

2 comments on commit f507b5e

@tcicha
Copy link

@tcicha tcicha commented on f507b5e Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

first of all, I did not find any support forums, so please feel free to redirect me somewhere else if commenting here is in any way inappropriate.

I tracked down this commit while searching for a reason why my loadbalancer is not loadbalancing :)

I have a setup with 2 gRPC backends in the same lb group and I want to balance incoming traffic between these two (all the traffic is coming to the frontend from the same IP). Before this (<1.27.0), everything is working as I would expect. This commit causes nghttpx to consistently reuse the connection to first backend and it never even attempts to connect to the second backend. Is this the desired behaviour? Please, correct me if I am doing something wrong (and I may be as this is my first contact with nghttpx), but in my opinion, this is not loadbalancing :)

Regards,

Tom

@tcicha
Copy link

@tcicha tcicha commented on f507b5e Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I just noticed this was reverted in master 9 hours ago :) Nevermind.

Have a nice day,

Tom

Please sign in to comment.