From f96e7fae5b96e1c4e13f6327bcc1029dad75b0bb Mon Sep 17 00:00:00 2001 From: David Vo Date: Fri, 17 Jul 2026 10:02:12 -0400 Subject: [PATCH] happy_eyeballs: sort upstream address list once per host Previously every upstream connection attempt to a multi-address host re-ran the RFC 8305 address interleave in HappyEyeballsConnectionProvider's constructor, allocating a fresh sorted vector, per family buckets, and a family order vector each time. The address list and the cluster happy eyeballs config are immutable between address list updates, so the sort now runs once: HostDescriptionImpl caches the sorted list at construction and LogicalHost recomputes it under its existing address lock when addresses are refreshed. The provider receives the pre-sorted shared vector and no longer sorts or copies it, and addressListOrNull() still returns the original raw order for its existing consumers. Connection attempt order is unchanged because the same sortAddresses function runs, just earlier and once. Fixes #32916 Signed-off-by: David Vo --- ...happy_eyeballs__sort-address-list-once.rst | 3 + .../network/happy_eyeballs_connection_impl.cc | 21 ++++--- .../network/happy_eyeballs_connection_impl.h | 34 +++++----- source/common/upstream/upstream_impl.cc | 39 +++++++----- source/common/upstream/upstream_impl.h | 32 +++++++++- .../clusters/common/logical_host.cc | 25 +++++--- .../extensions/clusters/common/logical_host.h | 8 +++ test/common/network/BUILD | 16 +++++ .../network/happy_eyeballs_speed_test.cc | 38 ++++++++++++ test/common/upstream/upstream_impl_test.cc | 62 +++++++++++++++++++ .../clusters/common/logical_host_test.cc | 47 ++++++++++++++ 11 files changed, 273 insertions(+), 52 deletions(-) create mode 100644 changelogs/current/minor_behavior_changes/happy_eyeballs__sort-address-list-once.rst create mode 100644 test/common/network/happy_eyeballs_speed_test.cc diff --git a/changelogs/current/minor_behavior_changes/happy_eyeballs__sort-address-list-once.rst b/changelogs/current/minor_behavior_changes/happy_eyeballs__sort-address-list-once.rst new file mode 100644 index 0000000000000..0081d98f1adbb --- /dev/null +++ b/changelogs/current/minor_behavior_changes/happy_eyeballs__sort-address-list-once.rst @@ -0,0 +1,3 @@ +The happy eyeballs sorting of a multi-address host's address list now happens once when the +address list is created or refreshed, instead of on every upstream connection attempt. The order +in which connection attempts are made is unchanged. diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 6785ab546ff4c..4bc66c2f7754e 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -10,22 +10,23 @@ namespace Envoy { namespace Network { HappyEyeballsConnectionProvider::HappyEyeballsConnectionProvider( - Event::Dispatcher& dispatcher, const std::vector& address_list, + Event::Dispatcher& dispatcher, + const Upstream::HostDescription::SharedConstAddressVector& sorted_address_list, const std::shared_ptr& upstream_local_address_selector, UpstreamTransportSocketFactory& socket_factory, TransportSocketOptionsConstSharedPtr transport_socket_options, const Upstream::HostDescriptionConstSharedPtr& host, - const ConnectionSocket::OptionsSharedPtr options, - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config) - : dispatcher_(dispatcher), address_list_(sortAddresses(address_list, happy_eyeballs_config)), + const ConnectionSocket::OptionsSharedPtr options) + : dispatcher_(dispatcher), address_list_(sorted_address_list), upstream_local_address_selector_(upstream_local_address_selector), socket_factory_(socket_factory), transport_socket_options_(transport_socket_options), - host_(host), options_(options) {} + host_(host), options_(options) { + ASSERT(address_list_ != nullptr && !address_list_->empty()); +} bool HappyEyeballsConnectionProvider::hasNextConnection() { - return next_address_ < address_list_.size(); + return next_address_ < address_list_->size(); } ClientConnectionPtr HappyEyeballsConnectionProvider::createNextConnection(const uint64_t id) { @@ -37,8 +38,8 @@ ClientConnectionPtr HappyEyeballsConnectionProvider::createNextConnection(const first_connection_created_ = true; ASSERT(hasNextConnection()); ENVOY_LOG_EVENT(debug, "happy_eyeballs_cx_attempt", "C[{}] address={}", id, - address_list_[next_address_]->asStringView()); - auto& address = address_list_[next_address_++]; + (*address_list_)[next_address_]->asStringView()); + auto& address = (*address_list_)[next_address_++]; auto upstream_local_address = upstream_local_address_selector_->getUpstreamLocalAddress( address, options_, makeOptRefFromPtr(transport_socket_options_.get())); @@ -50,7 +51,7 @@ ClientConnectionPtr HappyEyeballsConnectionProvider::createNextConnection(const size_t HappyEyeballsConnectionProvider::nextConnection() { return next_address_; } -size_t HappyEyeballsConnectionProvider::totalConnections() { return address_list_.size(); } +size_t HappyEyeballsConnectionProvider::totalConnections() { return address_list_->size(); } namespace { diff --git a/source/common/network/happy_eyeballs_connection_impl.h b/source/common/network/happy_eyeballs_connection_impl.h index 05758d290cf80..425981d36eb2a 100644 --- a/source/common/network/happy_eyeballs_connection_impl.h +++ b/source/common/network/happy_eyeballs_connection_impl.h @@ -12,22 +12,22 @@ namespace Network { /** * Implementation of ConnectionProvider for HappyEyeballs. It provides client * connections to multiple addresses in an specific order complying to - * HappyEyeballs. + * HappyEyeballs. The address list passed to the constructor must already be + * sorted with sortAddresses(); the host computes this once when its address + * list is created or refreshed rather than on every connection attempt. */ class HappyEyeballsConnectionProvider : public ConnectionProvider, Logger::Loggable { public: HappyEyeballsConnectionProvider( Event::Dispatcher& dispatcher, - const std::vector& address_list, + const Upstream::HostDescription::SharedConstAddressVector& sorted_address_list, const std::shared_ptr& upstream_local_address_selector, UpstreamTransportSocketFactory& socket_factory, TransportSocketOptionsConstSharedPtr transport_socket_options, const Upstream::HostDescriptionConstSharedPtr& host, - const ConnectionSocket::OptionsSharedPtr options, - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config); + const ConnectionSocket::OptionsSharedPtr options); bool hasNextConnection() override; ClientConnectionPtr createNextConnection(const uint64_t id) override; size_t nextConnection() override; @@ -44,8 +44,8 @@ class HappyEyeballsConnectionProvider : public ConnectionProvider, private: Event::Dispatcher& dispatcher_; - // List of addresses to attempt to connect to. - const std::vector address_list_; + // List of addresses to attempt to connect to, pre-sorted with sortAddresses(). + const Upstream::HostDescription::SharedConstAddressVector address_list_; const Upstream::UpstreamLocalAddressSelectorConstSharedPtr upstream_local_address_selector_; UpstreamTransportSocketFactory& socket_factory_; TransportSocketOptionsConstSharedPtr transport_socket_options_; @@ -69,28 +69,26 @@ class HappyEyeballsConnectionProvider : public ConnectionProvider, * they are applied to each open connection and applied when creating new ones. * * See the Happy Eyeballs RFC at https://datatracker.ietf.org/doc/html/rfc6555 - * TODO(RyanTheOptimist): Implement the Happy Eyeballs address sorting algorithm - * either in the class or in the resolution code. + * The address list must already be sorted with + * HappyEyeballsConnectionProvider::sortAddresses(), which the host does once + * when the address list is created or refreshed. */ class HappyEyeballsConnectionImpl : public MultiConnectionBaseImpl, Logger::Loggable { public: HappyEyeballsConnectionImpl( Event::Dispatcher& dispatcher, - const std::vector& address_list, + const Upstream::HostDescription::SharedConstAddressVector& sorted_address_list, const std::shared_ptr& upstream_local_address_selector, UpstreamTransportSocketFactory& socket_factory, TransportSocketOptionsConstSharedPtr transport_socket_options, const Upstream::HostDescriptionConstSharedPtr& host, - const ConnectionSocket::OptionsSharedPtr options, - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config) - : MultiConnectionBaseImpl(dispatcher, - std::make_unique( - dispatcher, address_list, upstream_local_address_selector, - socket_factory, transport_socket_options, host, options, - happy_eyeballs_config)) {} + const ConnectionSocket::OptionsSharedPtr options) + : MultiConnectionBaseImpl( + dispatcher, std::make_unique( + dispatcher, sorted_address_list, upstream_local_address_selector, + socket_factory, transport_socket_options, host, options)) {} }; } // namespace Network diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 763fd02e58b2b..96e06b2efc715 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -519,6 +519,7 @@ HostDescriptionImpl::HostDescriptionImpl( uint32_t priority, const AddressVector& address_list, absl::string_view stat_name) : HostDescriptionImplBase(cluster, hostname, dest_address, endpoint_metadata, locality_metadata, locality, health_check_config, priority, creation_status), + sorted_address_list_or_null_(makeSortedAddressListOrNull(*cluster, address_list)), address_(dest_address), address_list_or_null_(makeAddressListOrNull(dest_address, address_list)), health_check_address_(resolveHealthCheckAddress(health_check_config, dest_address)), @@ -565,6 +566,20 @@ HostDescription::SharedConstAddressVector HostDescriptionImplBase::makeAddressLi return std::make_shared(address_list); } +HostDescription::SharedConstAddressVector +HostDescriptionImplBase::makeSortedAddressListOrNull(const ClusterInfo& cluster, + const AddressVector& address_list) { + if (address_list.size() <= 1) { + return {}; + } + const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& + happy_eyeballs_config = + cluster.happyEyeballsConfig().has_value() ? *cluster.happyEyeballsConfig() + : defaultHappyEyeballsConfig(); + return std::make_shared( + Network::HappyEyeballsConnectionProvider::sortAddresses(address_list, happy_eyeballs_config)); +} + Network::UpstreamTransportSocketFactory& HostDescriptionImplBase::resolveTransportSocketFactory( const Network::Address::InstanceConstSharedPtr& dest_address, const envoy::config::core::v3::Metadata* endpoint_metadata, @@ -590,8 +605,8 @@ Host::CreateConnectionData HostImplBase::createConnection( ? resolveTransportSocketFactory(address(), metadata().get(), transport_socket_options) : transportSocketFactory(); - return createConnection(dispatcher, cluster(), address(), addressListOrNull(), factory, options, - transport_socket_options, shared_from_this()); + return createConnection(dispatcher, cluster(), address(), sortedAddressListOrNull(), factory, + options, transport_socket_options, shared_from_this()); } void HostImplBase::setEdsHealthFlag(envoy::config::core::v3::HealthStatus health_status) { @@ -637,7 +652,7 @@ Host::CreateConnectionData HostImplBase::createOrcaReportingConnection( Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address) const { return createOrcaConnection(dispatcher, std::move(transport_socket_options), factory, - std::move(orca_address), address(), addressListOrNull(), + std::move(orca_address), address(), sortedAddressListOrNull(), shared_from_this()); } @@ -647,13 +662,13 @@ Host::CreateConnectionData HostImplBase::createOrcaConnection( Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address, const Network::Address::InstanceConstSharedPtr& host_address, - const SharedConstAddressVector& address_list, HostDescriptionConstSharedPtr host) const { + const SharedConstAddressVector& sorted_address_list, HostDescriptionConstSharedPtr host) const { // The original-port address list applies only when dialing the host's own address. Compare // by value: pointer identity doesn't survive LogicalHost re-resolution. const bool use_address_list = *orca_address == *host_address; return createConnection(dispatcher, cluster(), orca_address, - use_address_list ? address_list : SharedConstAddressVector{}, factory, - /*options=*/nullptr, transport_socket_options, std::move(host)); + use_address_list ? sorted_address_list : SharedConstAddressVector{}, + factory, /*options=*/nullptr, transport_socket_options, std::move(host)); } std::optional HostImplBase::maybeGetProxyRedirectAddress( @@ -713,7 +728,7 @@ std::optional HostImplBase::maybeGetPr Host::CreateConnectionData HostImplBase::createConnection( Event::Dispatcher& dispatcher, const ClusterInfo& cluster, const Network::Address::InstanceConstSharedPtr& address, - const SharedConstAddressVector& address_list_or_null, + const SharedConstAddressVector& sorted_address_list, Network::UpstreamTransportSocketFactory& socket_factory, const Network::ConnectionSocket::OptionsSharedPtr& options, Network::TransportSocketOptionsConstSharedPtr transport_socket_options, @@ -736,15 +751,11 @@ Host::CreateConnectionData HostImplBase::createConnection( proxy_address.value(), upstream_local_address.address_, socket_factory.createTransportSocket(transport_socket_options, host), upstream_local_address.socket_options_, transport_socket_options); - } else if (address_list_or_null != nullptr && address_list_or_null->size() > 1) { + } else if (sorted_address_list != nullptr && sorted_address_list->size() > 1) { ENVOY_LOG(debug, "Upstream using happy eyeballs config."); - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config = - cluster.happyEyeballsConfig().has_value() ? *cluster.happyEyeballsConfig() - : defaultHappyEyeballsConfig(); connection = std::make_unique( - dispatcher, *address_list_or_null, source_address_selector, socket_factory, - transport_socket_options, host, options, happy_eyeballs_config); + dispatcher, sorted_address_list, source_address_selector, socket_factory, + transport_socket_options, host, options); } else { auto upstream_local_address = source_address_selector->getUpstreamLocalAddress( address, options, makeOptRefFromPtr(transport_socket_options.get())); diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index d253f0794c6f7..9afe2243da32e 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -279,6 +279,17 @@ class HostDescriptionImplBase : virtual public HostDescription, makeAddressListOrNull(const Network::Address::InstanceConstSharedPtr& address, const AddressVector& address_list); + /** + * @return nullptr if address_list has fewer than 2 addresses (happy eyeballs does not + * apply), otherwise a shared_ptr to a copy of address_list sorted with + * Network::HappyEyeballsConnectionProvider::sortAddresses() using the cluster's happy + * eyeballs config, or the default config if the cluster does not specify one. This is + * computed once when the address list is created or refreshed so that connection + * attempts do not re-sort it. + */ + static SharedConstAddressVector makeSortedAddressListOrNull(const ClusterInfo& cluster, + const AddressVector& address_list); + private: ClusterInfoConstSharedPtr cluster_; const std::string hostname_; @@ -347,6 +358,11 @@ class HostDescriptionImpl : public HostDescriptionImplBase { const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, uint32_t priority, const AddressVector& address_list = {}, absl::string_view stat_name = {}); + // Happy eyeballs sorted copy of the address list, or nullptr if the host does not have + // multiple addresses. Set at construction and never changed; read by + // HostImpl::sortedAddressListOrNull(). + const SharedConstAddressVector sorted_address_list_or_null_; + private: // No locks are required in this implementation: all address-related member // variables are set at construction and never change. See @@ -478,10 +494,17 @@ class HostImplBase : public Host, } protected: + /** + * @return the address list sorted for happy eyeballs connection attempts, or nullptr if + * the host does not have multiple addresses. The list is computed once when the address + * list is created or refreshed rather than on every connection attempt. + */ + virtual SharedConstAddressVector sortedAddressListOrNull() const PURE; + static CreateConnectionData createConnection(Event::Dispatcher& dispatcher, const ClusterInfo& cluster, const Network::Address::InstanceConstSharedPtr& address, - const SharedConstAddressVector& address_list, + const SharedConstAddressVector& sorted_address_list, Network::UpstreamTransportSocketFactory& socket_factory, const Network::ConnectionSocket::OptionsSharedPtr& options, Network::TransportSocketOptionsConstSharedPtr transport_socket_options, @@ -497,7 +520,7 @@ class HostImplBase : public Host, Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address, const Network::Address::InstanceConstSharedPtr& host_address, - const SharedConstAddressVector& address_list, + const SharedConstAddressVector& sorted_address_list, HostDescriptionConstSharedPtr host) const; private: @@ -556,6 +579,11 @@ class HostImpl : public HostImplBase, public HostDescriptionImpl { HostDescriptionImpl(creation_status, cluster, hostname, address, endpoint_metadata, locality_metadata, locality, health_check_config, priority, address_list, stat_name) {} + + // Upstream::HostImplBase + SharedConstAddressVector sortedAddressListOrNull() const override { + return sorted_address_list_or_null_; + } }; class HostsPerLocalityImpl : public HostsPerLocality { diff --git a/source/extensions/clusters/common/logical_host.cc b/source/extensions/clusters/common/logical_host.cc index e8d76fef0ad3e..284c6a3172d1e 100644 --- a/source/extensions/clusters/common/logical_host.cc +++ b/source/extensions/clusters/common/logical_host.cc @@ -38,7 +38,8 @@ LogicalHost::LogicalHost( lb_endpoint.endpoint().health_check_config(), locality_lb_endpoint.priority(), creation_status), override_transport_socket_options_(override_transport_socket_options), address_(address), - address_list_or_null_(makeAddressListOrNull(address, address_list)) { + address_list_or_null_(makeAddressListOrNull(address, address_list)), + sorted_address_list_or_null_(makeSortedAddressListOrNull(*cluster, address_list)) { health_check_address_ = resolveHealthCheckAddress(lb_endpoint.endpoint().health_check_config(), address); } @@ -59,10 +60,13 @@ void LogicalHost::setNewAddresses(const Network::Address::InstanceConstSharedPtr shared_address_list = std::make_shared(address_list); ASSERT(*address_list.front() == *address); } + SharedConstAddressVector sorted_address_list = + makeSortedAddressListOrNull(cluster(), address_list); { absl::MutexLock lock(address_lock_); address_ = address; address_list_or_null_ = std::move(shared_address_list); + sorted_address_list_or_null_ = std::move(sorted_address_list); health_check_address_ = std::move(health_check_address); } } @@ -72,6 +76,11 @@ HostDescription::SharedConstAddressVector LogicalHost::addressListOrNull() const return address_list_or_null_; } +HostDescription::SharedConstAddressVector LogicalHost::sortedAddressListOrNull() const { + absl::MutexLock lock(address_lock_); + return sorted_address_list_or_null_; +} + Network::Address::InstanceConstSharedPtr LogicalHost::address() const { absl::MutexLock lock(address_lock_); return address_; @@ -86,11 +95,11 @@ Upstream::Host::CreateConnectionData LogicalHost::createConnection( Event::Dispatcher& dispatcher, const Network::ConnectionSocket::OptionsSharedPtr& options, Network::TransportSocketOptionsConstSharedPtr transport_socket_options) const { Network::Address::InstanceConstSharedPtr address; - SharedConstAddressVector address_list_or_null; + SharedConstAddressVector sorted_address_list_or_null; { absl::MutexLock lock(address_lock_); address = address_; - address_list_or_null = address_list_or_null_; + sorted_address_list_or_null = sorted_address_list_or_null_; } // Use override_transport_socket_options if set, otherwise use the passed options. @@ -109,8 +118,8 @@ Upstream::Host::CreateConnectionData LogicalHost::createConnection( : transportSocketFactory(); return HostImplBase::createConnection( - dispatcher, cluster(), address, address_list_or_null, factory, options, effective_options, - std::make_shared(address, shared_from_this())); + dispatcher, cluster(), address, sorted_address_list_or_null, factory, options, + effective_options, std::make_shared(address, shared_from_this())); } Upstream::Host::CreateConnectionData LogicalHost::createOrcaReportingConnection( @@ -119,17 +128,17 @@ Upstream::Host::CreateConnectionData LogicalHost::createOrcaReportingConnection( Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address) const { Network::Address::InstanceConstSharedPtr host_address; - SharedConstAddressVector address_list_or_null; + SharedConstAddressVector sorted_address_list_or_null; { absl::MutexLock lock(address_lock_); host_address = address_; - address_list_or_null = address_list_or_null_; + sorted_address_list_or_null = sorted_address_list_or_null_; } // The caller's options pass through unchanged; as with health checks, // override_transport_socket_options_ is not consulted here. return createOrcaConnection( dispatcher, transport_socket_options, factory, orca_address, host_address, - address_list_or_null, + sorted_address_list_or_null, std::make_shared(orca_address, shared_from_this())); } diff --git a/source/extensions/clusters/common/logical_host.h b/source/extensions/clusters/common/logical_host.h index 97640bd43cff3..56f16caf65dfc 100644 --- a/source/extensions/clusters/common/logical_host.h +++ b/source/extensions/clusters/common/logical_host.h @@ -59,6 +59,11 @@ class LogicalHost : public HostImplBase, public HostDescriptionImplBase { absl::string_view observabilityName() const override { return {}; } Network::Address::InstanceConstSharedPtr orcaReportingAddress() const override; + // Upstream::HostImplBase + // Public (protected in the base class) so that tests can verify the sorted list is + // kept in sync with the raw list across address refreshes. + SharedConstAddressVector sortedAddressListOrNull() const override; + protected: LogicalHost( const ClusterInfoConstSharedPtr& cluster, const std::string& hostname, @@ -74,6 +79,9 @@ class LogicalHost : public HostImplBase, public HostDescriptionImplBase { // The first entry in the address_list_ should match the value in address_. Network::Address::InstanceConstSharedPtr address_ ABSL_GUARDED_BY(address_lock_); SharedConstAddressVector address_list_or_null_ ABSL_GUARDED_BY(address_lock_); + // Happy eyeballs sorted copy of address_list_or_null_, updated together with it so that + // the raw and sorted lists stay consistent. nullptr unless there are multiple addresses. + SharedConstAddressVector sorted_address_list_or_null_ ABSL_GUARDED_BY(address_lock_); Network::Address::InstanceConstSharedPtr health_check_address_ ABSL_GUARDED_BY(address_lock_); mutable absl::Mutex address_lock_; }; diff --git a/test/common/network/BUILD b/test/common/network/BUILD index b909fb7485f19..bed4b6fe9bd48 100644 --- a/test/common/network/BUILD +++ b/test/common/network/BUILD @@ -133,6 +133,22 @@ envoy_cc_test( ], ) +envoy_cc_benchmark_binary( + name = "happy_eyeballs_speed_test", + srcs = ["happy_eyeballs_speed_test.cc"], + rbe_pool = "6gig", + deps = [ + "//source/common/network:address_lib", + "//source/common/network:happy_eyeballs_connection_impl_lib", + "@benchmark", + ], +) + +envoy_benchmark_test( + name = "happy_eyeballs_speed_test_benchmark_test", + benchmark_binary = "happy_eyeballs_speed_test", +) + envoy_cc_test( name = "multi_connection_base_impl_test", srcs = ["multi_connection_base_impl_test.cc"], diff --git a/test/common/network/happy_eyeballs_speed_test.cc b/test/common/network/happy_eyeballs_speed_test.cc new file mode 100644 index 0000000000000..6287f8dd7a042 --- /dev/null +++ b/test/common/network/happy_eyeballs_speed_test.cc @@ -0,0 +1,38 @@ +// Measures the cost of sorting an upstream address list with address families +// interleaved as per RFC 8305 (happy eyeballs v2). This sort used to run on +// every upstream connection attempt; it now runs once when a host's address +// list is created or refreshed. + +#include "source/common/common/fmt.h" +#include "source/common/network/address_impl.h" +#include "source/common/network/happy_eyeballs_connection_impl.h" + +#include "benchmark/benchmark.h" + +namespace Envoy { +namespace Network { + +static void happyEyeballsSortAddresses(benchmark::State& state) { + const uint64_t num_addresses = state.range(0); + std::vector address_list; + address_list.reserve(num_addresses); + // Build a dual family list interleaving IPv6 and IPv4 addresses. + for (uint64_t i = 0; i < num_addresses; i++) { + if (i % 2 == 0) { + address_list.push_back( + std::make_shared(fmt::format("2001:db8::{}", i + 1), 443)); + } else { + address_list.push_back( + std::make_shared(fmt::format("10.0.0.{}", i + 1))); + } + } + envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig config; + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); + benchmark::DoNotOptimize(HappyEyeballsConnectionProvider::sortAddresses(address_list, config)); + } +} +BENCHMARK(happyEyeballsSortAddresses)->Arg(2)->Arg(8)->Arg(16); + +} // namespace Network +} // namespace Envoy diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index d825e639cbf43..2fc4f6186ae4c 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -2383,6 +2383,68 @@ TEST_F(HostImplTest, CreateConnectionHappyEyeballsWithEmptyConfig) { EXPECT_EQ(host, connection->stream_info_.upstreamInfo()->upstreamHost()); } +// Verifies that the happy eyeballs sort of the address list runs once when the host is +// created, and not again on each connection attempt. +TEST_F(HostImplTest, HappyEyeballsSortsAddressListOncePerHost) { + MockClusterMockPrioritySet cluster; + envoy::config::core::v3::Metadata metadata; + Config::Metadata::mutableMetadataValue(metadata, Config::MetadataFilters::get().ENVOY_LB, + Config::MetadataEnvoyLbKeys::get().CANARY) + .set_bool_value(true); + envoy::config::core::v3::Locality locality; + locality.set_region("oceania"); + locality.set_zone("hello"); + locality.set_sub_zone("world"); + Network::Address::InstanceConstSharedPtr address = + *Network::Utility::resolveUrl("tcp://[1:2:3::4]:8"); + AddressVector address_list = { + address, + *Network::Utility::resolveUrl("tcp://10.0.0.1:1235"), + }; + + // Creating the host sorts the address list exactly once. + std::shared_ptr host; + EXPECT_LOG_CONTAINS_N_TIMES("trace", "sort address with happy_eyeballs config", 1, { + host = std::shared_ptr(*HostImpl::create( + cluster.info_, "lyft.com", address, + std::make_shared(metadata), nullptr, 1, + std::make_shared(locality), + envoy::config::endpoint::v3::Endpoint::HealthCheckConfig::default_instance(), 1, + envoy::config::core::v3::UNKNOWN, address_list)); + }); + + testing::StrictMock dispatcher; + Network::TransportSocketOptionsConstSharedPtr transport_socket_options; + Network::ConnectionSocket::OptionsSharedPtr options; + + auto connection1 = new testing::StrictMock(); + EXPECT_CALL(*connection1, setBufferLimits(0)); + EXPECT_CALL(*connection1, addConnectionCallbacks(_)); + EXPECT_CALL(*connection1, connectionInfoSetter()); + EXPECT_CALL(*connection1, streamInfo()); + auto connection2 = new testing::StrictMock(); + EXPECT_CALL(*connection2, setBufferLimits(0)); + EXPECT_CALL(*connection2, addConnectionCallbacks(_)); + EXPECT_CALL(*connection2, connectionInfoSetter()); + EXPECT_CALL(*connection2, streamInfo()); + // Both connections should be created with the first address in the list. + EXPECT_CALL(dispatcher, createClientConnection_(address_list[0], _, _, _)) + .WillOnce(Return(connection1)) + .WillOnce(Return(connection2)); + EXPECT_CALL(dispatcher, createTimer_(_)).Times(2); + + // Creating connections reuses the sorted list and does not sort again. + Envoy::Upstream::Host::CreateConnectionData connection_data1; + Envoy::Upstream::Host::CreateConnectionData connection_data2; + EXPECT_LOG_CONTAINS_N_TIMES("trace", "sort address with happy_eyeballs config", 0, { + connection_data1 = host->createConnection(dispatcher, options, transport_socket_options); + connection_data2 = host->createConnection(dispatcher, options, transport_socket_options); + }); + // The created connections will be wrapped in HappyEyeballsConnectionImpls. + EXPECT_NE(connection1, connection_data1.connection_.get()); + EXPECT_NE(connection2, connection_data2.connection_.get()); +} + TEST_F(HostImplTest, HealthFlags) { MockClusterMockPrioritySet cluster; HostSharedPtr host = makeTestHost(cluster.info_, "tcp://10.0.0.1:1234", 1); diff --git a/test/extensions/clusters/common/logical_host_test.cc b/test/extensions/clusters/common/logical_host_test.cc index cd55893d16fee..e7f46b2f0dfad 100644 --- a/test/extensions/clusters/common/logical_host_test.cc +++ b/test/extensions/clusters/common/logical_host_test.cc @@ -80,6 +80,53 @@ TEST_F(RealHostDescriptionTest, UnitTest) { description_.setOutlierDetector(std::move(detector_host)); } +// Verifies that the happy eyeballs sorted address list is computed at construction and +// recomputed by setNewAddresses, while addressListOrNull() keeps the original order. +TEST(LogicalHostSortedAddressListTest, SortedListFollowsAddressUpdates) { + auto cluster_info = std::make_shared>(); + const Network::Address::InstanceConstSharedPtr v4_1 = + *Network::Utility::resolveUrl("tcp://10.0.0.1:1234"); + const Network::Address::InstanceConstSharedPtr v4_2 = + *Network::Utility::resolveUrl("tcp://10.0.0.2:1234"); + const Network::Address::InstanceConstSharedPtr v6_1 = + *Network::Utility::resolveUrl("tcp://[1::1]:1234"); + + auto host_or_error = Upstream::LogicalHost::create( + cluster_info, /*hostname=*/"", v4_1, /*address_list=*/{v4_1, v4_2, v6_1}, + envoy::config::endpoint::v3::LocalityLbEndpoints(), envoy::config::endpoint::v3::LbEndpoint(), + /*override_transport_socket_options=*/nullptr); + ASSERT_TRUE(host_or_error.ok()); + auto host = std::shared_ptr(std::move(*host_or_error)); + + // The raw list keeps the original order while the sorted list interleaves address families. + ASSERT_NE(host->addressListOrNull(), nullptr); + EXPECT_EQ(*host->addressListOrNull(), + (Upstream::HostDescription::AddressVector{v4_1, v4_2, v6_1})); + ASSERT_NE(host->sortedAddressListOrNull(), nullptr); + EXPECT_EQ(*host->sortedAddressListOrNull(), + (Upstream::HostDescription::AddressVector{v4_1, v6_1, v4_2})); + + // setNewAddresses recomputes both lists. + const Network::Address::InstanceConstSharedPtr v6_2 = + *Network::Utility::resolveUrl("tcp://[1::2]:1234"); + const Network::Address::InstanceConstSharedPtr v6_3 = + *Network::Utility::resolveUrl("tcp://[1::3]:1234"); + const Network::Address::InstanceConstSharedPtr v4_3 = + *Network::Utility::resolveUrl("tcp://10.0.0.3:1234"); + host->setNewAddresses(v6_2, {v6_2, v6_3, v4_3}, envoy::config::endpoint::v3::LbEndpoint()); + ASSERT_NE(host->addressListOrNull(), nullptr); + EXPECT_EQ(*host->addressListOrNull(), + (Upstream::HostDescription::AddressVector{v6_2, v6_3, v4_3})); + ASSERT_NE(host->sortedAddressListOrNull(), nullptr); + EXPECT_EQ(*host->sortedAddressListOrNull(), + (Upstream::HostDescription::AddressVector{v6_2, v4_3, v6_3})); + + // An update without an address list clears both lists. + host->setNewAddresses(v4_1, {}, envoy::config::endpoint::v3::LbEndpoint()); + EXPECT_EQ(host->addressListOrNull(), nullptr); + EXPECT_EQ(host->sortedAddressListOrNull(), nullptr); +} + // Test fixture for LogicalHost per-connection transport socket resolution. class LogicalHostTransportSocketResolutionTest : public testing::Test { public: