Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
f143d0c
conn_pool: support max stream duration for upstream connections
esmet Feb 17, 2021
4e8e73d
Formatting
esmet Feb 17, 2021
9d7d810
Only drain non-closed connections
esmet Feb 17, 2021
0964e31
Pipe down maxConnectionDuration from CommonHttpProtocolOptions
esmet Feb 18, 2021
7909f3f
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Aug 31, 2021
301e2a0
Fix proto comments
esmet Aug 31, 2021
4a776fe
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Aug 31, 2021
d61139e
Fix ClusterInfo to contain maxConnectionDuration
esmet Aug 31, 2021
8276bb6
Add tests
esmet Aug 31, 2021
51ecc21
Fix
esmet Sep 1, 2021
3abf8cd
Add back some bits
esmet Sep 2, 2021
e65ce1d
Fix changelog
esmet Sep 2, 2021
be1493b
Update max connection duration tests to use a real dispatcher and sim…
esmet Sep 6, 2021
6934dff
Fix format
esmet Sep 6, 2021
7976234
Add back two expects
esmet Sep 6, 2021
e0b793a
Remove the addition of closed_, since the lifetime of that bit is not…
esmet Sep 7, 2021
adf76d8
Fix format
esmet Sep 7, 2021
53963b4
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Sep 7, 2021
5ce63a6
Fix test
esmet Sep 7, 2021
35e6e70
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Sep 9, 2021
477d857
Factor out common test functionality.
esmet Sep 21, 2021
34eb063
Continue to factor out test mechanics from test cases
esmet Sep 21, 2021
e87d39c
Formatting
esmet Sep 21, 2021
0c4a16c
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Sep 21, 2021
a4aecec
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Sep 23, 2021
5b155f8
Fix typo
esmet Sep 23, 2021
90391aa
Refactor upstream_impl_test.cc
esmet Sep 23, 2021
f676853
Test improvements
esmet Sep 23, 2021
374e1c6
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Sep 24, 2021
24afd92
Fix changelog ordering
esmet Sep 24, 2021
e29ead0
Style comments
esmet Sep 26, 2021
c8882f2
FOrmatting
esmet Sep 26, 2021
8605363
ASSERT -> ENVOY_BUG
esmet Sep 27, 2021
e805385
Add doc for upstream_cx_max_duration_exceeded
esmet Sep 27, 2021
05e4599
Update tests
esmet Sep 27, 2021
59fccef
Add field reflink to changelog. Clarify docs to mention that drain_ti…
esmet Sep 28, 2021
1e95ac2
Fix spelling
esmet Sep 28, 2021
c3d16f1
Fix spacing
esmet Sep 28, 2021
4ab0322
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Sep 29, 2021
54117b5
Naming fixups
esmet Sep 30, 2021
187d823
Formatting
esmet Sep 30, 2021
ce6fa32
More formatting
esmet Sep 30, 2021
8637e46
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Oct 7, 2021
097525a
Fix merge on changelog
esmet Oct 7, 2021
0c64dcb
Add a basic integration test
esmet Oct 7, 2021
c0f413d
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Oct 7, 2021
230a177
Fix format
esmet Oct 7, 2021
926989c
Remove unused boilerplate
esmet Oct 7, 2021
8b99184
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Oct 7, 2021
5b23fb5
Fix changelog merge
esmet Oct 7, 2021
6474914
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Oct 13, 2021
ca9fa76
Fix changelog
esmet Oct 13, 2021
2bbef6c
Merge remote-tracking branch 'upstream/main' into max-upstream-connec…
esmet Oct 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion api/envoy/config/core/v3/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ message HttpProtocolOptions {
// the connection will be closed. Drain sequence will occur prior to closing the connection if
// if's applicable. See :ref:`drain_timeout
// <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.drain_timeout>`.
// Note: not implemented for upstream connections.
google.protobuf.Duration max_connection_duration = 3;

// The maximum number of headers. If unconfigured, the default
Expand Down
5 changes: 5 additions & 0 deletions envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,11 @@ class ClusterInfo {
*/
virtual const absl::optional<std::chrono::milliseconds> idleTimeout() const PURE;

/**
* @return optional maximum connection duration timeout for manager connections.
*/
virtual const absl::optional<std::chrono::milliseconds> maxConnectionDuration() const PURE;

/**
* @return how many streams should be anticipated per each current stream.
*/
Expand Down
1 change: 0 additions & 1 deletion generated_api_shadow/envoy/config/core/v3/protocol.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions source/common/conn_pool/conn_pool_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,11 @@ ActiveClient::ActiveClient(ConnPoolImplBase& parent, uint32_t lifetime_stream_li
conn_length_ = std::make_unique<Stats::HistogramCompletableTimespanImpl>(
parent_.host()->cluster().stats().upstream_cx_length_ms_, parent_.dispatcher().timeSource());
connect_timer_->enableTimer(parent_.host()->cluster().connectTimeout());
const auto max_connection_duration = parent_.host()->cluster().maxConnectionDuration();
if (max_connection_duration) {
lifetime_timer_ = parent_.dispatcher().createTimer([this]() -> void { onLifetimeTimeout(); });
lifetime_timer_->enableTimer(max_connection_duration.value());
}
parent_.host()->stats().cx_total_.inc();
parent_.host()->stats().cx_active_.inc();
parent_.host()->cluster().stats().upstream_cx_total_.inc();
Expand Down Expand Up @@ -596,6 +601,15 @@ void ActiveClient::onConnectTimeout() {
close();
}

void ActiveClient::onLifetimeTimeout() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

onConnectionDurationTimeout would be more consistent. ditto on the timer and the logs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I originally wanted to be consistent with the max streams variable name (which refers to "lifetime") but it probably makes more sense to be consistent with the API field names.

if (state_ != ActiveClient::State::CLOSED) {
ENVOY_CONN_LOG(debug, "lifetime timeout, DRAINING", *this);
parent_.host()->cluster().stats().upstream_cx_max_duration_.inc();
parent_.transitionActiveClientState(*this,
Envoy::ConnectionPool::ActiveClient::State::DRAINING);
}
}

void ActiveClient::drain() {
if (currentUnusedCapacity() <= 0) {
return;
Expand Down
5 changes: 5 additions & 0 deletions source/common/conn_pool/conn_pool_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ class ActiveClient : public LinkedObject<ActiveClient>,
// Called if the connection does not complete within the cluster's connectTimeout()
void onConnectTimeout();

// Called if the maximum connection duration is reached. If set, this puts an upper
// bound on the lifetime of any connection.
void onLifetimeTimeout();

// Returns the concurrent stream limit, accounting for if the total stream limit
// is less than the concurrent stream limit.
uint32_t effectiveConcurrentStreamLimit() const {
Expand Down Expand Up @@ -106,6 +110,7 @@ class ActiveClient : public LinkedObject<ActiveClient>,
Stats::TimespanPtr conn_connect_ms_;
Stats::TimespanPtr conn_length_;
Event::TimerPtr connect_timer_;
Event::TimerPtr lifetime_timer_;
bool resources_released_{false};
bool timed_out_{false};

Expand Down
4 changes: 4 additions & 0 deletions source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,9 @@ class ClusterInfoImpl : public ClusterInfo,
const absl::optional<std::chrono::milliseconds> idleTimeout() const override {
return idle_timeout_;
}
const absl::optional<std::chrono::milliseconds> maxConnectionDuration() const override {
return max_connection_duration_;
}
float perUpstreamPreconnectRatio() const override { return per_upstream_preconnect_ratio_; }
float peekaheadRatio() const override { return peekahead_ratio_; }
uint32_t perConnectionBufferLimitBytes() const override {
Expand Down Expand Up @@ -765,6 +768,7 @@ class ClusterInfoImpl : public ClusterInfo,
const uint32_t max_response_headers_count_;
const std::chrono::milliseconds connect_timeout_;
absl::optional<std::chrono::milliseconds> idle_timeout_;
absl::optional<std::chrono::milliseconds> max_connection_duration_;
const float per_upstream_preconnect_ratio_;
const float peekahead_ratio_;
const uint32_t per_connection_buffer_limit_bytes_;
Expand Down
7 changes: 7 additions & 0 deletions test/mocks/upstream/cluster_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ MockIdleTimeEnabledClusterInfo::MockIdleTimeEnabledClusterInfo() {

MockIdleTimeEnabledClusterInfo::~MockIdleTimeEnabledClusterInfo() = default;

MockMaxConnectionDurationEnabledClusterInfo::MockMaxConnectionDurationEnabledClusterInfo() {
ON_CALL(*this, maxConnectionDuration()).WillByDefault(Return(std::chrono::milliseconds(1000)));
}

MockMaxConnectionDurationEnabledClusterInfo::~MockMaxConnectionDurationEnabledClusterInfo() =
default;

MockClusterInfo::MockClusterInfo()
: http2_options_(::Envoy::Http2::Utility::initializeAndValidateOptions(
envoy::config::core::v3::Http2ProtocolOptions())),
Expand Down
7 changes: 7 additions & 0 deletions test/mocks/upstream/cluster_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class MockClusterInfo : public ClusterInfo {
MOCK_METHOD(bool, addedViaApi, (), (const));
MOCK_METHOD(std::chrono::milliseconds, connectTimeout, (), (const));
MOCK_METHOD(const absl::optional<std::chrono::milliseconds>, idleTimeout, (), (const));
MOCK_METHOD(const absl::optional<std::chrono::milliseconds>, maxConnectionDuration, (), (const));
MOCK_METHOD(const absl::optional<std::chrono::milliseconds>, maxStreamDuration, (), (const));
MOCK_METHOD(const absl::optional<std::chrono::milliseconds>, grpcTimeoutHeaderMax, (), (const));
MOCK_METHOD(const absl::optional<std::chrono::milliseconds>, grpcTimeoutHeaderOffset, (),
Expand Down Expand Up @@ -215,5 +216,11 @@ class MockIdleTimeEnabledClusterInfo : public MockClusterInfo {
~MockIdleTimeEnabledClusterInfo() override;
};

class MockMaxConnectionDurationEnabledClusterInfo : public MockClusterInfo {
public:
MockMaxConnectionDurationEnabledClusterInfo();
~MockMaxConnectionDurationEnabledClusterInfo() override;
};

} // namespace Upstream
} // namespace Envoy