From b6708a4b87d56f59f8ddee99f8abd5585ccd13fe Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 12 Apr 2016 23:30:52 +0900 Subject: [PATCH] nghttpx: Retry next HTTP/2 backend address when connection cannot be made --- src/shrpx_http2_downstream_connection.cc | 4 +- src/shrpx_http2_session.cc | 58 +++++++++++------------- src/shrpx_http2_session.h | 9 +++- src/shrpx_http2_upstream.cc | 5 ++ src/shrpx_https_upstream.cc | 5 ++ src/shrpx_spdy_upstream.cc | 5 ++ 6 files changed, 50 insertions(+), 36 deletions(-) diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index a72422f5..24d015b1 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -95,9 +95,7 @@ int Http2DownstreamConnection::attach_downstream(Downstream *downstream) { DCLOG(INFO, this) << "Attaching to DOWNSTREAM:" << downstream; } http2session_->add_downstream_connection(this); - if (http2session_->get_state() == Http2Session::DISCONNECTED) { - http2session_->signal_write(); - } + http2session_->signal_write(); downstream_ = downstream; downstream_->reset_downstream_rtimer(); diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index aecfa134..daf6798a 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -73,11 +73,9 @@ void connchk_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { if (LOG_ENABLED(INFO)) { SSLOG(INFO, http2session) << "ping timeout"; } - http2session->disconnect(); - if (http2session->get_num_dconns() == 0) { - delete http2session; - } + delete http2session; + return; default: if (LOG_ENABLED(INFO)) { @@ -95,10 +93,8 @@ void settings_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { http2session->stop_settings_timer(); SSLOG(INFO, http2session) << "SETTINGS timeout"; if (http2session->terminate_session(NGHTTP2_SETTINGS_TIMEOUT) != 0) { - http2session->disconnect(); - if (http2session->get_num_dconns() == 0) { - delete http2session; - } + delete http2session; + return; } http2session->signal_write(); @@ -114,11 +110,7 @@ void timeoutcb(struct ev_loop *loop, ev_timer *w, int revents) { SSLOG(INFO, http2session) << "Timeout"; } - http2session->disconnect(http2session->get_state() == - Http2Session::CONNECTING); - if (http2session->get_num_dconns() == 0) { - delete http2session; - } + delete http2session; } } // namespace @@ -129,20 +121,16 @@ void readcb(struct ev_loop *loop, ev_io *w, int revents) { auto http2session = static_cast(conn->data); rv = http2session->do_read(); if (rv != 0) { - http2session->disconnect(http2session->should_hard_fail()); - if (http2session->get_num_dconns() == 0) { - delete http2session; - } + delete http2session; + return; } http2session->connection_alive(); rv = http2session->do_write(); if (rv != 0) { - http2session->disconnect(http2session->should_hard_fail()); - if (http2session->get_num_dconns() == 0) { - delete http2session; - } + delete http2session; + return; } } @@ -155,10 +143,8 @@ void writecb(struct ev_loop *loop, ev_io *w, int revents) { auto http2session = static_cast(conn->data); rv = http2session->do_write(); if (rv != 0) { - http2session->disconnect(http2session->should_hard_fail()); - if (http2session->get_num_dconns() == 0) { - delete http2session; - } + delete http2session; + return; } http2session->reset_connection_check_timer_if_not_checking(); @@ -173,9 +159,9 @@ void initiate_connection_cb(struct ev_loop *loop, ev_timer *w, int revents) { if (LOG_ENABLED(INFO)) { SSLOG(INFO, http2session) << "Could not initiate backend connection"; } - http2session->disconnect(true); - assert(http2session->get_num_dconns() == 0); + delete http2session; + return; } } @@ -223,9 +209,8 @@ Http2Session::Http2Session(struct ev_loop *loop, SSL_CTX *ssl_ctx, } Http2Session::~Http2Session() { - disconnect(true); - - remove_from_freelist(); + exclude_from_scheduling(); + disconnect(should_hard_fail()); } int Http2Session::disconnect(bool hard) { @@ -1940,9 +1925,11 @@ bool Http2Session::should_hard_fail() const { switch (state_) { case PROXY_CONNECTING: case PROXY_FAILED: - case CONNECTING: - case CONNECT_FAILING: return true; + case DISCONNECTED: { + const auto &proxy = get_config()->downstream_http_proxy; + return !proxy.host.empty(); + } default: return false; } @@ -2112,9 +2099,16 @@ void Http2Session::remove_from_freelist() { } addr_->http2_extra_freelist.remove(this); break; + case FREELIST_ZONE_GONE: + return; } freelist_zone_ = FREELIST_ZONE_NONE; } +void Http2Session::exclude_from_scheduling() { + remove_from_freelist(); + freelist_zone_ = FREELIST_ZONE_GONE; +} + } // namespace shrpx diff --git a/src/shrpx_http2_session.h b/src/shrpx_http2_session.h index c6253a25..37a9d150 100644 --- a/src/shrpx_http2_session.h +++ b/src/shrpx_http2_session.h @@ -64,7 +64,10 @@ enum FreelistZone { FREELIST_ZONE_AVAIL, // Http2Session object is linked in address scope // http2_extra_freelist. - FREELIST_ZONE_EXTRA + FREELIST_ZONE_EXTRA, + // Http2Session object is about to be deleted, and it does not + // belong to any linked list. + FREELIST_ZONE_GONE }; class Http2Session { @@ -180,6 +183,10 @@ public: // linked from any freelist, this function does nothing. void remove_from_freelist(); + // Removes this object form any freelist, and marks this object as + // not schedulable. + void exclude_from_scheduling(); + // Returns true if the maximum concurrency is reached. In other // words, the number of currently participated streams in this // session is equal or greater than the max concurrent streams limit diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 06508d82..d0267d89 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1759,6 +1759,11 @@ int Http2Upstream::on_downstream_reset(bool no_retry) { goto fail; } + rv = downstream->push_request_headers(); + if (rv != 0) { + goto fail; + } + continue; fail: diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index a9bdd3e8..5bfeab50 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -1197,6 +1197,11 @@ int HttpsUpstream::on_downstream_reset(bool no_retry) { goto fail; } + rv = downstream_->push_request_headers(); + if (rv != 0) { + goto fail; + } + return 0; fail: diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 758ff525..bbb96ba6 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -1269,6 +1269,11 @@ int SpdyUpstream::on_downstream_reset(bool no_retry) { goto fail; } + rv = downstream->push_request_headers(); + if (rv != 0) { + goto fail; + } + continue; fail: