Skip to content

Commit

Permalink
grid: Track HTTP/3 broken-ness in ConnectivityGrid. (envoyproxy#15933)
Browse files Browse the repository at this point in the history 
* grid: Track HTTP/3 broken-ness in ConnectivityGrid.
Mark HTTP/3 as broken when the HTTP/3 connect attempt fails but the
fallback attempt succeeds. When HTTP/3 is broken, do not attempt to initiate new HTTP/3 connections.
Add an assert that the Grid's host and the callback's host are the same.

Signed-off-by: Ryan Hamilton <[email protected]>
Signed-off-by: Gokul Nair <[email protected]>
  • Loading branch information
@RyanTheOptimist
RyanTheOptimist authored and Gokul Nair committed May 6, 2021
1 parent c17bdd0 commit 65c4e5a
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 38 deletions.
110 changes: 84 additions & 26 deletions source/common/http/conn_pool_grid.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@
namespace Envoy {
namespace Http {

namespace {
absl::string_view describePool(const ConnectionPool::Instance& pool) {
return pool.protocolDescription();
}
} // namespace

ConnectivityGrid::WrapperCallbacks::WrapperCallbacks(ConnectivityGrid& grid,
Http::ResponseDecoder& decoder,
PoolIterator pool_it,
ConnectionPool::Callbacks& callbacks)
: grid_(grid), decoder_(decoder), inner_callbacks_(callbacks),
: grid_(grid), decoder_(decoder), inner_callbacks_(&callbacks),
next_attempt_timer_(
grid_.dispatcher_.createTimer([this]() -> void { tryAnotherConnection(); })),
current_(pool_it) {}
Expand All @@ -24,6 +26,12 @@ ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::ConnectionAttemp
WrapperCallbacks& parent, PoolIterator it)
: parent_(parent), pool_it_(it), cancellable_(nullptr) {}

ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::~ConnectionAttemptCallbacks() {
if (cancellable_ != nullptr) {
cancel(Envoy::ConnectionPool::CancelPolicy::Default);
}
}

ConnectivityGrid::StreamCreationResult
ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::newStream() {
auto* cancellable = pool().newStream(parent_.decoder_, *this);
Expand All @@ -37,14 +45,21 @@ ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::newStream() {
void ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::onPoolFailure(
ConnectionPool::PoolFailureReason reason, absl::string_view transport_failure_reason,
Upstream::HostDescriptionConstSharedPtr host) {
cancellable_ = nullptr; // Attempt failed and can no longer be cancelled.
parent_.onConnectionAttemptFailed(this, reason, transport_failure_reason, host);
}

void ConnectivityGrid::WrapperCallbacks::onConnectionAttemptFailed(
ConnectionAttemptCallbacks* attempt, ConnectionPool::PoolFailureReason reason,
absl::string_view transport_failure_reason, Upstream::HostDescriptionConstSharedPtr host) {
ASSERT(host == grid_.host_);
ENVOY_LOG(trace, "{} pool failed to create connection to host '{}'.",
describePool(attempt->pool()), grid_.host_->hostname());
describePool(attempt->pool()), host->hostname());
if (grid_.isPoolHttp3(attempt->pool())) {
http3_attempt_failed_ = true;
}
maybeMarkHttp3Broken();

auto delete_this_on_return = attempt->removeFromList(connection_attempts_);

// If there is another connection attempt in flight then let that proceed.
Expand All @@ -58,12 +73,15 @@ void ConnectivityGrid::WrapperCallbacks::onConnectionAttemptFailed(
}

// If this point is reached, all pools have been tried. Pass the pool failure up to the
// original caller.
ConnectionPool::Callbacks& callbacks = inner_callbacks_;
ENVOY_LOG(trace, "Passing pool failure up to caller.", describePool(attempt->pool()),
grid_.host_->hostname());
// original caller, if the caller hasn't already been notified.
ConnectionPool::Callbacks* callbacks = inner_callbacks_;
inner_callbacks_ = nullptr;
deleteThis();
callbacks.onPoolFailure(reason, transport_failure_reason, host);
if (callbacks != nullptr) {
ENVOY_LOG(trace, "Passing pool failure up to caller.", describePool(attempt->pool()),
host->hostname());
callbacks->onPoolFailure(reason, transport_failure_reason, host);
}
}

void ConnectivityGrid::WrapperCallbacks::deleteThis() {
Expand All @@ -87,37 +105,65 @@ void ConnectivityGrid::WrapperCallbacks::onConnectionAttemptReady(
ConnectionAttemptCallbacks* attempt, RequestEncoder& encoder,
Upstream::HostDescriptionConstSharedPtr host, const StreamInfo::StreamInfo& info,
absl::optional<Http::Protocol> protocol) {
ASSERT(host == grid_.host_);
ENVOY_LOG(trace, "{} pool successfully connected to host '{}'.", describePool(attempt->pool()),
grid_.host_->hostname());
auto delete_on_return = attempt->removeFromList(connection_attempts_);
// The first successful connection is passed up, and all others will be canceled.
// TODO: Ensure that if HTTP/2 succeeds, we can allow the HTTP/3 connection to run to completion.
for (auto& attempt : connection_attempts_) {
attempt->cancel(Envoy::ConnectionPool::CancelPolicy::Default);
host->hostname());
if (!grid_.isPoolHttp3(attempt->pool())) {
tcp_attempt_succeeded_ = true;
}
maybeMarkHttp3Broken();

auto delete_this_on_return = attempt->removeFromList(connection_attempts_);
ConnectionPool::Callbacks* callbacks = inner_callbacks_;
inner_callbacks_ = nullptr;
// If an HTTP/3 connection attempts is in progress, let it complete so that if it succeeds
// it can be used for future requests. But if there is a TCP connection attempt in progress,
// cancel it.
if (grid_.isPoolHttp3(attempt->pool())) {
cancelAllPendingAttempts(Envoy::ConnectionPool::CancelPolicy::Default);
}
if (connection_attempts_.empty()) {
deleteThis();
}
if (callbacks != nullptr) {
callbacks->onPoolReady(encoder, host, info, protocol);
}
}

void ConnectivityGrid::WrapperCallbacks::maybeMarkHttp3Broken() {
if (http3_attempt_failed_ && tcp_attempt_succeeded_) {
ENVOY_LOG(trace, "Marking HTTP/3 broken for host '{}'.", grid_.host_->hostname());
grid_.setIsHttp3Broken(true);
}
ConnectionPool::Callbacks& callbacks = inner_callbacks_;
deleteThis();
return callbacks.onPoolReady(encoder, host, info, protocol);
}

void ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::onPoolReady(
RequestEncoder& encoder, Upstream::HostDescriptionConstSharedPtr host,
const StreamInfo::StreamInfo& info, absl::optional<Http::Protocol> protocol) {
cancellable_ = nullptr; // Attempt succeeded and can no longer be cancelled.
parent_.onConnectionAttemptReady(this, encoder, host, info, protocol);
}

void ConnectivityGrid::WrapperCallbacks::ConnectionAttemptCallbacks::cancel(
Envoy::ConnectionPool::CancelPolicy cancel_policy) {
cancellable_->cancel(cancel_policy);
auto cancellable = cancellable_;
cancellable_ = nullptr; // Prevent repeated cancellations.
cancellable->cancel(cancel_policy);
}

void ConnectivityGrid::WrapperCallbacks::cancel(Envoy::ConnectionPool::CancelPolicy cancel_policy) {
// If the newStream caller cancels the stream request, pass the cancellation on
// to each connection attempt.
cancelAllPendingAttempts(cancel_policy);
deleteThis();
}

void ConnectivityGrid::WrapperCallbacks::cancelAllPendingAttempts(
Envoy::ConnectionPool::CancelPolicy cancel_policy) {
for (auto& attempt : connection_attempts_) {
attempt->cancel(cancel_policy);
}
deleteThis();
connection_attempts_.clear();
}

bool ConnectivityGrid::WrapperCallbacks::tryAnotherConnection() {
Expand Down Expand Up @@ -154,13 +200,16 @@ ConnectivityGrid::ConnectivityGrid(
ConnectivityGrid::~ConnectivityGrid() {
// Ignore drained callbacks while the pools are destroyed below.
destroying_ = true;
// Callbacks might have pending streams registered with the pools, so cancel and delete
// the callback before deleting the pools.
wrapped_callbacks_.clear();
pools_.clear();
}

absl::optional<ConnectivityGrid::PoolIterator> ConnectivityGrid::createNextPool() {
// Pools are created by newStream, which should not be called during draining.
ASSERT(drained_callbacks_.empty());
// Right now, only H3 and ALPN are supported, so if there are 2 pools we're done.
// Right now, only H3 and TCP are supported, so if there are 2 pools we're done.
if (pools_.size() == 2 || !drained_callbacks_.empty()) {
return absl::nullopt;
}
Expand Down Expand Up @@ -193,17 +242,22 @@ ConnectionPool::Cancellable* ConnectivityGrid::newStream(Http::ResponseDecoder&
if (pools_.empty()) {
createNextPool();
}

// TODO(#15649) track pools with successful connections: don't always start at
// the front of the list.
auto wrapped_callback =
std::make_unique<WrapperCallbacks>(*this, decoder, pools_.begin(), callbacks);
PoolIterator pool = pools_.begin();
if (is_http3_broken_) {
ENVOY_LOG(trace, "HTTP/3 is broken to host '{}', skipping.", describePool(**pool),
host_->hostname());
// Since HTTP/3 is broken, presumably both pools have already been created so this
// is just to be safe.
createNextPool();
++pool;
}
auto wrapped_callback = std::make_unique<WrapperCallbacks>(*this, decoder, pool, callbacks);
ConnectionPool::Cancellable* ret = wrapped_callback.get();
LinkedList::moveIntoList(std::move(wrapped_callback), wrapped_callbacks_);
// Note that in the case of immediate attempt/failure, newStream will delete this.
if (wrapped_callbacks_.front()->newStream() == StreamCreationResult::ImmediateResult) {
// If newStream succeeds, return nullptr as the caller has received their
// callback and does not need a cancellable handle.
// callback and does not need a cancellable handle. At this point the
// WrappedCallbacks object has also been deleted.
return nullptr;
}
return ret;
Expand Down Expand Up @@ -248,6 +302,10 @@ absl::optional<ConnectivityGrid::PoolIterator> ConnectivityGrid::nextPool(PoolIt
return createNextPool();
}

bool ConnectivityGrid::isPoolHttp3(const ConnectionPool::Instance& pool) {
return &pool == pools_.begin()->get();
}

void ConnectivityGrid::onDrainReceived() {
// Don't do any work under the stack of ~ConnectivityGrid()
if (destroying_) {
Expand Down
30 changes: 25 additions & 5 deletions source/common/http/conn_pool_grid.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class ConnectivityGrid : public ConnectionPool::Instance,
public LinkedObject<ConnectionAttemptCallbacks> {
public:
ConnectionAttemptCallbacks(WrapperCallbacks& parent, PoolIterator it);
~ConnectionAttemptCallbacks() override;

StreamCreationResult newStream();

Expand Down Expand Up @@ -74,9 +75,6 @@ class ConnectivityGrid : public ConnectionPool::Instance,
// Attempt to create a new stream for pool().
StreamCreationResult newStream();

// Removes this from the owning list, deleting it.
void deleteThis();

// Called on pool failure or timeout to kick off another connection attempt.
// Returns true if there is a failover pool and a connection has been
// attempted, false if all pools have been tried.
Expand All @@ -95,6 +93,16 @@ class ConnectivityGrid : public ConnectionPool::Instance,
absl::optional<Http::Protocol> protocol);

private:
// Removes this from the owning list, deleting it.
void deleteThis();

// Marks HTTP/3 broken if the HTTP/3 attempt failed but a TCP attempt succeeded.
// While HTTP/3 is broken the grid will not attempt to make new HTTP/3 connections.
void maybeMarkHttp3Broken();

// Cancels any pending attempts and deletes them.
void cancelAllPendingAttempts(Envoy::ConnectionPool::CancelPolicy cancel_policy);

// Tracks all the connection attempts which currently in flight.
std::list<ConnectionAttemptCallbacksPtr> connection_attempts_;

Expand All @@ -103,12 +111,17 @@ class ConnectivityGrid : public ConnectionPool::Instance,
// The decoder for the original newStream, needed to create streams on subsequent pools.
Http::ResponseDecoder& decoder_;
// The callbacks from the original caller, which must get onPoolFailure or
// onPoolReady unless there is call to cancel().
ConnectionPool::Callbacks& inner_callbacks_;
// onPoolReady unless there is call to cancel(). Will be nullptr if the caller
// has been notified while attempts are still pending.
ConnectionPool::Callbacks* inner_callbacks_;
// The timer which tracks when new connections should be attempted.
Event::TimerPtr next_attempt_timer_;
// The iterator to the last pool which had a connection attempt.
PoolIterator current_;
// True if the HTTP/3 attempt failed.
bool http3_attempt_failed_{};
// True if the TCP attempt succeeded.
bool tcp_attempt_succeeded_{};
};
using WrapperCallbacksPtr = std::unique_ptr<WrapperCallbacks>;

Expand All @@ -134,6 +147,12 @@ class ConnectivityGrid : public ConnectionPool::Instance,
// Returns the next pool in the ordered priority list.
absl::optional<PoolIterator> nextPool(PoolIterator pool_it);

// Returns true if pool is the grid's HTTP/3 connection pool.
bool isPoolHttp3(const ConnectionPool::Instance& pool);

bool isHttp3Broken() const { return is_http3_broken_; }
void setIsHttp3Broken(bool is_http3_broken) { is_http3_broken_ = is_http3_broken; }

private:
friend class ConnectivityGridForTest;

Expand All @@ -155,6 +174,7 @@ class ConnectivityGrid : public ConnectionPool::Instance,
Upstream::ClusterConnectivityState& state_;
std::chrono::milliseconds next_attempt_duration_;
TimeSource& time_source_;
bool is_http3_broken_{};

// Tracks how many drains are needed before calling drain callbacks. This is
// set to the number of pools when the first drain callbacks are added, and
Expand Down
Loading

0 comments on commit 65c4e5a

Please sign in to comment.