Skip to content

Commit

Permalink
http3: allowing caching srtt without alt-svc (envoyproxy#20766)
Browse files Browse the repository at this point in the history 
Risk Level: low
Testing: new unit tests
Docs Changes: n/a
Release Notes: n/a
Part of envoyproxy#20696

Signed-off-by: Alyssa Wilk <[email protected]>
  • Loading branch information
@alyssawilk
alyssawilk authored Apr 13, 2022
1 parent e4ee794 commit 8f13d85
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 63 deletions.
95 changes: 53 additions & 42 deletions source/common/http/alternate_protocols_cache_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ AlternateProtocolsCacheImpl::stringToOrigin(const std::string& str) {
return {};
}

std::string AlternateProtocolsCacheImpl::originDataToStringForCache(
const std::vector<AlternateProtocol>& protocols, std::chrono::microseconds srtt) {
if (protocols.empty()) {
return std::string("clear|0");
std::string AlternateProtocolsCacheImpl::originDataToStringForCache(const OriginData& data) {
if (!data.protocols.has_value() || data.protocols->empty()) {
return absl::StrCat("clear|", data.srtt.count());
}
std::string value;
for (auto& protocol : protocols) {
for (auto& protocol : *data.protocols) {
if (!value.empty()) {
value.push_back(',');
}
Expand All @@ -57,7 +56,7 @@ std::string AlternateProtocolsCacheImpl::originDataToStringForCache(
std::chrono::duration_cast<std::chrono::seconds>(protocol.expiration_.time_since_epoch())
.count());
}
absl::StrAppend(&value, "|", srtt.count());
absl::StrAppend(&value, "|", data.srtt.count());
return value;
}

Expand Down Expand Up @@ -124,8 +123,12 @@ AlternateProtocolsCacheImpl::AlternateProtocolsCacheImpl(
// We deferred transfering ownership into key_value_store_ prior, so
// that we won't end up doing redundant updates to the store while
// iterating.
setAlternativesImpl(origin.value(), origin_data.value().protocols);
setSrttImpl(origin.value(), origin_data.value().srtt);
OptRef<std::vector<AlternateProtocol>> protocols;
if (origin_data->protocols.has_value()) {
protocols = *origin_data->protocols;
}
OriginDataWithOptRef data{protocols, origin_data->srtt, nullptr};
setPropertiesImpl(*origin, data);
} else {
ENVOY_LOG(warn,
fmt::format("Unable to parse cache entry with key: {} value: {}", key, value));
Expand All @@ -141,28 +144,20 @@ AlternateProtocolsCacheImpl::~AlternateProtocolsCacheImpl() = default;

void AlternateProtocolsCacheImpl::setAlternatives(const Origin& origin,
std::vector<AlternateProtocol>& protocols) {
setAlternativesImpl(origin, protocols);
OriginDataWithOptRef data;
data.protocols = protocols;
auto it = setPropertiesImpl(origin, data);
if (key_value_store_) {
key_value_store_->addOrUpdate(
originToString(origin),
originDataToStringForCache(protocols, std::chrono::microseconds(0)));
key_value_store_->addOrUpdate(originToString(origin), originDataToStringForCache(it->second));
}
}

void AlternateProtocolsCacheImpl::setSrtt(const Origin& origin, std::chrono::microseconds srtt) {
setSrttImpl(origin, srtt);
}

void AlternateProtocolsCacheImpl::setSrttImpl(const Origin& origin,
std::chrono::microseconds srtt) {
auto entry_it = protocols_.find(origin);
if (entry_it == protocols_.end()) {
return;
}
entry_it->second.srtt = srtt;
OriginDataWithOptRef data;
data.srtt = srtt;
auto it = setPropertiesImpl(origin, data);
if (key_value_store_) {
key_value_store_->addOrUpdate(originToString(origin),
originDataToStringForCache(entry_it->second.protocols, srtt));
key_value_store_->addOrUpdate(originToString(origin), originDataToStringForCache(it->second));
}
}

Expand All @@ -174,38 +169,54 @@ std::chrono::microseconds AlternateProtocolsCacheImpl::getSrtt(const Origin& ori
return entry_it->second.srtt;
}

void AlternateProtocolsCacheImpl::setAlternativesImpl(const Origin& origin,
std::vector<AlternateProtocol>& protocols) {
static const size_t max_protocols = 10;
if (protocols.size() > max_protocols) {
ENVOY_LOG_MISC(trace, "Too many alternate protocols: {}, truncating", protocols.size());
protocols.erase(protocols.begin() + max_protocols, protocols.end());
AlternateProtocolsCacheImpl::ProtocolsMap::iterator
AlternateProtocolsCacheImpl::setPropertiesImpl(const Origin& origin,
OriginDataWithOptRef& origin_data) {
if (origin_data.protocols.has_value()) {
std::vector<AlternateProtocol>& protocols = *origin_data.protocols;
static const size_t max_protocols = 10;
if (protocols.size() > max_protocols) {
ENVOY_LOG_MISC(trace, "Too many alternate protocols: {}, truncating", protocols.size());
protocols.erase(protocols.begin() + max_protocols, protocols.end());
}
}
auto entry_it = protocols_.find(origin);
if (entry_it != protocols_.end()) {
entry_it->second.protocols = protocols;
return;
if (origin_data.protocols.has_value()) {
entry_it->second.protocols = *origin_data.protocols;
}
if (origin_data.srtt.count()) {
entry_it->second.srtt = origin_data.srtt;
}
if (origin_data.h3_status_tracker) {
entry_it->second.h3_status_tracker = std::move(origin_data.h3_status_tracker);
}

return entry_it;
}
addOriginData(origin, OriginData{protocols, std::chrono::microseconds(0), nullptr});
return addOriginData(
origin, {origin_data.protocols, origin_data.srtt, std::move(origin_data.h3_status_tracker)});
}

void AlternateProtocolsCacheImpl::addOriginData(const Origin& origin, OriginData&& origin_data) {
AlternateProtocolsCacheImpl::ProtocolsMap::iterator
AlternateProtocolsCacheImpl::addOriginData(const Origin& origin, OriginData&& origin_data) {
ASSERT(protocols_.find(origin) == protocols_.end());
while (protocols_.size() >= max_entries_) {
auto iter = protocols_.begin();
key_value_store_->remove(originToString(iter->first));
protocols_.erase(iter);
}
protocols_[origin] = std::move(origin_data);
return protocols_.find(origin);
}

OptRef<const std::vector<AlternateProtocolsCache::AlternateProtocol>>
AlternateProtocolsCacheImpl::findAlternatives(const Origin& origin) {
auto entry_it = protocols_.find(origin);
if (entry_it == protocols_.end()) {
if (entry_it == protocols_.end() || !entry_it->second.protocols.has_value()) {
return makeOptRefFromPtr<const std::vector<AlternateProtocol>>(nullptr);
}
std::vector<AlternateProtocol>& protocols = entry_it->second.protocols;
std::vector<AlternateProtocol>& protocols = *entry_it->second.protocols;

auto original_size = protocols.size();
const MonotonicTime now = dispatcher_.timeSource().monotonicTime();
Expand All @@ -223,7 +234,7 @@ AlternateProtocolsCacheImpl::findAlternatives(const Origin& origin) {
}
if (key_value_store_ && original_size != protocols.size()) {
key_value_store_->addOrUpdate(originToString(origin),
originDataToStringForCache(protocols, entry_it->second.srtt));
originDataToStringForCache(entry_it->second));
}
return makeOptRef(const_cast<const std::vector<AlternateProtocol>&>(protocols));
}
Expand All @@ -239,11 +250,11 @@ AlternateProtocolsCacheImpl::getOrCreateHttp3StatusTracker(const Origin& origin)
}
return *entry_it->second.h3_status_tracker;
}
OriginData origin_data{
{}, std::chrono::microseconds(0), std::make_unique<Http3StatusTrackerImpl>(dispatcher_)};
AlternateProtocolsCache::Http3StatusTracker& tracker = *(origin_data.h3_status_tracker);
addOriginData(origin, std::move(origin_data));
return tracker;

OriginDataWithOptRef data;
data.h3_status_tracker = std::make_unique<Http3StatusTrackerImpl>(dispatcher_);
auto it = setPropertiesImpl(origin, data);
return *it->second.h3_status_tracker;
}

} // namespace Http
Expand Down
54 changes: 37 additions & 17 deletions source/common/http/alternate_protocols_cache_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
namespace Envoy {
namespace Http {

// An implementation of AlternateProtocolsCache.
// See: source/docs/http3_upstream.md
//
// The primary purpose of this cache is to cache alternate protocols entries.
// Secondarily, it maps origins to srtt information, useful for
// tuning 0-rtt timeouts if the alternate protocol is HTTP/3.
// A cache of HTTP server properties.
// This caches
// - alternate protocol entries as documented here: source/docs/http3_upstream.md
// - QUIC SRTT, used for TCP failover
// - The last connectivity status of HTTP/3, if available.
// TODO(alyssawilk) move and rename.
class AlternateProtocolsCacheImpl : public AlternateProtocolsCache,
Logger::Loggable<Logger::Id::alternate_protocols_cache> {
public:
Expand All @@ -35,11 +35,16 @@ class AlternateProtocolsCacheImpl : public AlternateProtocolsCache,

// Captures the data tracked per origin;,
struct OriginData {
// The alternate protocols supported.
std::vector<AlternateProtocol> protocols;
// The last smoothed round trip time, if available.
OriginData() = default;
OriginData(OptRef<std::vector<AlternateProtocol>> protocols, std::chrono::microseconds srtt,
Http3StatusTrackerPtr&& tracker)
: protocols(protocols), srtt(srtt), h3_status_tracker(std::move(tracker)) {}

// The alternate protocols supported if available.
absl::optional<std::vector<AlternateProtocol>> protocols;
// The last smoothed round trip time, if available else 0.
std::chrono::microseconds srtt;
// The last connectivity status of HTTP/3, if available.
// The last connectivity status of HTTP/3, if available else nullptr.
Http3StatusTrackerPtr h3_status_tracker;
};

Expand All @@ -56,8 +61,7 @@ class AlternateProtocolsCacheImpl : public AlternateProtocolsCache,
// normalization will simply not be read from cache.
// The string format is:
// protocols|rtt
static std::string originDataToStringForCache(const std::vector<AlternateProtocol>& protocols,
std::chrono::microseconds srtt);
static std::string originDataToStringForCache(const OriginData& data);
// Parse an origin data into structured data, or absl::nullopt
// if it is empty or invalid.
// If from_cache is true, it is assumed the string was serialized using
Expand All @@ -83,10 +87,6 @@ class AlternateProtocolsCacheImpl : public AlternateProtocolsCache,
getOrCreateHttp3StatusTracker(const Origin& origin) override;

private:
void setAlternativesImpl(const Origin& origin, std::vector<AlternateProtocol>& protocols);
void setSrttImpl(const Origin& origin, std::chrono::microseconds srtt);
void addOriginData(const Origin& origin, OriginData&& origin_data);

// Time source used to check expiration of entries.
Event::Dispatcher& dispatcher_;

Expand All @@ -100,8 +100,28 @@ class AlternateProtocolsCacheImpl : public AlternateProtocolsCache,
}
};

using ProtocolsMap = quiche::QuicheLinkedHashMap<Origin, OriginData, OriginHash>;
// Map from origin to list of alternate protocols.
quiche::QuicheLinkedHashMap<Origin, OriginData, OriginHash> protocols_;
ProtocolsMap protocols_;

// This allows calling setPropertiesImpl without creating an additional copy
// of the protocols vector.
struct OriginDataWithOptRef {
OriginDataWithOptRef() : srtt(std::chrono::milliseconds(0)) {}
OriginDataWithOptRef(OptRef<std::vector<AlternateProtocol>> protocols,
std::chrono::microseconds s, Http3StatusTrackerPtr&& t)
: protocols(protocols), srtt(s), h3_status_tracker(std::move(t)) {}
// The alternate protocols supported if available.
OptRef<std::vector<AlternateProtocol>> protocols;
// The last smoothed round trip time, if available else 0.
std::chrono::microseconds srtt;
// The last connectivity status of HTTP/3, if available else nullptr.
Http3StatusTrackerPtr h3_status_tracker;
};

ProtocolsMap::iterator setPropertiesImpl(const Origin& origin, OriginDataWithOptRef& origin_data);

ProtocolsMap::iterator addOriginData(const Origin& origin, OriginData&& origin_data);

// The key value store, if flushing to persistent storage.
std::unique_ptr<KeyValueStore> key_value_store_;
Expand Down
31 changes: 27 additions & 4 deletions test/common/http/alternate_protocols_cache_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ TEST_F(AlternateProtocolsCacheImplTest, Init) {
EXPECT_EQ(0, protocols_->size());
}

TEST_F(AlternateProtocolsCacheImplTest, SetAlternativesAndSrtt) {
TEST_F(AlternateProtocolsCacheImplTest, SetAlternativesThenSrtt) {
initialize();
EXPECT_EQ(0, protocols_->size());
EXPECT_EQ(std::chrono::microseconds(0), protocols_->getSrtt(origin1_));
Expand All @@ -84,6 +84,18 @@ TEST_F(AlternateProtocolsCacheImplTest, SetAlternativesAndSrtt) {
EXPECT_EQ(std::chrono::microseconds(5), protocols_->getSrtt(origin1_));
}

TEST_F(AlternateProtocolsCacheImplTest, SetSrttThenAlternatives) {
initialize();
EXPECT_EQ(0, protocols_->size());
EXPECT_CALL(*store_, addOrUpdate("https://hostname1:1", "clear|5"));
protocols_->setSrtt(origin1_, std::chrono::microseconds(5));
EXPECT_EQ(1, protocols_->size());
EXPECT_EQ(std::chrono::microseconds(5), protocols_->getSrtt(origin1_));
EXPECT_CALL(*store_, addOrUpdate("https://hostname1:1", "alpn1=\"hostname1:1\"; ma=5|5"));
protocols_->setAlternatives(origin1_, protocols1_);
EXPECT_EQ(std::chrono::microseconds(5), protocols_->getSrtt(origin1_));
}

TEST_F(AlternateProtocolsCacheImplTest, FindAlternatives) {
initialize();
EXPECT_CALL(*store_, addOrUpdate("https://hostname1:1", "alpn1=\"hostname1:1\"; ma=5|0"));
Expand Down Expand Up @@ -226,7 +238,7 @@ TEST_F(AlternateProtocolsCacheImplTest, ToAndFromString) {
dispatcher_.timeSource(), true);
ASSERT(origin_data.has_value());
std::vector<AlternateProtocolsCache::AlternateProtocol>& protocols =
origin_data.value().protocols;
origin_data.value().protocols.value();
ASSERT_GE(protocols.size(), 1);
AlternateProtocolsCache::AlternateProtocol& protocol = protocols[0];
EXPECT_EQ("h3-29", protocol.alpn_);
Expand All @@ -245,8 +257,8 @@ TEST_F(AlternateProtocolsCacheImplTest, ToAndFromString) {
EXPECT_EQ(60, duration.count());
}

std::string alt_svc = AlternateProtocolsCacheImpl::originDataToStringForCache(
protocols, origin_data.value().srtt);
std::string alt_svc =
AlternateProtocolsCacheImpl::originDataToStringForCache(origin_data.value());
EXPECT_EQ(expected_alt_svc, alt_svc);
};

Expand Down Expand Up @@ -301,6 +313,17 @@ TEST_F(AlternateProtocolsCacheImplTest, CacheLoad) {
EXPECT_EQ(protocols1_, protocols.ref());
}

TEST_F(AlternateProtocolsCacheImplTest, CacheLoadSrttOnly) {
EXPECT_CALL(*store_, iterate(_)).WillOnce(Invoke([&](KeyValueStore::ConstIterateCb fn) {
fn("https://hostname1:1", "clear|5");
}));
initialize();

EXPECT_CALL(*store_, addOrUpdate(_, _)).Times(0);
ASSERT_FALSE(protocols_->findAlternatives(origin1_).has_value());
EXPECT_EQ(std::chrono::microseconds(5), protocols_->getSrtt(origin1_));
}

TEST_F(AlternateProtocolsCacheImplTest, ShouldNotUpdateStoreOnCacheLoad) {
EXPECT_CALL(*store_, addOrUpdate(_, _)).Times(0);
EXPECT_CALL(*store_, iterate(_)).WillOnce(Invoke([&](KeyValueStore::ConstIterateCb fn) {
Expand Down

0 comments on commit 8f13d85

Please sign in to comment.