Skip to content

Commit

Permalink
Enable load balancing policy extensions (#17400)
Browse files Browse the repository at this point in the history
Enables `LOAD_BALANCING_POLICY_CONFIG` enum value in `LbPolicy` and supports typed load balancers specified in `load_balancing_policy`. Continues work done by Charlie Getzen <[email protected]> in #15827.

Custom load balancers specified by `load_balancing_policy` are created as implementations of `ThreadAwareLoadBalancer`. Thread-local load balancers can be implemented as thread-aware load balancers that contain no logic at the thread-aware level, i.e. the purpose of the thread-aware LB is solely to contain the factory used to instantiate the thread-local LBs. (In the future it might be appropriate to provide a construct that abstracts away thread-aware aspects of `ThreadAwareLoadBalancer` for LBs that don't need to be thread-aware.)

A cluster that uses `LOAD_BALANCING_POLICY_CONFIG` may not also set a subset LB configuration. If the load balancer type makes use of subsetting, it should include a subset configuration in its own configuration message. Future work on load balancing extensions should include moving the subset LB to use load balancing extensions.

Similarly, a cluster that uses `LOAD_BALANCING_POLICY_CONFIG` may not set the `CommonLbConfig`, and it is not passed into load balancer creation (mostly owing to its dubious applicability as a top level configuration message to hierarchical load balancing policy). If the load balancer type makes use of the `CommonLbConfig`, it should include a `CommonLbConfig` in the configuration message for the load balancing policy.

Considerations for migration of existing load balancers:

- pieces of the `ThreadAwareLoadBalancerBase` implementation are specific to the built-in hashing load balancers and should be moved into a base class specifically for hashing load balancers. As it stands, custom load balancing policies are required to implement a `createLoadBalancer()` method even if the architecture of the LB policy does not require a hashing load balancer. I think we would also benefit from disentangling `ThreadAwareLoadBalancerBase` from `LoadBalancerBase`, as the former never actually does any host picking.

- as we convert existing thread-local load balancers to thread-aware load balancers, new local LBs will be re-created upon membership changes. We should provide a mechanism allowing load balancers to control whether this rebuild should occur, e.g. a callback that calls `create()` for thread-aware LBs by default, which can be overridden to do nothing for thread-local LBs.

Risk Level: low
Testing: brought up a cluster with a custom load balancer specified by `load_balancing_policy`; new unit tests included
Docs Changes: n/a
Release Notes: Enable load balancing policy extensions
Platform Specific Features: n/a
Fixes #5598

Signed-off-by: Eugene Chan <[email protected]>
  • Loading branch information
pianiststickman authored Aug 14, 2021
1 parent 5acb1d0 commit e31bc1e
Show file tree
Hide file tree
Showing 17 changed files with 360 additions and 48 deletions.
18 changes: 7 additions & 11 deletions api/envoy/config/cluster/v3/cluster.proto
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ message Cluster {
// this option or not.
CLUSTER_PROVIDED = 6;

// [#not-implemented-hide:] Use the new :ref:`load_balancing_policy
// Use the new :ref:`load_balancing_policy
// <envoy_v3_api_field_config.cluster.v3.Cluster.load_balancing_policy>` field to determine the LB policy.
// [#next-major-version: In the v3 API, we should consider deprecating the lb_policy field
// and instead using the new load_balancing_policy field as the one and only mechanism for
Expand Down Expand Up @@ -720,8 +720,7 @@ message Cluster {

// The :ref:`load balancer type <arch_overview_load_balancing_types>` to use
// when picking a host in the cluster.
// [#comment:TODO: Remove enum constraint :ref:`LOAD_BALANCING_POLICY_CONFIG<envoy_v3_api_enum_value_config.cluster.v3.Cluster.LbPolicy.LOAD_BALANCING_POLICY_CONFIG>` when implemented.]
LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true not_in: 7}];
LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true}];

