-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support slow Start mode in Envoy #13176
Changes from 1 commit
4173b08
2f8dad0
627c910
ed27cb7
80cd8eb
161cbaf
d7f395c
944607e
f966f8b
f61216c
6bfb2e0
a02698d
23e517e
a4f697d
d0f2cd2
3ab3951
e5a8534
e9e93ea
a365f6e
2156dc3
fe0e551
38f792a
bbc3fda
7d1cdb4
18f0463
3cf6f9a
1510abb
c038daf
c4b8f8b
bd87893
0963656
33737f8
0cbdbe7
43b2f54
4e8b9d7
78be70e
dc1bb99
5d5d231
fdbbd5f
b371ece
1602a7b
bf32ee5
9d96d4b
f1670a9
7a495e0
941a43e
3467ca4
bd467d6
7d8022d
49cd453
c6f2b86
514dabf
96d7b76
6a98431
74557b9
1a23da6
3e4f49a
4a2a508
875c763
ccc9338
19d288d
2766a4f
a2b1261
5e18212
2e9d0ff
2002d00
2128535
224daa2
b3c5c43
4d3efe7
3ed1ff4
e4a3c84
5c587e9
7f4b258
9ed50d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -440,7 +440,7 @@ message Cluster { | |
} | ||
|
||
// Common configuration for all load balancer implementations. | ||
// [#next-free-field: 8] | ||
// [#next-free-field: 9] | ||
message CommonLbConfig { | ||
option (udpa.annotations.versioning).previous_message_type = | ||
"envoy.api.v2.Cluster.CommonLbConfig"; | ||
|
@@ -508,6 +508,18 @@ message Cluster { | |
google.protobuf.UInt32Value hash_balance_factor = 2 [(validate.rules).uint32 = {gte: 100}]; | ||
} | ||
|
||
enum EndpointWarmingPolicy { | ||
NO_WAIT = 0; | ||
WAIT_FOR_FIRST_PASSING_HC = 1; | ||
} | ||
|
||
// Configuration for slow start mode. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you write some Envoy docs for this and link from here? I'd suggest translating the design doc into RST and then cleaning that up a bit for end users. |
||
// [#next-free-field: 3] | ||
message SlowStartConfig { | ||
google.protobuf.UInt32Value slow_start_window = 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add comment to fields There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for review @htuch, i will fix api+docs once PR is in more mature state. |
||
EndpointWarmingPolicy endpoint_warming_policy = 2; | ||
} | ||
|
||
// Configures the :ref:`healthy panic threshold <arch_overview_load_balancing_panic_threshold>`. | ||
// If not specified, the default is 50%. | ||
// To disable panic mode, set to 0%. | ||
|
@@ -565,6 +577,9 @@ message Cluster { | |
|
||
// Common Configuration for all consistent hashing load balancers (MaglevLb, RingHashLb, etc.) | ||
ConsistentHashingLbConfig consistent_hashing_lb_config = 7; | ||
|
||
// Configuration for slow start mode. | ||
mattklein123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SlowStartConfig slow_start_config = 8; | ||
} | ||
|
||
message RefreshRate { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -461,7 +461,16 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, | |
least_request_config.has_value() && least_request_config->has_active_request_bias() | ||
? std::make_unique<Runtime::Double>(least_request_config->active_request_bias(), | ||
runtime) | ||
: nullptr) { | ||
: nullptr), | ||
// todo(nezdolik) move this to base class | ||
endpoint_warming_policy(common_config.has_slow_start_config() | ||
? common_config.slow_start_config().endpoint_warming_policy() | ||
: envoy::config::cluster::v3::Cluster::CommonLbConfig::NO_WAIT), | ||
// todo(nezdolik) move this to base class | ||
slow_start_window(common_config.has_slow_start_config() | ||
? PROTOBUF_GET_WRAPPED_OR_DEFAULT(common_config.slow_start_config(), | ||
slow_start_window, 0) | ||
: 0) { | ||
initialize(); | ||
} | ||
|
||
|
@@ -521,18 +530,27 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, | |
double active_request_bias_{}; | ||
|
||
const std::unique_ptr<Runtime::Double> active_request_bias_runtime_; | ||
const envoy::config::cluster::v3::Cluster::CommonLbConfig::EndpointWarmingPolicy | ||
endpoint_warming_policy; | ||
const uint32_t slow_start_window; | ||
}; | ||
|
||
/** | ||
* Random load balancer that picks a random host out of all hosts. | ||
*/ | ||
class RandomLoadBalancer : public ZoneAwareLoadBalancerBase { | ||
class RandomLoadBalancer : public ZoneAwareLoadBalancerBase, | ||
Logger::Loggable<Logger::Id::upstream> { | ||
public: | ||
RandomLoadBalancer(const PrioritySet& priority_set, const PrioritySet* local_priority_set, | ||
ClusterStats& stats, Runtime::Loader& runtime, Random::RandomGenerator& random, | ||
const envoy::config::cluster::v3::Cluster::CommonLbConfig& common_config) | ||
: ZoneAwareLoadBalancerBase(priority_set, local_priority_set, stats, runtime, random, | ||
common_config) {} | ||
common_config) { | ||
if (common_config.has_slow_start_config()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then does this slow-start config even belong in the common LB config? Seems to defeat the purpose. You ought to just add the slow-start config message to the supported LBs and avoid these checks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have not started this one |
||
// todo(nezdolik) maybe use error status | ||
ENVOY_LOG(warn, "Slow start mode is not supported for random lb"); | ||
} | ||
} | ||
|
||
// Upstream::LoadBalancerBase | ||
HostConstSharedPtr chooseHostOnce(LoadBalancerContext* context) override; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,9 @@ RingHashLoadBalancer::RingHashLoadBalancer( | |
throw EnvoyException(fmt::format("ring hash: minimum_ring_size ({}) > maximum_ring_size ({})", | ||
min_ring_size_, max_ring_size_)); | ||
} | ||
if (common_config.has_slow_start_config()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I really don't think this belongs in the common config. let's just add the message to the supported LBs so we don't need to do this. |
||
throw EnvoyException("Slow start mode is not supported for ring hash lb"); | ||
} | ||
} | ||
|
||
RingHashLoadBalancerStats RingHashLoadBalancer::generateStats(Stats::Scope& scope) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,10 @@ SubsetLoadBalancer::SubsetLoadBalancer( | |
scale_locality_weight_(subsets.scaleLocalityWeight()), list_as_any_(subsets.listAsAny()) { | ||
ASSERT(subsets.isEnabled()); | ||
|
||
if (common_config.has_slow_start_config()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Subset + Roundrobin /LeastRequest is supported. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
throw EnvoyException("Slow start mode is not supported for subset lb"); | ||
} | ||
|
||
if (fallback_policy_ != envoy::config::cluster::v3::Cluster::LbSubsetConfig::NO_FALLBACK) { | ||
HostPredicate predicate; | ||
if (fallback_policy_ == envoy::config::cluster::v3::Cluster::LbSubsetConfig::ANY_ENDPOINT) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comment to enum values.