Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 0 deletions source/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 8 additions & 2 deletions source/common/http/alternate_protocols_cache_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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),
Expand Down
21 changes: 17 additions & 4 deletions source/common/http/alternate_protocols_cache_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "envoy/http/alternate_protocols_cache.h"

#include "absl/strings/string_view.h"
#include "quiche/common/quiche_linked_hash_map.h"

namespace Envoy {
namespace Http {
Expand All @@ -19,7 +20,8 @@ namespace Http {
// See: source/docs/http3_upstream.md
class AlternateProtocolsCacheImpl : public AlternateProtocolsCache {
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
Expand Down Expand Up @@ -48,12 +50,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_) +
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.

37?

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.

Ah, I stole this from source/common/upstream/load_balancer_impl.h which seems to be based on a pattern that's referenced in Effective Java (a great book). Good enough?

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.

Also added a comment.

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
21 changes: 20 additions & 1 deletion test/common/http/alternate_protocols_cache_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions test/common/http/conn_pool_grid_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand All @@ -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() {
Expand Down