From f75577daab6bcd98f191ad915dd21eb341dd3b95 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 10 Sep 2018 11:52:34 -0700 Subject: [PATCH] upstream: add interfaces for priority/host retry selection extension points (#4212) This adds the necessary configuration and interfaces to register implementations of RetryPriority and RetryHostPredicate, which will allow configuring smarter host selection during retries. Part of #3958 Risk Level: low, api changes Testing: n/a Doc Changes: inline Release Notes:n/a Signed-off-by: Snow Pettersen --- api/envoy/api/v2/route/route.proto | 26 ++++ include/envoy/router/BUILD | 1 + include/envoy/router/router.h | 1 + include/envoy/upstream/BUILD | 18 +++ .../envoy}/upstream/locality.h | 0 include/envoy/upstream/retry.h | 122 ++++++++++++++++++ include/envoy/upstream/upstream.h | 6 + source/common/upstream/BUILD | 13 +- source/common/upstream/eds.h | 2 +- source/common/upstream/upstream_impl.h | 7 +- test/mocks/router/mocks.h | 2 + 11 files changed, 180 insertions(+), 18 deletions(-) rename {source/common => include/envoy}/upstream/locality.h (100%) create mode 100644 include/envoy/upstream/retry.h diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index a1f58891c878..a4c0daaa2f92 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -455,6 +455,32 @@ message RouteAction { // retry policy, a request that times out will not be retried as the total timeout budget // would have been exhausted. google.protobuf.Duration per_try_timeout = 3 [(gogoproto.stdduration) = true]; + + message RetryPriority { + string name = 1 [(validate.rules).string.min_bytes = 1]; + google.protobuf.Struct config = 2; + } + + // [#not-implemented-hide:] + // Specifies an implementation of a RetryPriority which is used to determine the + // distribution of load across priorities used for retries. + RetryPriority retry_priority = 4; + + message RetryHostPredicate { + string name = 1 [(validate.rules).string.min_bytes = 1]; + google.protobuf.Struct config = 2; + } + + // [#not-implemented-hide:] + // Specifies a collection of RetryHostPredicates that will be consulted when selecting a host + // for retries. If any of the predicates reject the host, host selection will be reattempted. + repeated RetryHostPredicate retry_host_predicate = 5; + + // [#not-implemented-hide:] + // The maximum number of times host selection will be reattempted before giving up, at which + // point the host that was last selected will be routed to. If unspecified, this will default to + // retrying once. + int64 host_selection_retry_max_attempts = 6; } // Specifies the idle timeout for the route. If not specified, there is no per-route idle timeout, diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index 6b623640cee8..57c0a82fca82 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -44,6 +44,7 @@ envoy_cc_library( "//include/envoy/http:websocket_interface", "//include/envoy/tracing:http_tracer_interface", "//include/envoy/upstream:resource_manager_interface", + "//include/envoy/upstream:retry_interface", "//source/common/protobuf", "//source/common/protobuf:utility_lib", "@envoy_api//envoy/api/v2:rds_cc", diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index a69c40225191..004f6b097a2d 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -16,6 +16,7 @@ #include "envoy/http/websocket.h" #include "envoy/tracing/http_tracer.h" #include "envoy/upstream/resource_manager.h" +#include "envoy/upstream/retry.h" #include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" diff --git a/include/envoy/upstream/BUILD b/include/envoy/upstream/BUILD index e59ddb8a392f..4ef8fd8181de 100644 --- a/include/envoy/upstream/BUILD +++ b/include/envoy/upstream/BUILD @@ -75,6 +75,15 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "locality_lib", + hdrs = ["locality.h"], + deps = [ + "//source/common/protobuf:utility_lib", + "@envoy_api//envoy/api/v2/core:base_cc", + ], +) + envoy_cc_library( name = "outlier_detection_interface", hdrs = ["outlier_detection.h"], @@ -84,6 +93,14 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "retry_interface", + hdrs = ["retry.h"], + deps = [ + "//include/envoy/upstream:upstream_interface", + ], +) + envoy_cc_library( name = "resource_manager_interface", hdrs = ["resource_manager.h"], @@ -101,6 +118,7 @@ envoy_cc_library( deps = [ ":health_check_host_monitor_interface", ":load_balancer_type_interface", + ":locality_lib", ":resource_manager_interface", "//include/envoy/common:callback", "//include/envoy/http:codec_interface", diff --git a/source/common/upstream/locality.h b/include/envoy/upstream/locality.h similarity index 100% rename from source/common/upstream/locality.h rename to include/envoy/upstream/locality.h diff --git a/include/envoy/upstream/retry.h b/include/envoy/upstream/retry.h new file mode 100644 index 000000000000..ac7e4d9db90a --- /dev/null +++ b/include/envoy/upstream/retry.h @@ -0,0 +1,122 @@ +#pragma once + +#include "envoy/upstream/upstream.h" + +namespace Envoy { +namespace Upstream { + +// Redeclare this here in order to get around cyclical dependencies. +typedef std::vector PriorityLoad; + +/** + * Used to optionally modify the PriorityLoad when selecting a priority for + * a retry attempt. + * + * Each RetryPriority will live throughout the lifetime of a request and updated + * with attempted hosts through onHostAttempted. + */ +class RetryPriority { +public: + virtual ~RetryPriority() {} + + /** + * Determines what PriorityLoad to use. + * + * @param priority_state current state of cluster. + * @param original_priority the unmodified PriorityLoad. + * @return a reference to the PriorityLoad to use. Return original_priority if no changes should + * be made. + */ + virtual PriorityLoad& determinePriorityLoad(const PriorityState& priority_state, + const PriorityLoad& original_priority) PURE; + + /** + * Called after a host has been attempted but before host selection for the next attempt has + * begun. + * + * @param attempted_host the host that was previously attempted. + */ + virtual void onHostAttempted(HostSharedPtr attempted_host) PURE; +}; + +typedef std::shared_ptr RetryPrioritySharedPtr; + +/** + * Used to decide whether a selected host should be rejected during retries. Host selection will be + * reattempted until either the host predicate accepts the host or a configured max number of + * attempts is reached. + * + * Each RetryHostPredicate will live throughout the lifetime of a request and updated + * with attempted hosts through onHostAttempted. + */ +class RetryHostPredicate { +public: + virtual ~RetryHostPredicate() {} + + /** + * Determines whether a host should be rejected during host selection. + * + * @param candidate_host the host to either reject or accept. + * @return whether the host should be rejected and host selection reattempted. + */ + virtual bool shouldSelectAnotherHost(const Host& candidate_host) PURE; + + /** + * Called after a host has been attempted but before host selection for the next attempt has + * begun. + * + * @param attempted_host the host that was previously attempted. + */ + virtual void onHostAttempted(HostSharedPtr attempted_host) PURE; +}; + +typedef std::shared_ptr RetryHostPredicateSharedPtr; + +/** + * Callbacks given to a RetryPriorityFactory that allows adding retry filters. + */ +class RetryPriorityFactoryCallbacks { +public: + virtual ~RetryPriorityFactoryCallbacks() {} + + /** + * Called by the factory to add a RetryPriority. + */ + virtual void addRetryPriority(RetryPrioritySharedPtr filter) PURE; +}; + +/** + * Callbacks given to a RetryHostPredicateFactory that allows adding retry filters. + */ +class RetryHostPredicateFactoryCallbacks { +public: + virtual ~RetryHostPredicateFactoryCallbacks() {} + + /** + * Called by the factory to add a RetryHostPredicate. + */ + virtual void addHostPredicate(RetryHostPredicateSharedPtr filter) PURE; +}; + +/** + * Factory for RetryPriority. + */ +class RetryPriorityFactory { +public: + virtual ~RetryPriorityFactory() {} + + virtual void createRetryPriority(RetryPriorityFactoryCallbacks& callbacks) PURE; +}; + +/** + * Factory for RetryHostPredicate. + */ +class RetryHostPredicateFactory { +public: + virtual ~RetryHostPredicateFactory() {} + + virtual void createHostPredicate(RetryHostPredicateFactoryCallbacks& callbacks) PURE; +}; + +} // namespace Upstream +} // namespace Envoy diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 7f4d68b06131..7765c150b9f4 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -18,6 +18,7 @@ #include "envoy/stats/stats.h" #include "envoy/upstream/health_check_host_monitor.h" #include "envoy/upstream/load_balancer_type.h" +#include "envoy/upstream/locality.h" #include "envoy/upstream/outlier_detection.h" #include "envoy/upstream/resource_manager.h" @@ -160,6 +161,11 @@ typedef std::vector HostVector; typedef std::shared_ptr HostVectorSharedPtr; typedef std::shared_ptr HostVectorConstSharedPtr; +typedef std::unique_ptr HostListPtr; +typedef std::unordered_map + LocalityWeightsMap; +typedef std::vector> PriorityState; + /** * Bucket hosts by locality. */ diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 18559ccc3bf9..e2a541564790 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -186,15 +186,6 @@ envoy_cc_library( ], ) -envoy_cc_library( - name = "locality_lib", - hdrs = ["locality.h"], - deps = [ - "//source/common/protobuf:utility_lib", - "@envoy_api//envoy/api/v2/core:base_cc", - ], -) - envoy_cc_library( name = "logical_dns_cluster_lib", srcs = ["logical_dns_cluster.cc"], @@ -288,13 +279,13 @@ envoy_cc_library( srcs = ["eds.cc"], hdrs = ["eds.h"], deps = [ - ":locality_lib", ":sds_subscription_lib", ":upstream_includes", "//include/envoy/config:grpc_mux_interface", "//include/envoy/config:subscription_interface", "//include/envoy/local_info:local_info_interface", "//include/envoy/secret:secret_manager_interface", + "//include/envoy/upstream:locality_lib", "//source/common/config:metadata_lib", "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", @@ -399,6 +390,7 @@ envoy_cc_library( "//include/envoy/upstream:cluster_manager_interface", "//include/envoy/upstream:health_checker_interface", "//include/envoy/upstream:load_balancer_interface", + "//include/envoy/upstream:locality_lib", "//include/envoy/upstream:upstream_interface", "//source/common/common:callback_impl_lib", "//source/common/common:enum_to_int", @@ -407,7 +399,6 @@ envoy_cc_library( "//source/common/config:well_known_names", "//source/common/stats:isolated_store_lib", "//source/common/stats:stats_lib", - "//source/common/upstream:locality_lib", "//source/server:init_manager_lib", "//source/server:transport_socket_config_lib", "@envoy_api//envoy/api/v2/core:base_cc", diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index dbd40ac98f3f..5372c9542bc5 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -6,8 +6,8 @@ #include "envoy/local_info/local_info.h" #include "envoy/secret/secret_manager.h" #include "envoy/stats/scope.h" +#include "envoy/upstream/locality.h" -#include "common/upstream/locality.h" #include "common/upstream/upstream_impl.h" namespace Envoy { diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 87e035413b1b..1d9d5f80cb67 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -25,6 +25,7 @@ #include "envoy/upstream/cluster_manager.h" #include "envoy/upstream/health_checker.h" #include "envoy/upstream/load_balancer.h" +#include "envoy/upstream/locality.h" #include "envoy/upstream/upstream.h" #include "common/common/callback_impl.h" @@ -35,7 +36,6 @@ #include "common/network/utility.h" #include "common/stats/isolated_store_impl.h" #include "common/upstream/load_balancer_impl.h" -#include "common/upstream/locality.h" #include "common/upstream/outlier_detection_impl.h" #include "common/upstream/resource_manager_impl.h" @@ -553,11 +553,6 @@ class ClusterImplBase : public Cluster, protected Logger::Loggable HostListPtr; -typedef std::unordered_map - LocalityWeightsMap; -typedef std::vector> PriorityState; - /** * Manages PriorityState of a cluster. PriorityState is a per-priority binding of a set of hosts * with its corresponding locality weight map. This is useful to store priorities/hosts/localities diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 90626b4a35a7..ddc04144ae0c 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -166,6 +166,8 @@ class MockVirtualHost : public VirtualHost { MOCK_CONST_METHOD0(corsPolicy, const CorsPolicy*()); MOCK_CONST_METHOD0(routeConfig, const Config&()); MOCK_CONST_METHOD1(perFilterConfig, const RouteSpecificFilterConfig*(const std::string&)); + MOCK_METHOD0(retryPriority, Upstream::RetryPrioritySharedPtr()); + MOCK_METHOD0(retryHostPredicate, Upstream::RetryHostPredicateSharedPtr()); std::string name_{"fake_vhost"}; testing::NiceMock rate_limit_policy_;