Support slow Start mode in Envoy #13176

nezdolik · 2020-09-18T17:28:45Z

Signed-off-by: Kateryna Nezdolii [email protected]

Support progressive traffic increase in Envoy, implementation is according to design doc: https://docs.google.com/document/d/1NiG1X0gbfFChjl1aL-EE1hdfYxKErjJ2688wJZaj5a0/edit

Additional Description: Please refer to RFC
Risk Level: Medium
Testing: Done
Docs Changes: Done
Release Notes: Done
Fixes #11050

repokitteh-read-only · 2020-09-18T17:28:51Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #13176 was opened by nezdolik.

see: more, trace.

nezdolik · 2020-09-18T17:30:25Z

Planning to use callback mechanism for edf loadbalncer to be aware of which hosts are in slow start mode.

Signed-off-by: Kateryna Nezdolii <[email protected]>

mattklein123 · 2020-09-21T14:53:45Z

@nezdolik lmk when you want a first pass on this!

/wait

htuch · 2020-09-22T00:09:22Z

api/envoy/config/cluster/v3/cluster.proto

+    // Configuration for slow start mode.
+    // [#next-free-field: 3]
+    message SlowStartConfig {
+      google.protobuf.UInt32Value slow_start_window = 1;


Please add comment to fields

thanks for review @htuch, i will fix api+docs once PR is in more mature state.

htuch · 2020-09-22T00:09:30Z

api/envoy/config/cluster/v3/cluster.proto

@@ -508,6 +508,18 @@ message Cluster {
      google.protobuf.UInt32Value hash_balance_factor = 2 [(validate.rules).uint32 = {gte: 100}];
    }

+    enum EndpointWarmingPolicy {
+      NO_WAIT = 0;


Please add comment to enum values.

htuch · 2020-09-22T00:10:55Z

api/envoy/config/cluster/v3/cluster.proto

+      WAIT_FOR_FIRST_PASSING_HC = 1;
+    }
+
+    // Configuration for slow start mode.


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.

sschepens · 2020-09-24T17:24:50Z

I'm interested in this, would WAIT_FOR_FIRST_PASSING_HC apply when endpoint health is provided via EDS and no active HC is configured?

Signed-off-by: Kateryna Nezdolii <[email protected]>

nezdolik · 2020-10-05T18:10:58Z

@sschepens currently this is the logic for WAIT_FOR_FIRST_PASSING_HC:

if (host->health() == Upstream::Host::Health::Healthy &&
            !host->healthFlagGet(Host::HealthFlag::PENDING_ACTIVE_HC))

This may be not he final version, but currently it checks host health flags. If passive HC keeps those flags up to date it should work.

nezdolik · 2020-10-05T18:17:10Z

@mattklein123 @snowp need your initial thoughts on suggested approach for tracking hosts in slow start. The code is still wip and plenty of things will be reworked (eg todos, duplicated code, fix format etc).
Upon creation of edf lb base, we install 2 callbacks that recalculate hosts in slow start on each priority update/cluster membership update. To track hosts in slow start, added collection (set of host ptrs) and time of latest added hosts in slow start. Keeping the time of latest added host in slow start allows to do fast check if any hosts are in slow start.

source/common/upstream/load_balancer_impl.cc

mattklein123

Thanks for working on this. The shape of this LGTM but I would definitely be interested in hearing from @snowp @antoniovicente @tonya11en if they have other impl ideas. Thank you!

/wait

include/envoy/upstream/host_description.h

source/common/upstream/cluster_manager_impl.cc

mattklein123 · 2020-10-05T23:20:18Z

source/common/upstream/load_balancer_impl.h

+      endpoint_warming_policy;
+  const uint32_t slow_start_window;
+  TimeSource& time_source_;
+  absl::node_hash_set<HostSharedPtr> hosts_in_slow_start_;


You should be able to use a flat_hash_set here.

it's using absl::btree_set now

mattklein123 · 2020-10-05T23:21:49Z

source/common/upstream/load_balancer_impl.cc

+  // If all hosts are out of the window, we no longer need to track them and therefore we erase
+  // tracked hosts set.
+  if (current_time - latest_host_added_time > slow_start_window_ms) {
+    hosts_in_slow_start_.erase(hosts_in_slow_start_.begin(), hosts_in_slow_start_.end());


Just use clear()

source/common/upstream/load_balancer_impl.cc

nezdolik · 2020-10-06T09:30:07Z

Just realised that storing only time of latest added host will not work, for example in case host is added to the cluster and then immediately removed. It needs to be more complex data structure, that supports ordering by time, querying for latest time and lookups by host.

mattklein123 · 2020-10-06T15:04:53Z

Just realised that storing only time of latest added host will not work, for example in case host is added to the cluster and then immediately removed. It needs to be more complex data structure, that supports ordering by time, querying for latest time and lookups by host.

Assuming we stick with the high level approach, I think you could probably use absl::btree_set with a heterogeneous lookup implementation that allows storing a host and sorting on last addition time.

Signed-off-by: Kateryna Nezdolii <[email protected]>

nezdolik · 2021-09-28T22:22:41Z

Applied the latest review comments, fingers crossed that all checks will pass

Signed-off-by: Kateryna Nezdolii <[email protected]>

nezdolik · 2021-09-29T08:21:45Z

@mattklein123 am not sure why envoy-presubmit failed (looking at the logs it says operation was cancelled)

mattklein123

Awesome, thanks. Just one small comment and then let's ship!

/wait

mattklein123 · 2021-09-29T15:45:04Z

test/integration/stats_integration_test.cc

-  // 2021/08/15  17290                40349   add all host map to priority set for fast host
-  //                                          searching


Is this a merge issue? I don't think this should be deleted?

Signed-off-by: Kateryna Nezdolii <[email protected]>

mattklein123

Thanks, awesome work!

tonya11en · 2021-09-30T19:32:47Z

🎉 🎉 🎉

wbpcode · 2021-10-01T01:05:21Z

🎉 🎉 🎉 It's great.

costimuraru · 2021-10-01T07:24:57Z

Woohoo!

du2016 · 2021-10-08T02:33:16Z

🎉 🎉 🎉

ZPerling · 2021-10-19T06:58:10Z

🎉 🎉 🎉 Coooooooool

Support slow Start mode in Envoy (envoyproxy#13176)

* feat: cherry pick commit 6c56dd8 Support slow Start mode in Envoy (envoyproxy#13176) * fix: fix compile error * fix: run proto_format.sh * fix: include file error fix * fix: remove undefined fields * fix: assign lb_round_robin_config_ in ClusterInfoImpl constructor * fix: change applySlowStartFactor to use static 0.3 * host_weight as new weight because formula way will result too small new weight which cause the edf will wait a long time to choose the slow start endpoint Co-authored-by: jst <[email protected]>

ramaraochavali · 2022-12-20T11:56:19Z

@nezdolik one question on this - I understand that for new deployments, when all the pods are in slow start mode, all of them receive similar amount based on their host weights - so slow start mode is essentially not useful in that case and mostly would make sense if new pods come in HPA case. Is that correct?

nezdolik · 2022-12-20T13:05:01Z

@nezdolik one question on this - I understand that for new deployments, when all the pods are in slow start mode, all of them receive similar amount based on their host weights - so slow start mode is essentially not useful in that case and mostly would make sense if new pods come in HPA case. Is that correct?

Correct @ramaraochavali

ramaraochavali · 2022-12-20T13:31:43Z

Thank you

repokitteh-read-only bot added the api label Sep 18, 2020

mattklein123 assigned mattklein123 and snowp Sep 18, 2020

Support slow Start mode in Envoy

4173b08

Signed-off-by: Kateryna Nezdolii <[email protected]>

nezdolik force-pushed the slow-start branch from 8516d7e to 4173b08 Compare September 20, 2020 21:35

repokitteh-read-only bot added the waiting label Sep 21, 2020

htuch reviewed Sep 22, 2020

View reviewed changes

Kateryna Nezdolii added 2 commits September 29, 2020 11:32

Support slow Start mode in Envoy

2f8dad0

Signed-off-by: Kateryna Nezdolii <[email protected]>

Introduce creation_time field into host description

627c910

Signed-off-by: Kateryna Nezdolii <[email protected]>

repokitteh-read-only bot removed the waiting label Sep 29, 2020

mattklein123 added the waiting label Sep 30, 2020

Propagate timeSource to edf lb

ed27cb7

Signed-off-by: Kateryna Nezdolii <[email protected]>

repokitteh-read-only bot removed the waiting label Sep 30, 2020

Kateryna Nezdolii added 2 commits October 1, 2020 12:44

Fix weight adjustment formula

80cd8eb

Signed-off-by: Kateryna Nezdolii <[email protected]>

Draft: Track hosts in slow start mode in edf lb

161cbaf

Signed-off-by: Kateryna Nezdolii <[email protected]>

mattklein123 added the waiting label Oct 1, 2020

Track hosts in slow start mode in edf lb

d7f395c

Signed-off-by: Kateryna Nezdolii <[email protected]>

repokitteh-read-only bot removed the waiting label Oct 5, 2020

mattklein123 added the waiting label Oct 5, 2020

nezdolik commented Oct 5, 2020

View reviewed changes

source/common/upstream/load_balancer_impl.cc Outdated Show resolved Hide resolved

nezdolik commented Oct 5, 2020

View reviewed changes

source/common/upstream/load_balancer_impl.cc Show resolved Hide resolved

mattklein123 requested changes Oct 5, 2020

View reviewed changes

Apply review comments

e4a3c84

Signed-off-by: Kateryna Nezdolii <[email protected]>

nezdolik dismissed tonya11en’s stale review via e4a3c84 September 28, 2021 22:17

repokitteh-read-only bot removed the waiting label Sep 28, 2021

Merge branch 'main' into slow-start

5c587e9

fix format

7f4b258

Signed-off-by: Kateryna Nezdolii <[email protected]>

mattklein123 requested changes Sep 29, 2021

View reviewed changes

repokitteh-read-only bot added the waiting label Sep 29, 2021

nezdolik mentioned this pull request Sep 30, 2021

Start slow start window on first successful HC #18351

Closed

Fix merge error

9ed50d9

Signed-off-by: Kateryna Nezdolii <[email protected]>

repokitteh-read-only bot removed the waiting label Sep 30, 2021

mattklein123 approved these changes Sep 30, 2021

View reviewed changes

repokitteh-read-only bot removed the api label Sep 30, 2021

mattklein123 merged commit 6c56dd8 into envoyproxy:main Sep 30, 2021

jiangshantao-dbg pushed a commit to istio-mt/envoy that referenced this pull request Jan 4, 2022

feat: cherry pick commit 6c56dd8

554ec07

Support slow Start mode in Envoy (envoyproxy#13176)

wrowe mentioned this pull request Jul 8, 2022

Big edf entry deadline when slow start enabled. #22054

Closed

nickcaballero mentioned this pull request Feb 15, 2023

Support Envoy slow start mode kgateway-dev/kgateway#7807

Closed

adisuissa mentioned this pull request Jul 18, 2023

edf-lb: internal refactor - convert hostWeight() to const #28464

Merged

KBaichoo mentioned this pull request Nov 4, 2024

Slow start configuration understanding #36961

Closed

leosarra mentioned this pull request Dec 3, 2024

Slow start mode for Ring-hash load balancer #37484

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support slow Start mode in Envoy #13176

Support slow Start mode in Envoy #13176

nezdolik commented Sep 18, 2020 •

edited

Loading

repokitteh-read-only bot commented Sep 18, 2020

nezdolik commented Sep 18, 2020

mattklein123 commented Sep 21, 2020

htuch Sep 22, 2020

nezdolik Sep 29, 2020

htuch Sep 22, 2020

htuch Sep 22, 2020

sschepens commented Sep 24, 2020

nezdolik commented Oct 5, 2020

nezdolik commented Oct 5, 2020

mattklein123 left a comment

mattklein123 Oct 5, 2020

nezdolik Feb 4, 2021

mattklein123 Oct 5, 2020

nezdolik commented Oct 6, 2020

mattklein123 commented Oct 6, 2020

nezdolik commented Sep 28, 2021

nezdolik commented Sep 29, 2021

mattklein123 left a comment

mattklein123 Sep 29, 2021

nezdolik Sep 30, 2021

mattklein123 left a comment

tonya11en commented Sep 30, 2021

wbpcode commented Oct 1, 2021

costimuraru commented Oct 1, 2021

du2016 commented Oct 8, 2021

ZPerling commented Oct 19, 2021

ramaraochavali commented Dec 20, 2022

nezdolik commented Dec 20, 2022

ramaraochavali commented Dec 20, 2022

		// 2021/08/15 17290 40349 add all host map to priority set for fast host
		// searching

Support slow Start mode in Envoy #13176

Support slow Start mode in Envoy #13176

Conversation

nezdolik commented Sep 18, 2020 • edited Loading

repokitteh-read-only bot commented Sep 18, 2020

nezdolik commented Sep 18, 2020

mattklein123 commented Sep 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sschepens commented Sep 24, 2020

nezdolik commented Oct 5, 2020

nezdolik commented Oct 5, 2020

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nezdolik commented Oct 6, 2020

mattklein123 commented Oct 6, 2020

nezdolik commented Sep 28, 2021

nezdolik commented Sep 29, 2021

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

tonya11en commented Sep 30, 2021

wbpcode commented Oct 1, 2021

costimuraru commented Oct 1, 2021

du2016 commented Oct 8, 2021

ZPerling commented Oct 19, 2021

ramaraochavali commented Dec 20, 2022

nezdolik commented Dec 20, 2022

ramaraochavali commented Dec 20, 2022

nezdolik commented Sep 18, 2020 •

edited

Loading