grid: Add a new class for tracking HTTP/3 status

source/common/http/BUILD

@@ -149,11 +149,22 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "http3_status_tracker",
+    srcs = ["http3_status_tracker.cc"],
+    hdrs = ["http3_status_tracker.h"],
+    deps = [
+        "//include/envoy/event:dispatcher_interface",
+        "//include/envoy/event:timer_interface",
+    ],
+)
+
 envoy_cc_library(
     name = "conn_pool_grid",
     srcs = ["conn_pool_grid.cc"],
     hdrs = ["conn_pool_grid.h"],
     deps = [
+        ":http3_status_tracker",
         ":mixed_conn_pool",
         "//source/common/http/http3:conn_pool_lib",
     ],

source/common/http/conn_pool_grid.cc

@@ -110,8 +110,11 @@ void ConnectivityGrid::WrapperCallbacks::onConnectionAttemptReady(
             host->hostname());
   if (!grid_.isPoolHttp3(attempt->pool())) {
     tcp_attempt_succeeded_ = true;
+    maybeMarkHttp3Broken();
+  } else {
+    ENVOY_LOG(trace, "Marking HTTP/3 confirmed for host '{}'.", grid_.host_->hostname());
+    grid_.markHttp3Confirmed();
   }
-  maybeMarkHttp3Broken();
 
   auto delete_this_on_return = attempt->removeFromList(connection_attempts_);
   ConnectionPool::Callbacks* callbacks = inner_callbacks_;
@@ -133,7 +136,7 @@ void ConnectivityGrid::WrapperCallbacks::onConnectionAttemptReady(
 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);
+    grid_.markHttp3Broken();
   }
 }
 
@@ -190,7 +193,8 @@ ConnectivityGrid::ConnectivityGrid(
     std::chrono::milliseconds next_attempt_duration, ConnectivityOptions connectivity_options)
     : dispatcher_(dispatcher), random_generator_(random_generator), host_(host),
       priority_(priority), options_(options), transport_socket_options_(transport_socket_options),
