alternate_protocols_cache: Impose a max size limit on the alternate p…

…rotocols cache (#18258)

alternate_protocols_cache: Impose a max size limit on the alternate protocols cache

Implement a very basic MRU map in AlternateProtocolsCacheImpl.

Risk Level: Low
Testing: Unit tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Ryan Hamilton <rch@google.com>

source/common/http/BUILD

@@ -169,6 +169,7 @@ envoy_cc_library(
         "alternate_protocols_cache_impl.h",
         "alternate_protocols_cache_manager_impl.h",
     ],
+    external_deps = ["quiche_quic_platform"],
     deps = [
         "//envoy/common:time_interface",
         "//envoy/event:dispatcher_interface",

source/common/http/alternate_protocols_cache_impl.cc

@@ -64,8 +64,9 @@ AlternateProtocolsCacheImpl::protocolsFromString(absl::string_view alt_svc_strin
 }
 
 AlternateProtocolsCacheImpl::AlternateProtocolsCacheImpl(
-    TimeSource& time_source, std::unique_ptr<KeyValueStore>&& key_value_store)
-    : time_source_(time_source), key_value_store_(std::move(key_value_store)) {}
+    TimeSource& time_source, std::unique_ptr<KeyValueStore>&& key_value_store, size_t max_entries)
+    : time_source_(time_source), key_value_store_(std::move(key_value_store)),
+      max_entries_(max_entries > 0 ? max_entries : 1024) {}
 
 AlternateProtocolsCacheImpl::~AlternateProtocolsCacheImpl() = default;
 
@@ -76,6 +77,11 @@ void AlternateProtocolsCacheImpl::setAlternatives(const Origin& origin,
     ENVOY_LOG_MISC(trace, "Too many alternate protocols: {}, truncating", protocols.size());
     protocols.erase(protocols.begin() + max_protocols, protocols.end());
   }
+  while (protocols_.size() >= max_entries_) {
+    auto iter = protocols_.begin();
+    key_value_store_->remove(originToString(iter->first));
+    protocols_.erase(iter);
+  }
   protocols_[origin] = protocols;
   if (key_value_store_) {
     key_value_store_->addOrUpdate(originToString(origin),

source/common/http/alternate_protocols_cache_impl.h

@@ -13,6 +13,7 @@
 #include "source/common/common/logger.h"
 
 #include "absl/strings/string_view.h"
+#include "quiche/common/quiche_linked_hash_map.h"
 
 namespace Envoy {
 namespace Http {
@@ -22,7 +23,8 @@ namespace Http {
 class AlternateProtocolsCacheImpl : public AlternateProtocolsCache,
                                     Logger::Loggable<Logger::Id::alternate_protocols_cache> {
 public:
-  AlternateProtocolsCacheImpl(TimeSource& time_source, std::unique_ptr<KeyValueStore>&& store);
+  AlternateProtocolsCacheImpl(TimeSource& time_source, std::unique_ptr<KeyValueStore>&& store,
+                              size_t max_entries);
   ~AlternateProtocolsCacheImpl() override;
 
   // Convert an AlternateProtocol vector to a string to cache to the key value
@@ -51,12 +53,23 @@ class AlternateProtocolsCacheImpl : public AlternateProtocolsCache,
   // Time source used to check expiration of entries.
   TimeSource& time_source_;
 
-  // Map from hostname to list of alternate protocols.
-  // TODO(RyanTheOptimist): Add a limit to the size of this map and evict based on usage.
-  std::map<Origin, std::vector<AlternateProtocol>> protocols_;
+  struct OriginHash {
+    size_t operator()(const Origin& origin) const {
+      // Multiply the hashes by the magic number 37 to spread the bits around.
+      size_t hash = std::hash<std::string>()(origin.scheme_) +
+                    37 * (std::hash<std::string>()(origin.hostname_) +
+                          37 * std::hash<uint32_t>()(origin.port_));
+      return hash;
+    }
+  };
+
+  // Map from origin to list of alternate protocols.
+  quiche::QuicheLinkedHashMap<Origin, std::vector<AlternateProtocol>, OriginHash> protocols_;
 
   // The key value store, if flushing to persistent storage.
   std::unique_ptr<KeyValueStore> key_value_store_;
+
+  const size_t max_entries_;
 };
 
 } // namespace Http

source/common/http/alternate_protocols_cache_manager_impl.cc

@@ -51,7 +51,7 @@ AlternateProtocolsCacheSharedPtr AlternateProtocolsCacheManagerImpl::getCache(
   }
 
   AlternateProtocolsCacheSharedPtr new_cache = std::make_shared<AlternateProtocolsCacheImpl>(
-      data_.dispatcher_.timeSource(), std::move(store));
+      data_.dispatcher_.timeSource(), std::move(store), options.max_entries().value());
   (*slot_).caches_.emplace(options.name(), CacheWithOptions{options, new_cache});
   return new_cache;
 }

test/common/http/alternate_protocols_cache_impl_test.cc

@@ -15,10 +15,13 @@ class AlternateProtocolsCacheImplTest : public testing::Test, public Event::Test
 public:
   AlternateProtocolsCacheImplTest()
       : store_(new NiceMock<MockKeyValueStore>()),
-        protocols_(simTime(), std::unique_ptr<KeyValueStore>(store_)) {}
+        protocols_(simTime(), std::unique_ptr<KeyValueStore>(store_), max_entries_) {}
+
+  const size_t max_entries_ = 10;
 
   MockKeyValueStore* store_;
   AlternateProtocolsCacheImpl protocols_;
+
   const std::string hostname1_ = "hostname1";
   const std::string hostname2_ = "hostname2";
   const uint32_t port1_ = 1;
@@ -132,6 +135,22 @@ TEST_F(AlternateProtocolsCacheImplTest, FindAlternativesAfterTruncation) {
   EXPECT_EQ(expected_protocols, protocols.ref());
 }
 
+TEST_F(AlternateProtocolsCacheImplTest, MaxEntries) {
+  EXPECT_EQ(0, protocols_.size());
+  const std::string hostname = "hostname";
+  for (uint32_t i = 0; i <= max_entries_; ++i) {
+    const AlternateProtocolsCache::Origin origin = {https_, hostname, i};
+    AlternateProtocolsCache::AlternateProtocol protocol = {alpn1_, hostname, i, expiration1_};
+    std::vector<AlternateProtocolsCache::AlternateProtocol> protocols = {protocol};
+    EXPECT_CALL(*store_, addOrUpdate(absl::StrCat("https://hostname:", i),
+                                     absl::StrCat("alpn1=\"hostname:", i, "\"; ma=5")));
+    if (i == max_entries_) {
+      EXPECT_CALL(*store_, remove("https://hostname:0"));
+    }
+    protocols_.setAlternatives(origin, protocols);
+  }
+}
+
 TEST_F(AlternateProtocolsCacheImplTest, ToAndFromString) {
   auto testAltSvc = [&](const std::string& original_alt_svc,
                         const std::string& expected_alt_svc) -> void {

test/common/http/conn_pool_grid_test.cc

@@ -102,7 +102,7 @@ class ConnectivityGridTest : public Event::TestUsingSimulatedTime, public testin
 public:
   ConnectivityGridTest()
       : options_({Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3}),
-        alternate_protocols_(std::make_shared<AlternateProtocolsCacheImpl>(simTime(), nullptr)),
+        alternate_protocols_(std::make_shared<AlternateProtocolsCacheImpl>(simTime(), nullptr, 10)),
         quic_stat_names_(store_.symbolTable()),
         grid_(dispatcher_, random_,
               Upstream::makeTestHost(cluster_, "hostname", "tcp://127.0.0.1:9000", simTime()),
@@ -120,7 +120,7 @@ class ConnectivityGridTest : public Event::TestUsingSimulatedTime, public testin
     if (!use_alternate_protocols) {
       return nullptr;
     }
-    return std::make_shared<AlternateProtocolsCacheImpl>(simTime(), nullptr);
+    return std::make_shared<AlternateProtocolsCacheImpl>(simTime(), nullptr, 10);
   }
 
   void addHttp3AlternateProtocol() {