From 95ee48d7a8c3e95b8094eabc0566f049d3448192 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 10 Jun 2024 15:52:16 +0000 Subject: [PATCH] runtime: removing thrift_connection_draining Signed-off-by: Alyssa Wilk --- changelogs/current.yaml | 7 ++-- source/common/runtime/runtime_features.cc | 1 - .../network/thrift_proxy/conn_manager.cc | 35 +++++++++---------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 3c3e6ba20947..38ae27f3d90b 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -178,10 +178,13 @@ removed_config_or_runtime: Removed ``envoy.reloadable_features.detect_and_raise_rst_tcp_connection`` runtime flag and legacy code paths. - area: http change: | - Removed ``envoy.reloadable_features.lowercase_scheme`` runtime flag and lagacy code paths. + Removed ``envoy.reloadable_features.lowercase_scheme`` runtime flag and legacy code paths. - area: upstream change: | - Removed ``envoy.reloadable_features.convert_legacy_lb_config`` runtime flag and lagacy code paths. + Removed ``envoy.reloadable_features.convert_legacy_lb_config`` runtime flag and legacy code paths. +- area: thrift + change: | + Removed ``envoy.reloadable_features.thrift_connection_draining`` runtime flag and legacy code paths. - area: router change: | Removed ``envoy.reloadable_features.copy_response_code_to_downstream_stream_info`` runtime flag and legacy code paths. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 50bc1655a634..4b47ce1fa452 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -96,7 +96,6 @@ RUNTIME_GUARD(envoy_reloadable_features_strict_duration_validation); RUNTIME_GUARD(envoy_reloadable_features_tcp_tunneling_send_downstream_fin_on_upstream_trailers); RUNTIME_GUARD(envoy_reloadable_features_test_feature_true); RUNTIME_GUARD(envoy_reloadable_features_thrift_allow_negative_field_ids); -RUNTIME_GUARD(envoy_reloadable_features_thrift_connection_draining); RUNTIME_GUARD(envoy_reloadable_features_udp_socket_apply_aggregated_read_limit); RUNTIME_GUARD(envoy_reloadable_features_uhv_allow_malformed_url_encoding); RUNTIME_GUARD(envoy_reloadable_features_upstream_allow_connect_with_2xx); diff --git a/source/extensions/filters/network/thrift_proxy/conn_manager.cc b/source/extensions/filters/network/thrift_proxy/conn_manager.cc index 53ab248b3cec..4cb3d62fb5d3 100644 --- a/source/extensions/filters/network/thrift_proxy/conn_manager.cc +++ b/source/extensions/filters/network/thrift_proxy/conn_manager.cc @@ -344,23 +344,21 @@ FilterStatus ConnectionManager::ResponseDecoder::messageBegin(MessageMetadataSha // that we can support the header in TTwitter protocol, which reads/adds response headers to // metadata in messageBegin when reading the response from upstream. Therefore detecting a drain // should happen here. - if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.thrift_connection_draining")) { - metadata_->setDraining(!metadata->responseHeaders().get(Headers::get().Drain).empty()); - metadata->responseHeaders().remove(Headers::get().Drain); - - // Check if this host itself is draining. - // - // Note: Similarly as above, the response is buffered until transportEnd. Therefore metadata - // should be set before the encodeFrame() call. It should be set at or after the messageBegin - // call so that the header is added after all upstream headers passed, due to messageBegin - // possibly not getting headers in transportBegin. - if (cm.drain_decision_.drainClose()) { - ENVOY_STREAM_LOG(debug, "propogate Drain header for drain close decision", parent_); - // TODO(rgs1): should the key value contain something useful (e.g.: minutes til drain is - // over)? - metadata->responseHeaders().addReferenceKey(Headers::get().Drain, "true"); - cm.stats_.downstream_response_drain_close_.inc(); - } + metadata_->setDraining(!metadata->responseHeaders().get(Headers::get().Drain).empty()); + metadata->responseHeaders().remove(Headers::get().Drain); + + // Check if this host itself is draining. + // + // Note: Similarly as above, the response is buffered until transportEnd. Therefore metadata + // should be set before the encodeFrame() call. It should be set at or after the messageBegin + // call so that the header is added after all upstream headers passed, due to messageBegin + // possibly not getting headers in transportBegin. + if (cm.drain_decision_.drainClose()) { + ENVOY_STREAM_LOG(debug, "propogate Drain header for drain close decision", parent_); + // TODO(rgs1): should the key value contain something useful (e.g.: minutes til drain is + // over)? + metadata->responseHeaders().addReferenceKey(Headers::get().Drain, "true"); + cm.stats_.downstream_response_drain_close_.inc(); } parent_.recordResponseAccessLog(metadata); @@ -1046,8 +1044,7 @@ void ConnectionManager::ActiveRpc::sendLocalReply(const DirectResponse& response onLocalReply(*localReplyMetadata_, end_stream); - if (end_stream && - Runtime::runtimeFeatureEnabled("envoy.reloadable_features.thrift_connection_draining")) { + if (end_stream) { localReplyMetadata_->responseHeaders().addReferenceKey(Headers::get().Drain, "true"); ConnectionManager& cm = parent_; cm.stats_.downstream_response_drain_close_.inc();