Skip to content

Commit

Permalink
upstream: add interfaces for priority/host retry selection extension …
Browse files Browse the repository at this point in the history
…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 <snowp@squareup.com>
  • Loading branch information
snowp authored and zuercher committed Sep 10, 2018
1 parent 89d8a20 commit f75577d
Show file tree
Hide file tree
Showing 11 changed files with 180 additions and 18 deletions.
26 changes: 26 additions & 0 deletions api/envoy/api/v2/route/route.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions include/envoy/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
18 changes: 18 additions & 0 deletions include/envoy/upstream/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand All @@ -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"],
Expand All @@ -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",
Expand Down
File renamed without changes.
122 changes: 122 additions & 0 deletions include/envoy/upstream/retry.h
Original file line number Diff line number Diff line change
@@ -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<uint32_t> 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<RetryPriority> 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<RetryHostPredicate> 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
6 changes: 6 additions & 0 deletions include/envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -160,6 +161,11 @@ typedef std::vector<HostSharedPtr> HostVector;
typedef std::shared_ptr<HostVector> HostVectorSharedPtr;
typedef std::shared_ptr<const HostVector> HostVectorConstSharedPtr;

typedef std::unique_ptr<HostVector> HostListPtr;
typedef std::unordered_map<envoy::api::v2::core::Locality, uint32_t, LocalityHash, LocalityEqualTo>
LocalityWeightsMap;
typedef std::vector<std::pair<HostListPtr, LocalityWeightsMap>> PriorityState;

/**
* Bucket hosts by locality.
*/
Expand Down
13 changes: 2 additions & 11 deletions source/common/upstream/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion source/common/upstream/eds.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 1 addition & 6 deletions source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"

Expand Down Expand Up @@ -553,11 +553,6 @@ class ClusterImplBase : public Cluster, protected Logger::Loggable<Logger::Id::u
uint64_t pending_initialize_health_checks_{};
};

typedef std::unique_ptr<HostVector> HostListPtr;
typedef std::unordered_map<envoy::api::v2::core::Locality, uint32_t, LocalityHash, LocalityEqualTo>
LocalityWeightsMap;
typedef std::vector<std::pair<HostListPtr, LocalityWeightsMap>> 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
Expand Down
2 changes: 2 additions & 0 deletions test/mocks/router/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<MockRateLimitPolicy> rate_limit_policy_;
Expand Down

0 comments on commit f75577d

Please sign in to comment.