Skip to content

Commit 5abbffd

Browse files
authored
http2: send graceful goaways on close (#41284)
### Description With this change envoy will send one GOAWAY frame with a last_stream_id set to 2^31-1, as described in [RFC 9113](https://www.rfc-editor.org/rfc/rfc9113.html#name-goaway). After a reasonable interval (defaults to 1s), envoy will send a followup GOAWAY frame with last_stream_id set to the highest received stream ID at that point in time. This interval is configurable using `graceful_goaway_timeout` field. --- Commit Message: http2: send graceful GoAway on close Additional Description: According to RFC 9113, envoy should send a goaway with last_stream_id set to 2^31-1 before sending a final goaway with the highest received stream ID. Risk Level: Low Testing: Tests added. Manually verified. Docs Changes: NA Release Notes: Changelog added Platform Specific Features: Runtime guard: `envoy.reloadable_features.http2_graceful_goaway` Fixes #39876 [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Anurag Aggarwal <[email protected]>
1 parent c39f099 commit 5abbffd

File tree

11 files changed

+80
-18
lines changed

11 files changed

+80
-18
lines changed

changelogs/current.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ new_features:
124124
Added :ref:`vhost_header <envoy_v3_api_field_config.route.v3.RouteConfiguration.vhost_header>` to
125125
:ref:`RouteConfiguration <envoy_v3_api_msg_config.route.v3.RouteConfiguration>`, allowing use of a different
126126
header for vhost matching.
127+
- area: http2
128+
change: |
129+
Added new parameter to the ``sendGoAwayAndClose`` to support gracefully closing of HTTP/2 connection.
127130
- area: logging
128131
change: |
129132
Added support for the not-equal operator in access log filter rules, in

envoy/http/filter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ class StreamDecoderFilterCallbacks : public virtual StreamFilterCallbacks {
668668
/**
669669
* Attempt to send GOAWAY and close the connection, and no filter chain will move forward.
670670
*/
671-
virtual void sendGoAwayAndClose() PURE;
671+
virtual void sendGoAwayAndClose(bool graceful = false) PURE;
672672

673673
/**
674674
* Adds decoded metadata. This function can only be called in

source/common/http/async_client_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ class AsyncStreamImpl : public virtual AsyncClient::Stream,
228228
}
229229
void addDownstreamWatermarkCallbacks(DownstreamWatermarkCallbacks&) override {}
230230
void removeDownstreamWatermarkCallbacks(DownstreamWatermarkCallbacks&) override {}
231-
void sendGoAwayAndClose() override {}
231+
void sendGoAwayAndClose(bool graceful [[maybe_unused]] = false) override {}
232232

233233
void setDecoderBufferLimit(uint64_t) override {
234234
IS_ENVOY_BUG("decoder buffer limits should not be overridden on async streams.");

source/common/http/conn_manager_impl.cc

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -780,16 +780,33 @@ void ConnectionManagerImpl::onDrainTimeout() {
780780
checkForDeferredClose(false);
781781
}
782782

783-
void ConnectionManagerImpl::sendGoAwayAndClose() {
783+
void ConnectionManagerImpl::sendGoAwayAndClose(bool graceful) {
784784
ENVOY_CONN_LOG(trace, "connection manager sendGoAwayAndClose was triggerred from filters.",
785785
read_callbacks_->connection());
786786
if (go_away_sent_) {
787787
return;
788788
}
789-
codec_->goAway();
790-
go_away_sent_ = true;
791-
doConnectionClose(Network::ConnectionCloseType::FlushWriteAndDelay, absl::nullopt,
792-
"forced_goaway");
789+
790+
// Use graceful drain sequence if graceful shutdown is requested.
791+
// startDrainSequence() works for both HTTP/1 and HTTP/2:
792+
// - HTTP/1: shutdownNotice() + goAway() provides graceful close
793+
// - HTTP/2: shutdownNotice() sends GOAWAY with high stream ID, then goAway() sends final GOAWAY
794+
if (graceful) {
795+
if (drain_state_ == DrainState::NotDraining) {
796+
startDrainSequence();
797+
}
798+
// Consider the "go away" process started once draining begins.
799+
// The actual GOAWAY frame will be sent in onDrainTimeout(), but we want to
800+
// prevent multiple calls to sendGoAwayAndClose() from starting multiple drain sequences.
801+
go_away_sent_ = true;
802+
} else {
803+
// Immediate close - send GOAWAY and close immediately
804+
codec_->shutdownNotice();
805+
codec_->goAway();
806+
go_away_sent_ = true;
807+
doConnectionClose(Network::ConnectionCloseType::FlushWriteAndDelay, absl::nullopt,
808+
"forced_goaway");
809+
}
793810
}
794811

795812
void ConnectionManagerImpl::chargeTracingStats(const Tracing::Reason& tracing_reason,

source/common/http/conn_manager_impl.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,9 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
191191
return filter_manager_.sendLocalReply(code, body, modify_headers, grpc_status, details);
192192
}
193193

194-
void sendGoAwayAndClose() override { return connection_manager_.sendGoAwayAndClose(); }
194+
void sendGoAwayAndClose(bool graceful = false) override {
195+
return connection_manager_.sendGoAwayAndClose(graceful);
196+
}
195197

196198
AccessLog::InstanceSharedPtrVector accessLogHandlers() override {
197199
const AccessLog::InstanceSharedPtrVector& config_log_handlers =
@@ -601,7 +603,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
601603
void doConnectionClose(absl::optional<Network::ConnectionCloseType> close_type,
602604
absl::optional<StreamInfo::CoreResponseFlag> response_flag,
603605
absl::string_view details);
604-
void sendGoAwayAndClose();
606+
void sendGoAwayAndClose(bool graceful = false);
605607

606608
// Returns true if a RST_STREAM for the given stream is premature. Premature
607609
// means the RST_STREAM arrived before response headers were sent and than

source/common/http/filter_manager.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,9 @@ void ActiveStreamDecoderFilter::sendLocalReply(
487487
ActiveStreamFilterBase::sendLocalReply(code, body, modify_headers, grpc_status, details);
488488
}
489489

490-
void ActiveStreamDecoderFilter::sendGoAwayAndClose() { parent_.sendGoAwayAndClose(); }
490+
void ActiveStreamDecoderFilter::sendGoAwayAndClose(bool graceful) {
491+
parent_.sendGoAwayAndClose(graceful);
492+
}
491493

492494
void ActiveStreamDecoderFilter::encode1xxHeaders(ResponseHeaderMapPtr&& headers) {
493495
// If Envoy is not configured to proxy 100-Continue responses, swallow the 100 Continue

source/common/http/filter_manager.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ struct ActiveStreamDecoderFilter : public ActiveStreamFilterBase,
300300
void setUpstreamOverrideHost(Upstream::LoadBalancerContext::OverrideHost) override;
301301
absl::optional<Upstream::LoadBalancerContext::OverrideHost> upstreamOverrideHost() const override;
302302
bool shouldLoadShed() const override;
303-
void sendGoAwayAndClose() override;
303+
void sendGoAwayAndClose(bool graceful = false) override;
304304

305305
// Each decoder filter instance checks if the request passed to the filter is gRPC
306306
// so that we can issue gRPC local responses to gRPC requests. Filter's decodeHeaders()
@@ -489,7 +489,7 @@ class FilterManagerCallbacks {
489489
/**
490490
* Attempt to send GOAWAY and close the connection.
491491
*/
492-
virtual void sendGoAwayAndClose() PURE;
492+
virtual void sendGoAwayAndClose(bool graceful = false) PURE;
493493

494494
/**
495495
* Called when the stream write buffer is no longer above the low watermark.
@@ -906,14 +906,14 @@ class FilterManager : public ScopeTrackedObject,
906906

907907
virtual bool shouldLoadShed() { return false; };
908908

909-
void sendGoAwayAndClose() {
909+
void sendGoAwayAndClose(bool graceful = false) {
910910
// Stop filter chain iteration by checking encoder or decoder chain.
911911
if (state_.filter_call_state_ & FilterCallState::IsDecodingMask) {
912912
state_.decoder_filter_chain_aborted_ = true;
913913
} else if (state_.filter_call_state_ & FilterCallState::IsEncodingMask) {
914914
state_.encoder_filter_chain_aborted_ = true;
915915
}
916-
filter_manager_callbacks_.sendGoAwayAndClose();
916+
filter_manager_callbacks_.sendGoAwayAndClose(graceful);
917917
}
918918

919919
protected:

source/common/router/upstream_request.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ class UpstreamRequestFilterManagerCallbacks : public Http::FilterManagerCallback
344344
void disarmRequestTimeout() override {}
345345
void resetIdleTimer() override {}
346346
void onLocalReply(Http::Code) override {}
347-
void sendGoAwayAndClose() override {}
347+
void sendGoAwayAndClose(bool graceful [[maybe_unused]] = false) override {}
348348
// Upgrade filter chains not supported.
349349
const Router::RouteEntry::UpgradeMap* upgradeMap() override { return nullptr; }
350350

source/common/tcp_proxy/tcp_proxy.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ class Filter : public Network::ReadFilter,
563563
std::function<void(Http::ResponseHeaderMap& headers)>,
564564
const absl::optional<Grpc::Status::GrpcStatus>,
565565
absl::string_view) override {}
566-
void sendGoAwayAndClose() override {}
566+
void sendGoAwayAndClose(bool graceful [[maybe_unused]] = false) override {}
567567
void encode1xxHeaders(Http::ResponseHeaderMapPtr&&) override {}
568568
Http::ResponseHeaderMapOptRef informationalHeaders() override { return {}; }
569569
void encodeHeaders(Http::ResponseHeaderMapPtr&&, bool, absl::string_view) override {}

test/common/http/conn_manager_impl_test.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4933,5 +4933,43 @@ TEST_F(HttpConnectionManagerImplTest, ShouldDrainConnectionUponCompletionHttp11)
49334933
response_encoder_.stream_.codec_callbacks_->onCodecEncodeComplete();
49344934
}
49354935

4936+
// Test graceful shutdown behavior with sendGoAwayAndClose(true)
4937+
TEST_F(HttpConnectionManagerImplTest, SendGoAwayAndCloseGraceful) {
4938+
setup();
4939+
// Mock codec to return HTTP/2 protocol for graceful shutdown support.
4940+
EXPECT_CALL(*codec_, protocol()).WillRepeatedly(Return(Protocol::Http2));
4941+
MockStreamDecoderFilter* filter = new NiceMock<MockStreamDecoderFilter>();
4942+
EXPECT_CALL(filter_factory_, createFilterChain(_))
4943+
.WillOnce(Invoke([&](FilterChainManager& manager) -> bool {
4944+
auto factory = createDecoderFilterFactoryCb(StreamDecoderFilterSharedPtr{filter});
4945+
manager.applyFilterFactoryCb({}, factory);
4946+
return true;
4947+
}));
4948+
EXPECT_CALL(*filter, decodeHeaders(_, true))
4949+
.WillOnce(Invoke([&](RequestHeaderMap&, bool) -> FilterHeadersStatus {
4950+
// Trigger graceful shutdown during request processing
4951+
filter->callbacks_->sendGoAwayAndClose(true);
4952+
return FilterHeadersStatus::StopIteration;
4953+
}));
4954+
EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> Http::Status {
4955+
decoder_ = &conn_manager_->newStream(response_encoder_);
4956+
RequestHeaderMapPtr headers{
4957+
new TestRequestHeaderMapImpl{{":authority", "host"}, {":path", "/"}, {":method", "GET"}}};
4958+
decoder_->decodeHeaders(std::move(headers), true);
4959+
return Http::okStatus();
4960+
}));
4961+
// Expect graceful shutdown to start drain timer and send shutdown notice
4962+
Event::MockTimer* drain_timer = setUpTimer();
4963+
EXPECT_CALL(*drain_timer, enableTimer(_, _));
4964+
EXPECT_CALL(*codec_, shutdownNotice());
4965+
Buffer::OwnedImpl fake_input;
4966+
conn_manager_->onData(fake_input, false);
4967+
4968+
// Complete the existing stream
4969+
ResponseHeaderMapPtr response_headers{new TestResponseHeaderMapImpl{{":status", "200"}}};
4970+
filter->callbacks_->streamInfo().setResponseCodeDetails("");
4971+
filter->callbacks_->encodeHeaders(std::move(response_headers), true, "details");
4972+
}
4973+
49364974
} // namespace Http
49374975
} // namespace Envoy

0 commit comments

Comments
 (0)