From 9ecc481946e95c6fdd18320270065f25c4a57443 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Tue, 17 Jul 2018 13:47:49 -0400 Subject: [PATCH 1/6] http: global connection manager per-stream idle timeouts. This is a followup to #3841, where we introduce HCM-wide stream idle timeouts. This has two effects: 1. We can now timeout immediately after stream creation, potentially before receiving request headers and routing. 2. A default timeout can be configured across all routes. This is overridable on a per-route basis. The default and overriding semantics are explained in the docs. Also added as a bonus some docs about how timeouts interact more generally in Envoy. Fixes #3853. Risk Level: Low. While there is some change to the per-route vs. HCM wide semantics for stream idle timeouts, it's not anticipated this feature is in common use yet (it's only a couple of days since landing), and the caveats in #3841 with the new 5 minute default timeout should already apply. Testing: Unit/integration tests added. Signed-off-by: Harvey Tuch --- api/envoy/api/v2/route/route.proto | 11 ++-- .../v2/http_connection_manager.proto | 23 ++++++- .../http_connection_management.rst | 24 +++++++ docs/root/intro/version_history.rst | 10 +-- source/common/http/conn_manager_config.h | 7 +- source/common/http/conn_manager_impl.cc | 17 ++++- source/common/router/config_impl.cc | 3 +- source/common/router/config_impl.h | 10 +-- .../network/http_connection_manager/config.cc | 7 +- .../network/http_connection_manager/config.h | 11 +++- source/server/http/admin.h | 5 +- test/common/http/conn_manager_impl_test.cc | 65 ++++++++++++++++++- test/common/http/conn_manager_utility_test.cc | 3 +- .../http_connection_manager/config_test.cc | 25 +++++++ .../idle_timeout_integration_test.cc | 33 ++++++++-- 15 files changed, 215 insertions(+), 39 deletions(-) diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index 1ac5b46e4ba2a..6bcc7b1e6669e 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -439,12 +439,11 @@ message RouteAction { google.protobuf.Duration per_try_timeout = 3 [(gogoproto.stdduration) = true]; } - // Specifies the idle timeout for the route. If not specified, this defaults - // to 5 minutes. The default value was select so as not to interfere with any - // smaller configured timeouts that may have existed in configurations prior - // to the introduction of this feature, while introducing robustness to TCP - // connections that terminate without FIN. A value of 0 will completely - // disable the idle timeout. + // Specifies the idle timeout for the route. If not specified, there is no per-route + // idle timeout specified, although the connection manager wide + // :ref:`stream_idle_timeout + // ` + // will still apply. // // The idle timeout is distinct to :ref:`timeout // `, which provides an upper bound diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index d4c8cd3077d13..d9e8c02cc3b57 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -19,7 +19,7 @@ import "gogoproto/gogo.proto"; // [#protodoc-title: HTTP connection manager] // HTTP connection manager :ref:`configuration overview `. -// [#comment:next free field: 24] +// [#comment:next free field: 25] message HttpConnectionManager { enum CodecType { option (gogoproto.goproto_enum_prefix) = false; @@ -137,6 +137,27 @@ message HttpConnectionManager { // `. google.protobuf.Duration idle_timeout = 11 [(gogoproto.stdduration) = true]; + // The stream idle timeout for connections managed by the connection manager. + // If not specified, this defaults to 5 minutes. The default value was select + // so as not to interfere with any smaller configured timeouts that may have + // existed in configurations prior to the introduction of this feature, while + // introducing robustness to TCP connections that terminate without FIN. + // + // This idle timeout applies to new streams and is overridable by the + // :ref:`route-level idle_timeout + // `. Even on a stream in + // which the override applies, prior to receipt of the initial request + // headers, the :ref:`stream_idle_timeout + // ` + // applies. Each time an encode/decode event for headers or data is processed + // for the stream, the timer will be reset. If the timeout fires, the stream + // is terminated with a 408 Request Timeout error code if no upstream response + // header has been received, otherwise a stream reset occurs. + // + // A value of 0 will completely disable the connection manager stream idle + // timeout, although per-route idle timeout overrides will continue to apply. + google.protobuf.Duration stream_idle_timeout = 24 [(gogoproto.stdduration) = true]; + // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. // This is used so that Envoy provides a grace period for new streams that diff --git a/docs/root/intro/arch_overview/http_connection_management.rst b/docs/root/intro/arch_overview/http_connection_management.rst index 4f1d415b48e3d..b7c8f9dc0dad8 100644 --- a/docs/root/intro/arch_overview/http_connection_management.rst +++ b/docs/root/intro/arch_overview/http_connection_management.rst @@ -42,3 +42,27 @@ table `. The route table can be specified in one of * Statically. * Dynamically via the :ref:`RDS API `. + +Timeouts +-------- + +Various configurable timeouts apply to an HTTP connection and its constituent streams: + +* Connection-level :ref:`idle timeout + `: + this applies to the idle period where no streams are active. +* Connection-level :ref:`drain timeout + `: + this spans between an Envoy originated GOAWAY and connection termination. +* Stream-level idle timeout: this applies to each individual stream. It may be configured at both + the :ref:`connection manage + ` + and :ref:`per-route ` granularity. + Header/data/trailer events on the stream reset the idle timeout. +* Stream-level :ref:`per-route upstream timeout `: this + applies to the upstream response, i.e. a maximum bound on the time from the end of the downstream + request until the end of the upstream response. This may also be specified at the :ref:`per-retry + ` granularity. +* Stream-level :ref:`per-route gRPC max timeout + `: this bounds the upstream timeout and allows + the timeout to be overriden via the *grpc-timeout* request header. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index e6f55e59f3c5f..dc9a7920f2b5c 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -14,10 +14,12 @@ Version history * health check: added support for :ref:`custom health check `. * health check: added support for :ref:`specifying jitter as a percentage `. * health_check: added support for :ref:`health check event logging `. -* http: added support for a :ref:`per-stream idle timeout - `. This defaults to 5 minutes; if you have - other timeouts (e.g. connection idle timeout, upstream response per-retry) that are longer than - this in duration, you may want to consider setting a non-default per-stream idle timeout. +* http: added support for a per-stream idle timeout. This applies at both :ref:`connection manager + ` + and :ref:`per-route granularity `. The timeout + defaults to 5 minutes; if you have other timeouts (e.g. connection idle timeout, upstream + response per-retry) that are longer than this in duration, you may want to consider setting a + non-default per-stream idle timeout. * http: better handling of HEAD requests. Now sending transfer-encoding: chunked rather than content-length: 0. * http: response filters not applied to early error paths such as http_parser generated 400s. * proxy_protocol: added support for HAProxy Proxy Protocol v2 (AF_INET/AF_INET6 only). diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 4d6ca959adbab..0efec40e76682 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -192,7 +192,12 @@ class ConnectionManagerConfig { /** * @return optional idle timeout for incoming connection manager connections. */ - virtual const absl::optional& idleTimeout() PURE; + virtual const absl::optional idleTimeout() PURE; + + /** + * @return optional per-stream idle timeout for incoming connection manager connections. + */ + virtual const absl::optional streamIdleTimeout() PURE; /** * @return Router::RouteConfigProvider& the configuration provider used to acquire a route diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index f21aa0fab0be4..78b8e464a30cb 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -369,6 +369,13 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect // prevents surprises for logging code in edge cases. request_info_.setDownstreamRemoteAddress( connection_manager_.read_callbacks_->connection().remoteAddress()); + + if (connection_manager_.config_.streamIdleTimeout()) { + idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout().value(); + idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createTimer( + [this]() -> void { onIdleTimeout(); }); + resetIdleTimer(); + } } ConnectionManagerImpl::ActiveStream::~ActiveStream() { @@ -605,9 +612,10 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, const Router::RouteEntry* route_entry = cached_route_.value()->routeEntry(); if (route_entry != nullptr && route_entry->idleTimeout()) { idle_timeout_ms_ = route_entry->idleTimeout().value(); - idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createTimer( - [this]() -> void { onIdleTimeout(); }); - resetIdleTimer(); + if (idle_timer_ == nullptr) { + idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createTimer( + [this]() -> void { onIdleTimeout(); }); + } } } @@ -617,6 +625,9 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, } decodeHeaders(nullptr, *request_headers_, end_stream); + + // Reset it here for both global and overriden cases. + resetIdleTimer(); } void ConnectionManagerImpl::ActiveStream::traceRequest() { diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 94877655ebed0..3f1361aed5af8 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -258,8 +258,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, cluster_not_found_response_code_(ConfigUtility::parseClusterNotFoundResponseCode( route.route().cluster_not_found_response_code())), timeout_(PROTOBUF_GET_MS_OR_DEFAULT(route.route(), timeout, DEFAULT_ROUTE_TIMEOUT_MS)), - idle_timeout_( - PROTOBUF_GET_MS_OR_DEFAULT(route.route(), idle_timeout, DEFAULT_ROUTE_IDLE_TIMEOUT_MS)), + idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), idle_timeout)), max_grpc_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), max_grpc_timeout)), runtime_(loadRuntimeData(route.match())), loader_(factory_context.runtime()), host_redirect_(route.redirect().host_redirect()), diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index ac274dd28e7be..e5c79a6c62229 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -311,10 +311,7 @@ class RouteEntryImplBase : public RouteEntry, return vhost_.virtualClusterFromEntries(headers); } std::chrono::milliseconds timeout() const override { return timeout_; } - absl::optional idleTimeout() const override { - return idle_timeout_.count() == 0 ? absl::nullopt - : absl::optional(idle_timeout_); - } + absl::optional idleTimeout() const override { return idle_timeout_; } absl::optional maxGrpcTimeout() const override { return max_grpc_timeout_; } @@ -510,9 +507,6 @@ class RouteEntryImplBase : public RouteEntry, // Default timeout is 15s if nothing is specified in the route config. static const uint64_t DEFAULT_ROUTE_TIMEOUT_MS = 15000; - // Default idle timeout is 5 minutes if nothing is specified in the route config. - static const uint64_t DEFAULT_ROUTE_IDLE_TIMEOUT_MS = 5 * 60 * 1000; - std::unique_ptr cors_policy_; const VirtualHostImpl& vhost_; // See note in RouteEntryImplBase::clusterEntry() on why raw ref // to virtual host is currently safe. @@ -522,7 +516,7 @@ class RouteEntryImplBase : public RouteEntry, const Http::LowerCaseString cluster_header_name_; const Http::Code cluster_not_found_response_code_; const std::chrono::milliseconds timeout_; - const std::chrono::milliseconds idle_timeout_; + const absl::optional idle_timeout_; const absl::optional max_grpc_timeout_; const absl::optional runtime_; Runtime::Loader& loader_; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 476efe295c74e..9c613fc29c3d4 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -129,6 +129,9 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( route_config_provider_manager_(route_config_provider_manager), http2_settings_(Http::Utility::parseHttp2Settings(config.http2_protocol_options())), http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())), + idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(config, idle_timeout)), + stream_idle_timeout_( + PROTOBUF_GET_MS_OR_DEFAULT(config, stream_idle_timeout, StreamIdleTimeoutMs)), drain_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, drain_timeout, 5000)), generate_request_id_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, generate_request_id, true)), date_provider_(date_provider), @@ -216,10 +219,6 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( overall_sampling})); } - if (config.has_idle_timeout()) { - idle_timeout_ = std::chrono::milliseconds(PROTOBUF_GET_MS_REQUIRED(config, idle_timeout)); - } - for (const auto& access_log : config.access_log()) { AccessLog::InstanceSharedPtr current_access_log = AccessLog::AccessLogFactory::fromProto(access_log, context_); diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 9e5863b0efcb5..e355ee86d8bd2 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -87,7 +87,12 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::chrono::milliseconds drainTimeout() override { return drain_timeout_; } FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return generate_request_id_; } - const absl::optional& idleTimeout() override { return idle_timeout_; } + const absl::optional idleTimeout() override { return idle_timeout_; } + const absl::optional streamIdleTimeout() override { + return stream_idle_timeout_.count() == 0 + ? absl::nullopt + : absl::optional(stream_idle_timeout_); + } Router::RouteConfigProvider& routeConfigProvider() override { return *route_config_provider_; } const std::string& serverName() override { return server_name_; } Http::ConnectionManagerStats& stats() override { return stats_; } @@ -137,12 +142,16 @@ class HttpConnectionManagerConfig : Logger::Loggable, Http::TracingConnectionManagerConfigPtr tracing_config_; absl::optional user_agent_; absl::optional idle_timeout_; + std::chrono::milliseconds stream_idle_timeout_; Router::RouteConfigProviderSharedPtr route_config_provider_; std::chrono::milliseconds drain_timeout_; bool generate_request_id_; Http::DateProvider& date_provider_; Http::ConnectionManagerListenerStats listener_stats_; const bool proxy_100_continue_; + + // Default idle timeout is 5 minutes if nothing is specified in the HCM config. + static const uint64_t StreamIdleTimeoutMs = 5 * 60 * 1000; }; } // namespace HttpConnectionManager diff --git a/source/server/http/admin.h b/source/server/http/admin.h index e5e798851d49c..03890f7c21d30 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -89,7 +89,10 @@ class AdminImpl : public Admin, std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } Http::FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return false; } - const absl::optional& idleTimeout() override { return idle_timeout_; } + const absl::optional idleTimeout() override { return idle_timeout_; } + const absl::optional streamIdleTimeout() override { + return absl::nullopt; + } Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } const std::string& serverName() override { return Http::DefaultServerString::get(); } Http::ConnectionManagerStats& stats() override { return stats_; } diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 7b4b1a2007bd2..69f42b913a065 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -254,7 +254,10 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() override { return true; } - const absl::optional& idleTimeout() override { return idle_timeout_; } + const absl::optional idleTimeout() override { return idle_timeout_; } + const absl::optional streamIdleTimeout() override { + return stream_idle_timeout_; + } Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } const std::string& serverName() override { return server_name_; } ConnectionManagerStats& stats() override { return stats_; } @@ -294,6 +297,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi std::vector set_current_client_cert_details_; absl::optional user_agent_; absl::optional idle_timeout_; + absl::optional stream_idle_timeout_; NiceMock random_; NiceMock local_info_; NiceMock factory_context_; @@ -1093,7 +1097,9 @@ TEST_F(HttpConnectionManagerImplTest, NoPath) { conn_manager_->onData(fake_input, false); } -// No idle timeout when route idle timeout is not configured. +// No idle timeout when route idle timeout is implied at both global and +// per-route level. The connection manager config is responsible for managing +// the default configuration aspects. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutNotConfigured) { setup(false, ""); @@ -1115,6 +1121,61 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutNotConfigured) { EXPECT_EQ(0U, stats_.named_.downstream_rq_idle_timeout_.value()); } +// When the global timeout is configured, the timer is enabled before we receive +// headers. +TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutGlobal) { + stream_idle_timeout_ = std::chrono::milliseconds(10); + setup(false, ""); + + EXPECT_CALL(*codec_, dispatch(_)) + .Times(1) + .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> void { + Event::MockTimer* idle_timer = + new Event::MockTimer(&filter_callbacks_.connection_.dispatcher_); + EXPECT_CALL(*idle_timer, enableTimer(std::chrono::milliseconds(10))); + StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); + + HeaderMapPtr headers{new TestHeaderMapImpl{{":authority", "host"}, {":path", "/"}}}; + EXPECT_CALL(*idle_timer, enableTimer(std::chrono::milliseconds(10))); + decoder->decodeHeaders(std::move(headers), false); + + data.drain(4); + })); + + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); + + EXPECT_EQ(0U, stats_.named_.downstream_rq_idle_timeout_.value()); +} + +// Per-route timeouts override the global stream idle timeout. +TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutRouteOverride) { + stream_idle_timeout_ = std::chrono::milliseconds(10); + setup(false, ""); + ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) + .WillByDefault(Return(std::chrono::milliseconds(30))); + + EXPECT_CALL(*codec_, dispatch(_)) + .Times(1) + .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> void { + Event::MockTimer* idle_timer = + new Event::MockTimer(&filter_callbacks_.connection_.dispatcher_); + EXPECT_CALL(*idle_timer, enableTimer(std::chrono::milliseconds(10))); + StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); + + HeaderMapPtr headers{new TestHeaderMapImpl{{":authority", "host"}, {":path", "/"}}}; + EXPECT_CALL(*idle_timer, enableTimer(std::chrono::milliseconds(30))); + decoder->decodeHeaders(std::move(headers), false); + + data.drain(4); + })); + + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); + + EXPECT_EQ(0U, stats_.named_.downstream_rq_idle_timeout_.value()); +} + // Validate the per-stream idle timeout after having sent downstream headers. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterDownstreamHeaders) { setup(false, ""); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 83d7116ddfebd..d5ce42ebc05be 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -44,7 +44,8 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD0(drainTimeout, std::chrono::milliseconds()); MOCK_METHOD0(filterFactory, FilterChainFactory&()); MOCK_METHOD0(generateRequestId, bool()); - MOCK_METHOD0(idleTimeout, const absl::optional&()); + MOCK_METHOD0(idleTimeout, const absl::optional()); + MOCK_METHOD0(streamIdleTimeout, const absl::optional()); MOCK_METHOD0(routeConfigProvider, Router::RouteConfigProvider&()); MOCK_METHOD0(serverName, const std::string&()); MOCK_METHOD0(stats, ConnectionManagerStats&()); diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 819f4221731e5..aa7b07861de34 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -35,6 +35,14 @@ parseHttpConnectionManagerFromJson(const std::string& json_string) { return http_connection_manager; } +envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager +parseHttpConnectionManagerFromV2Yaml(const std::string& yaml) { + envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager + http_connection_manager; + MessageUtil::loadFromYaml(yaml, http_connection_manager); + return http_connection_manager; +} + class HttpConnectionManagerConfigTest : public testing::Test { public: NiceMock context_; @@ -120,6 +128,23 @@ TEST_F(HttpConnectionManagerConfigTest, MiscConfig) { ContainerEq(config.tracingConfig()->request_headers_for_tags_)); EXPECT_EQ(*context_.local_info_.address_, config.localAddress()); EXPECT_EQ("foo", config.serverName()); + EXPECT_EQ(5 * 60 * 1000, config.streamIdleTimeout().value().count()); +} + +// Validated that an explicit zero stream idle timeout disables. +TEST_F(HttpConnectionManagerConfigTest, DisabledStreamIdleTimeout) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + stream_idle_timeout: 0s + route_config: + name: local_route + http_filters: + - name: envoy.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_); + EXPECT_FALSE(config.streamIdleTimeout()); } TEST_F(HttpConnectionManagerConfigTest, SingleDateProvider) { diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index 97b71aefbb139..aa15e6d5ad633 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -9,11 +9,17 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { config_helper_.addConfigModifier( [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) -> void { - auto* route_config = hcm.mutable_route_config(); - auto* virtual_host = route_config->mutable_virtual_hosts(0); - auto* route = virtual_host->mutable_routes(0)->mutable_route(); - route->mutable_idle_timeout()->set_seconds(0); - route->mutable_idle_timeout()->set_nanos(TimeoutMs * 1000 * 1000); + if (enable_global_idle_timeout_) { + hcm.mutable_stream_idle_timeout()->set_seconds(0); + hcm.mutable_stream_idle_timeout()->set_nanos(TimeoutMs * 1000 * 1000); + } + if (enable_per_stream_idle_timeout_) { + auto* route_config = hcm.mutable_route_config(); + auto* virtual_host = route_config->mutable_virtual_hosts(0); + auto* route = virtual_host->mutable_routes(0)->mutable_route(); + route->mutable_idle_timeout()->set_seconds(0); + route->mutable_idle_timeout()->set_nanos(TimeoutMs * 1000 * 1000); + } // For validating encode100ContinueHeaders() timer kick. hcm.set_proxy_100_continue(true); }); @@ -52,6 +58,8 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { // TODO(htuch): This might require scaling for TSAN/ASAN/Valgrind/etc. Bump if // this is the cause of flakes. static constexpr uint64_t TimeoutMs = 200; + bool enable_global_idle_timeout_{}; + bool enable_per_stream_idle_timeout_{true}; }; INSTANTIATE_TEST_CASE_P(Protocols, IdleTimeoutIntegrationTest, @@ -71,6 +79,21 @@ TEST_P(IdleTimeoutIntegrationTest, PerStreamIdleTimeoutAfterDownstreamHeaders) { EXPECT_EQ("stream timeout", response->body()); } +// Global per-stream idle timeout applies if there is no per-stream idle timeout. +TEST_P(IdleTimeoutIntegrationTest, GlobalPerStreamIdleTimeoutAfterDownstreamHeaders) { + enable_global_idle_timeout_ = true; + enable_per_stream_idle_timeout_ = false; + auto response = setupPerStreamIdleTimeoutTest(); + + waitForTimeout(*response); + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("408", response->headers().Status()->value().c_str()); + EXPECT_EQ("stream timeout", response->body()); +} + // Per-stream idle timeout after having sent downstream headers+body. TEST_P(IdleTimeoutIntegrationTest, PerStreamIdleTimeoutAfterDownstreamHeadersAndBody) { auto response = setupPerStreamIdleTimeoutTest(); From a30818944bed86c6e974a7d9a50ec5f5dcd97d2c Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 18 Jul 2018 12:38:21 -0400 Subject: [PATCH 2/6] Review feedback. Signed-off-by: Harvey Tuch --- api/envoy/api/v2/route/route.proto | 8 ++--- .../v2/http_connection_manager.proto | 10 ++++-- include/envoy/router/router.h | 3 +- source/common/http/conn_manager_config.h | 7 ++-- source/common/http/conn_manager_impl.cc | 15 +++++--- .../network/http_connection_manager/config.h | 8 ++--- source/server/http/admin.h | 6 ++-- test/common/http/conn_manager_impl_test.cc | 36 ++++++++++++++++--- test/common/http/conn_manager_utility_test.cc | 4 +-- test/common/router/config_impl_test.cc | 14 ++++---- 10 files changed, 72 insertions(+), 39 deletions(-) diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index 6bcc7b1e6669e..0a3bb20ee7491 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -439,11 +439,11 @@ message RouteAction { google.protobuf.Duration per_try_timeout = 3 [(gogoproto.stdduration) = true]; } - // Specifies the idle timeout for the route. If not specified, there is no per-route - // idle timeout specified, although the connection manager wide - // :ref:`stream_idle_timeout + // Specifies the idle timeout for the route. If not specified, there is no per-route idle timeout + // specified, although the connection manager wide :ref:`stream_idle_timeout // ` - // will still apply. + // will still apply. A value of 0 will completely disable the route's idle timeout, even if a + // connection manager stream idle timeout is configured. // // The idle timeout is distinct to :ref:`timeout // `, which provides an upper bound diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index d9e8c02cc3b57..1e6999cf2077a 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -138,10 +138,10 @@ message HttpConnectionManager { google.protobuf.Duration idle_timeout = 11 [(gogoproto.stdduration) = true]; // The stream idle timeout for connections managed by the connection manager. - // If not specified, this defaults to 5 minutes. The default value was select + // If not specified, this defaults to 5 minutes. The default value was selected // so as not to interfere with any smaller configured timeouts that may have // existed in configurations prior to the introduction of this feature, while - // introducing robustness to TCP connections that terminate without FIN. + // introducing robustness to TCP connections that terminate without a FIN. // // This idle timeout applies to new streams and is overridable by the // :ref:`route-level idle_timeout @@ -154,6 +154,12 @@ message HttpConnectionManager { // is terminated with a 408 Request Timeout error code if no upstream response // header has been received, otherwise a stream reset occurs. // + // Note that it is possible to idle timeout even if the wire traffic for a stream is non-idle, due + // to the granularity of events presented to the connection manager. For example, while receiving + // very large request headers, it may be the case that there is traffic regularly arriving on the + // wire while the connection manage is only able to observe the end-of-headers event, hence the + // stream may still idle timeout. + // // A value of 0 will completely disable the connection manager stream idle // timeout, although per-route idle timeout overrides will continue to apply. google.protobuf.Duration stream_idle_timeout = 24 [(gogoproto.stdduration) = true]; diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 9aa244f29d051..a69c402251910 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -473,7 +473,8 @@ class RouteEntry : public ResponseEntry { virtual std::chrono::milliseconds timeout() const PURE; /** - * @return absl::optional the route's idle timeout. + * @return optional the route's idle timeout. Zero indicates a + * disabled idle timeout, while nullopt indicates deference to the global timeout. */ virtual absl::optional idleTimeout() const PURE; diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 0efec40e76682..7c172903f7a44 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -192,12 +192,13 @@ class ConnectionManagerConfig { /** * @return optional idle timeout for incoming connection manager connections. */ - virtual const absl::optional idleTimeout() PURE; + virtual absl::optional idleTimeout() const PURE; /** - * @return optional per-stream idle timeout for incoming connection manager connections. + * @return per-stream idle timeout for incoming connection manager connections. Zero indicates a + * disabled idle timeout. */ - virtual const absl::optional streamIdleTimeout() PURE; + virtual std::chrono::milliseconds streamIdleTimeout() const PURE; /** * @return Router::RouteConfigProvider& the configuration provider used to acquire a route diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index e2284105c5811..7499e5f3b0a16 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -370,8 +370,8 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect request_info_.setDownstreamRemoteAddress( connection_manager_.read_callbacks_->connection().remoteAddress()); - if (connection_manager_.config_.streamIdleTimeout()) { - idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout().value(); + if (connection_manager_.config_.streamIdleTimeout().count()) { + idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout(); idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createTimer( [this]() -> void { onIdleTimeout(); }); resetIdleTimer(); @@ -612,9 +612,14 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, const Router::RouteEntry* route_entry = cached_route_.value()->routeEntry(); if (route_entry != nullptr && route_entry->idleTimeout()) { idle_timeout_ms_ = route_entry->idleTimeout().value(); - if (idle_timer_ == nullptr) { - idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createTimer( - [this]() -> void { onIdleTimeout(); }); + if (idle_timeout_ms_.count()) { + if (idle_timer_ == nullptr) { + idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createTimer( + [this]() -> void { onIdleTimeout(); }); + } + } else if (idle_timer_ != nullptr) { + idle_timer_->disableTimer(); + idle_timer_ = nullptr; } } } diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index e355ee86d8bd2..eddbd12f77c4c 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -87,12 +87,8 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::chrono::milliseconds drainTimeout() override { return drain_timeout_; } FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return generate_request_id_; } - const absl::optional idleTimeout() override { return idle_timeout_; } - const absl::optional streamIdleTimeout() override { - return stream_idle_timeout_.count() == 0 - ? absl::nullopt - : absl::optional(stream_idle_timeout_); - } + absl::optional idleTimeout() const override { return idle_timeout_; } + std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } Router::RouteConfigProvider& routeConfigProvider() override { return *route_config_provider_; } const std::string& serverName() override { return server_name_; } Http::ConnectionManagerStats& stats() override { return stats_; } diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 03890f7c21d30..b16a58e2b2858 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -89,10 +89,8 @@ class AdminImpl : public Admin, std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } Http::FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return false; } - const absl::optional idleTimeout() override { return idle_timeout_; } - const absl::optional streamIdleTimeout() override { - return absl::nullopt; - } + absl::optional idleTimeout() const override { return idle_timeout_; } + std::chrono::milliseconds streamIdleTimeout() const override { return {}; } Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } const std::string& serverName() override { return Http::DefaultServerString::get(); } Http::ConnectionManagerStats& stats() override { return stats_; } diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 69f42b913a065..10ed29faa0ef5 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -254,10 +254,8 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() override { return true; } - const absl::optional idleTimeout() override { return idle_timeout_; } - const absl::optional streamIdleTimeout() override { - return stream_idle_timeout_; - } + absl::optional idleTimeout() const override { return idle_timeout_; } + std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } const std::string& serverName() override { return server_name_; } ConnectionManagerStats& stats() override { return stats_; } @@ -297,7 +295,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi std::vector set_current_client_cert_details_; absl::optional user_agent_; absl::optional idle_timeout_; - absl::optional stream_idle_timeout_; + std::chrono::milliseconds stream_idle_timeout_{}; NiceMock random_; NiceMock local_info_; NiceMock factory_context_; @@ -1176,6 +1174,34 @@ TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutRouteOverride) { EXPECT_EQ(0U, stats_.named_.downstream_rq_idle_timeout_.value()); } +// Per-route zero timeout overrides the global stream idle timeout. +TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutRouteZeroOverride) { + stream_idle_timeout_ = std::chrono::milliseconds(10); + setup(false, ""); + ON_CALL(route_config_provider_.route_config_->route_->route_entry_, idleTimeout()) + .WillByDefault(Return(std::chrono::milliseconds(0))); + + EXPECT_CALL(*codec_, dispatch(_)) + .Times(1) + .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> void { + Event::MockTimer* idle_timer = + new Event::MockTimer(&filter_callbacks_.connection_.dispatcher_); + EXPECT_CALL(*idle_timer, enableTimer(std::chrono::milliseconds(10))); + StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); + + HeaderMapPtr headers{new TestHeaderMapImpl{{":authority", "host"}, {":path", "/"}}}; + EXPECT_CALL(*idle_timer, disableTimer()); + decoder->decodeHeaders(std::move(headers), false); + + data.drain(4); + })); + + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); + + EXPECT_EQ(0U, stats_.named_.downstream_rq_idle_timeout_.value()); +} + // Validate the per-stream idle timeout after having sent downstream headers. TEST_F(HttpConnectionManagerImplTest, PerStreamIdleTimeoutAfterDownstreamHeaders) { setup(false, ""); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index d5ce42ebc05be..91780c603c1d2 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -44,8 +44,8 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD0(drainTimeout, std::chrono::milliseconds()); MOCK_METHOD0(filterFactory, FilterChainFactory&()); MOCK_METHOD0(generateRequestId, bool()); - MOCK_METHOD0(idleTimeout, const absl::optional()); - MOCK_METHOD0(streamIdleTimeout, const absl::optional()); + MOCK_CONST_METHOD0(idleTimeout, absl::optional()); + MOCK_CONST_METHOD0(streamIdleTimeout, std::chrono::milliseconds()); MOCK_METHOD0(routeConfigProvider, Router::RouteConfigProvider&()); MOCK_METHOD0(serverName, const std::string&()); MOCK_METHOD0(stats, ConnectionManagerStats&()); diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 5596dc2553199..5cd2e4bae4604 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -4383,7 +4383,6 @@ name: NoIdleTimeout - match: { regex: "/regex"} route: cluster: some-cluster - idle_timeout: 0s )EOF"; NiceMock factory_context; @@ -4393,9 +4392,9 @@ name: NoIdleTimeout EXPECT_EQ(absl::nullopt, route_entry->idleTimeout()); } -TEST(RouteConfigurationV2, DefaultIdleTimeout) { - const std::string DefaultIdleTimeot = R"EOF( -name: NoIdleTimeout +TEST(RouteConfigurationV2, ZeroIdleTimeout) { + const std::string ZeroIdleTimeot = R"EOF( +name: ZeroIdleTimeout virtual_hosts: - name: regex domains: [idle.lyft.com] @@ -4403,18 +4402,19 @@ name: NoIdleTimeout - match: { regex: "/regex"} route: cluster: some-cluster + idle_timeout: 0s )EOF"; NiceMock factory_context; - ConfigImpl config(parseRouteConfigurationFromV2Yaml(DefaultIdleTimeot), factory_context, true); + ConfigImpl config(parseRouteConfigurationFromV2Yaml(ZeroIdleTimeot), factory_context, true); Http::TestHeaderMapImpl headers = genRedirectHeaders("idle.lyft.com", "/regex", true, false); const RouteEntry* route_entry = config.route(headers, 0)->routeEntry(); - EXPECT_EQ(5 * 60 * 1000, route_entry->idleTimeout().value().count()); + EXPECT_EQ(0, route_entry->idleTimeout().value().count()); } TEST(RouteConfigurationV2, ExplicitIdleTimeout) { const std::string ExplicitIdleTimeot = R"EOF( -name: NoIdleTimeout +name: ExplicitIdleTimeout virtual_hosts: - name: regex domains: [idle.lyft.com] From 1407f36549cc6062d62f50dde3935ff3e51987c4 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 18 Jul 2018 13:54:55 -0400 Subject: [PATCH 3/6] Fix TSAN race with set_allow_unexpected_disconnects. Signed-off-by: Harvey Tuch --- test/integration/idle_timeout_integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index aa15e6d5ad633..6aa5fa3b82cdf 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -28,6 +28,7 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { IntegrationStreamDecoderPtr setupPerStreamIdleTimeoutTest() { initialize(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); auto encoder_decoder = codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "GET"}, @@ -36,7 +37,6 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { {":authority", "host"}}); request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - fake_upstreams_[0]->set_allow_unexpected_disconnects(true); fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); upstream_request_->waitForHeadersComplete(); From f0736fc9efcaaf7d223cea5eaf7a0a2e1904be7c Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 18 Jul 2018 13:57:56 -0400 Subject: [PATCH 4/6] More test fixes. Signed-off-by: Harvey Tuch --- .../filters/network/http_connection_manager/config_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index aa7b07861de34..1fc646bdba6f0 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -128,7 +128,7 @@ TEST_F(HttpConnectionManagerConfigTest, MiscConfig) { ContainerEq(config.tracingConfig()->request_headers_for_tags_)); EXPECT_EQ(*context_.local_info_.address_, config.localAddress()); EXPECT_EQ("foo", config.serverName()); - EXPECT_EQ(5 * 60 * 1000, config.streamIdleTimeout().value().count()); + EXPECT_EQ(5 * 60 * 1000, config.streamIdleTimeout().count()); } // Validated that an explicit zero stream idle timeout disables. @@ -144,7 +144,7 @@ TEST_F(HttpConnectionManagerConfigTest, DisabledStreamIdleTimeout) { HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, date_provider_, route_config_provider_manager_); - EXPECT_FALSE(config.streamIdleTimeout()); + EXPECT_EQ(0, config.streamIdleTimeout().count()); } TEST_F(HttpConnectionManagerConfigTest, SingleDateProvider) { From 3f039a5345e1c3048d8a0249064e926792b6d458 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 19 Jul 2018 14:24:11 -0400 Subject: [PATCH 5/6] Additional comments. Signed-off-by: Harvey Tuch --- source/common/http/conn_manager_impl.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 7499e5f3b0a16..aeadafa948ff2 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -613,11 +613,14 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, if (route_entry != nullptr && route_entry->idleTimeout()) { idle_timeout_ms_ = route_entry->idleTimeout().value(); if (idle_timeout_ms_.count()) { + // If we have a route-level idle timeout but no global stream idle timeout, create a timer. if (idle_timer_ == nullptr) { idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createTimer( [this]() -> void { onIdleTimeout(); }); } } else if (idle_timer_ != nullptr) { + // If we had a global stream idle timeout but the route-level idle timeout is set to zero + // (to override), we disable the idle timer. idle_timer_->disableTimer(); idle_timer_ = nullptr; } From 528b32b1841e8be3d5ffa0a869c9f0cf2f824124 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 19 Jul 2018 14:33:34 -0400 Subject: [PATCH 6/6] Typo. Signed-off-by: Harvey Tuch --- docs/root/intro/arch_overview/http_connection_management.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/intro/arch_overview/http_connection_management.rst b/docs/root/intro/arch_overview/http_connection_management.rst index b7c8f9dc0dad8..40415481322fc 100644 --- a/docs/root/intro/arch_overview/http_connection_management.rst +++ b/docs/root/intro/arch_overview/http_connection_management.rst @@ -55,7 +55,7 @@ Various configurable timeouts apply to an HTTP connection and its constituent st `: this spans between an Envoy originated GOAWAY and connection termination. * Stream-level idle timeout: this applies to each individual stream. It may be configured at both - the :ref:`connection manage + the :ref:`connection manager ` and :ref:`per-route ` granularity. Header/data/trailer events on the stream reset the idle timeout.