Skip to content

Commit 5a363b3

Browse files
authored
http: wire ConnectionLifetimeCallbacks into pool connection events (#43702)
## Commit Message http: wire ConnectionLifetimeCallbacks into pool connection events ## Description Wires up the `ConnectionLifetimeCallbacks` interface that is there but is not connected to actual connection events from what I see at least. The cluster manager now passes the LB's callbacks into each HTTP pool at creation time, and `MultiplexedActiveClientBase` fires them on Connected, Close, and GOAWAY events. Primary motivation: allows the reverse tunnel cluster's LB to detect GOAWAY and report it to the tunnel reporter ([raw example](aakugan@31094c9)). Also applicable to DFP clusters for connection coalescing awareness from what I [see](#17874). ## Risk Level Should be low. Additive-only changes to production code. Callbacks are no-ops unless explicitly set by the LB. No behavior change for existing clusters. ## Testing - HTTP/2 conn pool unit tests (7 tests): no-op without callbacks, Connected/RemoteClose/LocalClose with arg verification, GOAWAY with active streams, idle GOAWAY, callback replacement. - HTTP/3 conn pool unit test: Connected + idle GOAWAY through `MultiplexedActiveClientBase`. - Cluster manager unit tests: verifies `setLifetimeCallbacks` is called on pool creation. ## Docs Changes N/A ## Release Notes N/A --------- Signed-off-by: aakugan <aakashganapathy2@gmail.com>
1 parent a158136 commit 5a363b3

15 files changed

Lines changed: 578 additions & 0 deletions

envoy/http/conn_pool.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include <functional>
44
#include <memory>
5+
#include <vector>
56

67
#include "envoy/common/conn_pool.h"
78
#include "envoy/common/pure.h"
@@ -119,6 +120,9 @@ class Instance : public Envoy::ConnectionPool::Instance, public Event::DeferredD
119120
* @return absl::string_view a protocol description for logging.
120121
*/
121122
virtual absl::string_view protocolDescription() const PURE;
123+
124+
virtual void setLifetimeCallbacks(OptRef<ConnectionLifetimeCallbacks> callbacks,
125+
std::vector<uint8_t> hash_key) PURE;
122126
};
123127

124128
using InstancePtr = std::unique_ptr<Instance>;

source/common/http/codec_client.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,9 @@ class CodecClient : protected Logger::Loggable<Logger::Id::client>,
155155

156156
bool connectCalled() const { return connect_called_; }
157157