-      state_(state), next_attempt_duration_(next_attempt_duration), time_source_(time_source) {
+      state_(state), next_attempt_duration_(next_attempt_duration), time_source_(time_source),
+      http3_status_tracker_(dispatcher_) {
   // ProdClusterManagerFactory::allocateConnPool verifies the protocols are HTTP/1, HTTP/2 and
   // HTTP/3.
   // TODO(#15649) support v6/v4, WiFi/cellular.
@@ -243,7 +247,7 @@ ConnectionPool::Cancellable* ConnectivityGrid::newStream(Http::ResponseDecoder&
     createNextPool();
   }
   PoolIterator pool = pools_.begin();
-  if (is_http3_broken_) {
+  if (http3_status_tracker_.isHttp3Broken()) {
     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
@@ -306,6 +310,12 @@ bool ConnectivityGrid::isPoolHttp3(const ConnectionPool::Instance& pool) {
   return &pool == pools_.begin()->get();
 }
 
+bool ConnectivityGrid::isHttp3Broken() const { return http3_status_tracker_.isHttp3Broken(); }
+
+void ConnectivityGrid::markHttp3Broken() { http3_status_tracker_.markHttp3Broken(); }
+
+void ConnectivityGrid::markHttp3Confirmed() { http3_status_tracker_.markHttp3Confirmed(); }
+
 void ConnectivityGrid::onDrainReceived() {
   // Don't do any work under the stack of ~ConnectivityGrid()
   if (destroying_) {

source/common/http/conn_pool_grid.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "common/http/conn_pool_base.h"
+#include "common/http/http3_status_tracker.h"
 
 #include "absl/container/flat_hash_map.h"
 
@@ -150,8 +151,9 @@ class ConnectivityGrid : public ConnectionPool::Instance,
   // 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; }
+  bool isHttp3Broken() const;
+  void markHttp3Broken();
+  void markHttp3Confirmed();
 
 private:
   friend class ConnectivityGridForTest;
@@ -174,7 +176,7 @@ class ConnectivityGrid : public ConnectionPool::Instance,
   Upstream::ClusterConnectivityState& state_;
   std::chrono::milliseconds next_attempt_duration_;
   TimeSource& time_source_;
-  bool is_http3_broken_{};
+  Http3StatusTracker http3_status_tracker_;
 
   // 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

source/common/http/http3_status_tracker.cc

@@ -0,0 +1,47 @@
+#include "common/http/http3_status_tracker.h"
+
+namespace Envoy {
+namespace Http {
+
+namespace {
+
+// Initially, HTTP/3 is be marked broken for 5 minutes.
+const std::chrono::minutes DefaultExpirationTime{5};
+// Cap the broken period at just under 1 day.
+const int MaxConsecutiveBrokenCount = 8;
+} // namespace
+
+Http3StatusTracker::Http3StatusTracker(Event::Dispatcher& dispatcher)
+    : expiration_timer_(dispatcher.createTimer([this]() -> void { onExpirationTimeout(); })) {}
+
+bool Http3StatusTracker::isHttp3Broken() const { return state_ == State::Broken; }
+
+bool Http3StatusTracker::isHttp3Confirmed() const { return state_ == State::Confirmed; }
+
+void Http3StatusTracker::markHttp3Broken() {
+  ASSERT(state_ != State::Broken);
+  state_ = State::Broken;
+  if (!expiration_timer_->enabled()) {
+    expiration_timer_->enableTimer(std::chrono::duration_cast<std::chrono::milliseconds>(
+        DefaultExpirationTime * (1 << consecutive_broken_count_)));
+    if (consecutive_broken_count_ < MaxConsecutiveBrokenCount) {
+      ++consecutive_broken_count_;
+    }
+  }
+}
+
+void Http3StatusTracker::markHttp3Confirmed() {
+  ASSERT(state_ == State::Pending);
+  ASSERT(!expiration_timer_->enabled());
+  state_ = State::Confirmed;
+  consecutive_broken_count_ = 0;
+}
+
+void Http3StatusTracker::onExpirationTimeout() {
+  ASSERT(state_ == State::Broken);
+
+  state_ = State::Pending;
+}
+
+} // namespace Http
+} // namespace Envoy

source/common/http/http3_status_tracker.h

@@ -0,0 +1,42 @@
+#pragma once
+
+#include "envoy/event/dispatcher.h"
+#include "envoy/event/timer.h"
+
+namespace Envoy {
+namespace Http {
+
+// Tracks the status of HTTP/3 being broken for a period of time
+// subject to exponential backoff.
+class Http3StatusTracker {
+public:
+  Http3StatusTracker(Event::Dispatcher& dispatcher);
+
+  // Returns true if HTTP/3 is broken.
+  bool isHttp3Broken() const;
+  // Returns true if HTTP/3 is confirmed to be working.
+  bool isHttp3Confirmed() const;
+  // Marks HTTP/3 broken for a period of time, subject to backoff.
+  void markHttp3Broken();
+  // Marks HTTP/3 as confirmed to be working and resets the backoff timeout.
+  void markHttp3Confirmed();
+
+private:
+  enum class State {
+    Pending,
+    Broken,
+    Confirmed,
+  };
+
+  // Called when the expiration timer fires.
+  void onExpirationTimeout();
+
+  State state_{State::Pending};
+  // The number of consecutive times HTTP/3 has been marked broken.
+  int consecutive_broken_count_{};
+  // The timer which tracks when HTTP/3 broken status should expire
+  Event::TimerPtr expiration_timer_;
+};
+
+} // namespace Http
+} // namespace Envoy

test/common/http/BUILD

@@ -440,6 +440,16 @@ envoy_cc_test(
     ]),
 )
 
+envoy_cc_test(
+    name = "http3_status_tracker_test",
+    srcs = ["http3_status_tracker_test.cc"],
+    deps = [
+        ":common_lib",
+        "//source/common/http:http3_status_tracker",
+        "//test/mocks:common_lib",
+    ],
+)
+
 envoy_proto_library(
     name = "path_utility_fuzz_proto",
     srcs = ["path_utility_fuzz.proto"],

test/common/http/conn_pool_grid_test.cc

@@ -424,7 +424,7 @@ TEST_F(ConnectivityGridTest, NoDrainOnTeardown) {
 
 // Test that when HTTP/3 is broken then the HTTP/3 pool is skipped.
 TEST_F(ConnectivityGridTest, SuccessAfterBroken) {
-  grid_.setIsHttp3Broken(true);
+  grid_.markHttp3Broken();
   EXPECT_EQ(grid_.first(), nullptr);
 
   EXPECT_LOG_CONTAINS("trace", "HTTP/3 is broken to host 'first', skipping.",

test/common/http/http3_status_tracker_test.cc

@@ -0,0 +1,170 @@
+#include "common/http/http3_status_tracker.h"
+
+#include "test/mocks/common.h"
+#include "test/mocks/event/mocks.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using Envoy::Event::MockTimer;
+using testing::_;
+using testing::AnyNumber;
+using testing::NiceMock;
+using testing::Return;
+using testing::StrictMock;
+
+namespace Envoy {
+namespace Http {
+
+namespace {
+class Http3StatusTrackerTest : public testing::Test {
+public:
+  Http3StatusTrackerTest()
+      : timer_(new StrictMock<MockTimer>(&dispatcher_)), tracker_(dispatcher_) {}
+
+  NiceMock<Event::MockDispatcher> dispatcher_;
+  StrictMock<MockTimer>* timer_; // Owned by tracker_;
+  Http3StatusTracker tracker_;
+};
+
+TEST_F(Http3StatusTrackerTest, Initialized) {
+  EXPECT_FALSE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+}
+
+TEST_F(Http3StatusTrackerTest, MarkBroken) {
+  EXPECT_CALL(*timer_, enabled()).WillOnce(Return(false));
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(5 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  EXPECT_TRUE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+}
+
+TEST_F(Http3StatusTrackerTest, MarkBrokenThenExpires) {
+  EXPECT_CALL(*timer_, enabled()).WillOnce(Return(false));
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(5 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+
+  timer_->invokeCallback();
+  EXPECT_FALSE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+}
+
+TEST_F(Http3StatusTrackerTest, MarkBrokenWithBackoff) {
+  // markBroken will only be called when the time is not enabled.
+  EXPECT_CALL(*timer_, enabled()).WillRepeatedly(Return(false));
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(5 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(10 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+
+  timer_->invokeCallback();
+  EXPECT_FALSE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(20 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  EXPECT_TRUE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+
+  timer_->invokeCallback();
+  EXPECT_FALSE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(40 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  EXPECT_TRUE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+
+  timer_->invokeCallback();
+  EXPECT_FALSE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+}
+
+TEST_F(Http3StatusTrackerTest, MarkBrokenWithBackoffMax) {
+  // markBroken will only be called when the time is not enabled.
+  EXPECT_CALL(*timer_, enabled()).WillRepeatedly(Return(false));
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(5 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(10 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(20 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(40 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(80 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(160 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(320 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(640 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(1280 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+
+  // Broken period no longer increases.
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(1280 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+  timer_->invokeCallback();
+}
+
+TEST_F(Http3StatusTrackerTest, MarkBrokenThenExpiresThenConfirmedThenBroken) {
+  EXPECT_CALL(*timer_, enabled()).WillOnce(Return(false));
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(5 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enabled()).WillOnce(Return(false));
+  tracker_.markHttp3Confirmed();
+  EXPECT_FALSE(tracker_.isHttp3Broken());
+  EXPECT_TRUE(tracker_.isHttp3Confirmed());
+
+  // markConfirmed will have reset the timeout back to the initial value.
+  EXPECT_CALL(*timer_, enabled()).WillOnce(Return(false));
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(5 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+
+  EXPECT_TRUE(tracker_.isHttp3Broken());
+  EXPECT_FALSE(tracker_.isHttp3Confirmed());
+}
+
+TEST_F(Http3StatusTrackerTest, MarkBrokenThenConfirmed) {
+  EXPECT_CALL(*timer_, enabled()).WillOnce(Return(false));
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(5 * 60 * 1000), nullptr));
+  tracker_.markHttp3Broken();
+
+  timer_->invokeCallback();
+
+  EXPECT_CALL(*timer_, enabled()).WillRepeatedly(Return(false));
+  tracker_.markHttp3Confirmed();
+  EXPECT_FALSE(tracker_.isHttp3Broken());
+  EXPECT_TRUE(tracker_.isHttp3Confirmed());
+}
+
+} // namespace
+} // namespace Http
+} // namespace Envoy