// Setting this is required for specifying members of
// :ref:`STATIC<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.STATIC>`,
Expand Down Expand Up @@ -1008,7 +1007,7 @@ message Cluster {
// servers of this cluster.
repeated Filter filters = 40;

// [#not-implemented-hide:] New mechanism for LB policy configuration. Used only if the
// New mechanism for LB policy configuration. Used only if the
// :ref:`lb_policy<envoy_v3_api_field_config.cluster.v3.Cluster.lb_policy>` field has the value
// :ref:`LOAD_BALANCING_POLICY_CONFIG<envoy_v3_api_enum_value_config.cluster.v3.Cluster.LbPolicy.LOAD_BALANCING_POLICY_CONFIG>`.
LoadBalancingPolicy load_balancing_policy = 41;
Expand Down Expand Up @@ -1073,7 +1072,7 @@ message Cluster {
bool connection_pool_per_downstream_connection = 51;
}

// [#not-implemented-hide:] Extensible load balancing policy configuration.
// Extensible load balancing policy configuration.
//
// Every LB policy defined via this mechanism will be identified via a unique name using reverse
// DNS notation. If the policy needs configuration parameters, it must define a message for its
Expand All @@ -1099,14 +1098,11 @@ message LoadBalancingPolicy {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.LoadBalancingPolicy.Policy";

reserved 2;

reserved "config";
reserved 2, 1, 3;

// Required. The name of the LB policy.
string name = 1;
reserved "config", "name", "typed_config";

google.protobuf.Any typed_config = 3;
core.v3.TypedExtensionConfig typed_extension_config = 4;
}

// Each client will iterate over the list in order and stop at the first policy that it
Expand Down
18 changes: 7 additions & 11 deletions api/envoy/config/cluster/v4alpha/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions envoy/upstream/load_balancer.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,5 +173,26 @@ class ThreadAwareLoadBalancer {

using ThreadAwareLoadBalancerPtr = std::unique_ptr<ThreadAwareLoadBalancer>;

/**
* Factory for (thread-aware) load balancers. To support a load balancing policy of
* LOAD_BALANCING_POLICY_CONFIG, at least one load balancer factory corresponding to a policy in
* load_balancing_policy must be registered with Envoy. Envoy will use the first policy for which
* it has a registered factory.
*/
class TypedLoadBalancerFactory : public Config::UntypedFactory {
public:
~TypedLoadBalancerFactory() override = default;

/**
* @return ThreadAwareLoadBalancerPtr a new thread-aware load balancer.
*/
virtual ThreadAwareLoadBalancerPtr
create(const PrioritySet& priority_set, ClusterStats& stats, Stats::Scope& stats_scope,
Runtime::Loader& runtime, Random::RandomGenerator& random,
const ::envoy::config::cluster::v3::LoadBalancingPolicy_Policy& lb_policy) PURE;

std::string category() const override { return "envoy.load_balancers"; }
};

} // namespace Upstream
} // namespace Envoy
3 changes: 2 additions & 1 deletion envoy/upstream/load_balancer_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ enum class LoadBalancerType {
RingHash,
OriginalDst,
Maglev,
ClusterProvided
ClusterProvided,
LoadBalancingPolicyConfig
};

/**
Expand Down
15 changes: 15 additions & 0 deletions envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,8 @@ using ProtocolOptionsConfigConstSharedPtr = std::shared_ptr<const ProtocolOption
*/
class ClusterTypedMetadataFactory : public Envoy::Config::TypedMetadataFactory {};

class TypedLoadBalancerFactory;

/**
* Information about a given upstream cluster.
*/
Expand Down Expand Up @@ -786,6 +788,19 @@ class ClusterInfo {
return std::dynamic_pointer_cast<const Derived>(extensionProtocolOptions(name));
}

/**
* @return const envoy::config::cluster::v3::LoadBalancingPolicy_Policy& the load balancing policy
* to use for this cluster.
*/
virtual const envoy::config::cluster::v3::LoadBalancingPolicy_Policy&
loadBalancingPolicy() const PURE;