158+
// Get the underlying connection of the codec client for lifetimeCallbacks.
159+
const Network::Connection& connection() const { return *connection_; }
160+
158161
protected:
159162
/**
160163
* Create a codec client and connect to a remote host/port.

source/common/http/conn_pool_base.cc

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,24 @@ void HttpConnPoolImplBase::onPoolReady(Envoy::ConnectionPool::ActiveClient& clie
104104
http_client->codec_client_->protocol());
105105
}
106106

107+
void HttpConnPoolImplBase::setLifetimeCallbacks(
108+
OptRef<ConnectionPool::ConnectionLifetimeCallbacks> callbacks, std::vector<uint8_t> hash_key) {
109+
callbacks_ = callbacks;
110+
hash_key_ = std::move(hash_key);
111+
}
112+
113+
void HttpConnPoolImplBase::onConnectionOpen(const Network::Connection& connection) {
114+
if (callbacks_.has_value()) {
115+
callbacks_->onConnectionOpen(*this, hash_key_, connection);
116+
}
117+
}
118+
119+
void HttpConnPoolImplBase::onConnectionDraining(const Network::Connection& connection) {
120+
if (callbacks_.has_value()) {
121+
callbacks_->onConnectionDraining(*this, hash_key_, connection);
122+
}
123+
}
124+
107125
// All streams are 2^31. Client streams are half that, minus stream 0. Just to be on the safe
108126
// side we do 2^29.
109127
constexpr uint32_t DEFAULT_MAX_STREAMS = 1U << 29;
@@ -115,6 +133,7 @@ void MultiplexedActiveClientBase::onGoAway(Http::GoAwayErrorCode) {
115133
if (codec_client_->numActiveRequests() == 0) {
116134
codec_client_->close();
117135
} else {
136+
parent().onConnectionDraining(codec_client_->connection());
118137
parent_.transitionActiveClientState(*this, ActiveClient::State::Draining);
119138
}
120139
}
@@ -222,5 +241,17 @@ MultiplexedActiveClientBase::newStreamEncoder(ResponseDecoderHandlePtr response_
222241
return codec_client_->newStream(std::move(response_decoder_handle));
223242
}
224243

244+
void MultiplexedActiveClientBase::onEvent(Network::ConnectionEvent event) {
245+
if (event == Network::ConnectionEvent::Connected ||
246+
event == Network::ConnectionEvent::ConnectedZeroRtt) {
247+
parent().onConnectionOpen(codec_client_->connection());
248+
} else if (event == Network::ConnectionEvent::LocalClose ||
249+
event == Network::ConnectionEvent::RemoteClose) {
250+
parent().onConnectionDraining(codec_client_->connection());
251+
}
252+
253+
ActiveClient::onEvent(event);
254+
}
255+
225256
} // namespace Http
226257
} // namespace Envoy

source/common/http/conn_pool_base.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,13 @@ class HttpConnPoolImplBase : public Envoy::ConnectionPool::ConnPoolImplBase,
9898
virtual absl::optional<HttpServerPropertiesCache::Origin>& origin() { return origin_; }
9999
virtual Http::HttpServerPropertiesCacheSharedPtr cache() { return nullptr; }
100100

101+
void setLifetimeCallbacks(OptRef<ConnectionPool::ConnectionLifetimeCallbacks> callbacks,
102+
std::vector<uint8_t> hash_key) override;
103+
104+
void onConnectionOpen(const Network::Connection& connection);
105+
106+
void onConnectionDraining(const Network::Connection& connection);
107+
101108
protected:
102109
friend class ActiveClient;
103110

@@ -107,6 +114,8 @@ class HttpConnPoolImplBase : public Envoy::ConnectionPool::ConnPoolImplBase,
107114

108115
private:
109116
absl::optional<HttpServerPropertiesCache::Origin> origin_;
117+
OptRef<ConnectionPool::ConnectionLifetimeCallbacks> callbacks_;
118+
std::vector<uint8_t> hash_key_;
110119
};
111120

112121
// An implementation of Envoy::ConnectionPool::ActiveClient for HTTP/1.1 and HTTP/2
@@ -242,6 +251,9 @@ class MultiplexedActiveClientBase : public CodecClientCallbacks,
242251
void onGoAway(Http::GoAwayErrorCode error_code) override;
243252
void onSettings(ReceivedSettings& settings) override;
244253

254+
// Override to provide the lifetimeCallbacks.
255+
void onEvent(Network::ConnectionEvent event) override;
256+
245257
private:
246258
bool closed_with_active_rq_{};
247259
};

source/common/http/conn_pool_grid.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,7 @@ ConnectionPool::InstancePtr ConnectivityGrid::createHttp3Pool(bool attempt_alter
417417

418418
void ConnectivityGrid::setupPool(ConnectionPool::Instance& pool) {
419419
pool.addIdleCallback([this]() { onIdleReceived(); });
420+
pool.setLifetimeCallbacks(makeOptRefFromPtr(this), hash_key_);
420421
}
421422

422423
bool ConnectivityGrid::hasActiveConnections() const {
@@ -610,5 +611,25 @@ void ConnectivityGrid::onZeroRttHandshakeFailed() {
610611
getHttp3StatusTracker().markHttp3FailedRecently();
611612
}
612613

614+
void ConnectivityGrid::onConnectionOpen(ConnectionPool::Instance&, std::vector<uint8_t>&,
615+
const Network::Connection& connection) {
616+
if (callbacks_.has_value()) {
617+
callbacks_->onConnectionOpen(*this, hash_key_, connection);
618+
}
619+
}
620+
621+
void ConnectivityGrid::onConnectionDraining(ConnectionPool::Instance&, std::vector<uint8_t>&,
622+
const Network::Connection& connection) {
623+
if (callbacks_.has_value()) {
624+
callbacks_->onConnectionDraining(*this, hash_key_, connection);
625+
}
626+
}
627+
628+
void ConnectivityGrid::setLifetimeCallbacks(
629+
OptRef<ConnectionPool::ConnectionLifetimeCallbacks> callbacks, std::vector<uint8_t> hash_key) {
630+
callbacks_ = callbacks;
631+
hash_key_ = std::move(hash_key);
632+
}
633+
613634
} // namespace Http
614635
} // namespace Envoy

source/common/http/conn_pool_grid.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ namespace Http {
3030
// the host's address list.
3131
class ConnectivityGrid : public ConnectionPool::Instance,
3232
public Http3::PoolConnectResultCallback,
33+
public ConnectionPool::ConnectionLifetimeCallbacks,
3334
protected Logger::Loggable<Logger::Id::pool> {
3435
public:
3536
struct ConnectivityOptions {
@@ -228,6 +229,15 @@ class ConnectivityGrid : public ConnectionPool::Instance,
228229
void onHandshakeComplete() override;
229230
void onZeroRttHandshakeFailed() override;
230231

232+
// ConnectionPool::ConnectionLifetimeCallbacks
233+
void onConnectionOpen(ConnectionPool::Instance& pool, std::vector<uint8_t>& hash_key,
234+
const Network::Connection& connection) override;
235+
void onConnectionDraining(ConnectionPool::Instance& pool, std::vector<uint8_t>& hash_key,
236+
const Network::Connection& connection) override;
237+
238+
void setLifetimeCallbacks(OptRef<ConnectionPool::ConnectionLifetimeCallbacks> callbacks,
239+
std::vector<uint8_t> hash_key) override;
240+
231241
protected:
232242
// Set the required idle callback on the pool.
233243
void setupPool(ConnectionPool::Instance& pool);
@@ -308,6 +318,9 @@ class ConnectivityGrid : public ConnectionPool::Instance,
308318
bool deferred_deleting_{};
309319

310320
OptRef<Quic::EnvoyQuicNetworkObserverRegistry> network_observer_registry_;
321+
322+
OptRef<ConnectionPool::ConnectionLifetimeCallbacks> callbacks_;
323+
std::vector<uint8_t> hash_key_;
311324
};
312325

313326
} // namespace Http

source/common/upstream/cluster_manager_impl.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2023,6 +2023,8 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::httpConnPoolImp
20232023
parent.httpConnPoolIsIdle(host, priority, hash_key);
20242024
});
20252025

2026+
pool->setLifetimeCallbacks(lb_->lifetimeCallbacks(), hash_key);
2027+
20262028
return pool;
20272029
});
20282030

0 commit comments

Comments
 (0)