/**
* @return the load balancer factory for this cluster if the load balancing type is
* LOAD_BALANCING_POLICY_CONFIG.
*/
virtual TypedLoadBalancerFactory* loadBalancerFactory() const PURE;

/**
* @return const envoy::config::cluster::v3::Cluster::CommonLbConfig& the common configuration for
* all load balancers for this cluster.
Expand Down
16 changes: 8 additions & 8 deletions generated_api_shadow/envoy/config/cluster/v3/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 7 additions & 11 deletions generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions source/common/upstream/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,14 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "load_balancer_factory_base_lib",
hdrs = ["load_balancer_factory_base.h"],
deps = [
":load_balancer_lib",
],
)

envoy_cc_library(
name = "load_stats_reporter_lib",
srcs = ["load_stats_reporter.cc"],
Expand Down
8 changes: 8 additions & 0 deletions source/common/upstream/cluster_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,13 @@ ClusterManagerImpl::loadCluster(const envoy::config::cluster::v3::Cluster& clust
}
} else if (cluster_reference.info()->lbType() == LoadBalancerType::ClusterProvided) {
cluster_entry_it->second->thread_aware_lb_ = std::move(new_cluster_pair.second);
} else if (cluster_reference.info()->lbType() == LoadBalancerType::LoadBalancingPolicyConfig) {
const auto& policy = cluster_reference.info()->loadBalancingPolicy();
TypedLoadBalancerFactory* typed_lb_factory = cluster_reference.info()->loadBalancerFactory();
RELEASE_ASSERT(typed_lb_factory != nullptr, "ClusterInfo should contain a valid factory");
cluster_entry_it->second->thread_aware_lb_ =
typed_lb_factory->create(cluster_reference.prioritySet(), cluster_reference.info()->stats(),
cluster_reference.info()->statsScope(), runtime_, random_, policy);
}

updateClusterCounts();
Expand Down Expand Up @@ -1363,6 +1370,7 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::ClusterEntry(
break;
}
case LoadBalancerType::ClusterProvided:
case LoadBalancerType::LoadBalancingPolicyConfig:
case LoadBalancerType::RingHash:
case LoadBalancerType::Maglev:
case LoadBalancerType::OriginalDst: {
Expand Down
29 changes: 29 additions & 0 deletions source/common/upstream/load_balancer_factory_base.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#pragma once

#include "envoy/upstream/load_balancer.h"

namespace Envoy {
namespace Upstream {

/**
* Base class for cluster provided load balancers and load balancers specified by load balancing
* policy config. This class should be extended directly if the load balancing policy specifies a
* thread-aware load balancer.
*
* TODO: provide a ThreadLocalLoadBalancer construct to abstract away thread-awareness from load
* balancing extensions that don't require it.
*/
class TypedLoadBalancerFactoryBase : public TypedLoadBalancerFactory {
public:
// Upstream::TypedLoadBalancerFactory
std::string name() const override { return name_; }

protected:
TypedLoadBalancerFactoryBase(const std::string& name) : name_(name) {}

private:
const std::string name_;
};

} // namespace Upstream
} // namespace Envoy
4 changes: 2 additions & 2 deletions source/common/upstream/subset_lb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -781,8 +781,8 @@ SubsetLoadBalancer::PrioritySubsetImpl::PrioritySubsetImpl(const SubsetLoadBalan

case LoadBalancerType::OriginalDst:
case LoadBalancerType::ClusterProvided:
// LoadBalancerType::OriginalDst is blocked in the factory. LoadBalancerType::ClusterProvided
// is impossible because the subset LB returns a null load balancer from its factory.
case LoadBalancerType::LoadBalancingPolicyConfig:
// These load balancer types can only be created when there is no subset configuration.
NOT_REACHED_GCOVR_EXCL_LINE;
}

Expand Down
Loading

0 comments on commit e31bc1e

Please sign in to comment.