enable deferred cluster creation #50785

ramaraochavali · 2024-05-01T10:40:22Z

Enables deferred cluster creation on worker threads

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

keithmattix

What's the motivation for this?

howardjohn

same question, when I tried in #49868 I saw no benefits

ramaraochavali · 2024-05-01T15:33:28Z

I am seeing improvement in server initialization times with two static clusters and concurrency of 50

server.initialization_time_ms: P0(nan,100) P25(nan,102.5) P50(nan,105) P75(nan,107.5) P90(nan,109) P95(nan,109.5) P99(nan,109.9) P99.5(nan,109.95) P99.9(nan,109.99) P100(nan,110)

server.initialization_time_ms: P0(nan,140) P25(nan,142.5) P50(nan,145) P75(nan,147.5) P90(nan,149) P95(nan,149.5) P99(nan,149.9) P99.5(nan,149.95) P99.9(nan,149.99) P100(nan,150)

Will do more tests and report back

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

ramaraochavali · 2024-05-02T09:29:01Z

Here are the results of my test

Concurrency 2 Clusters 3600

Without Deferred creation

server.memory_allocated: 759852680
server.memory_heap_size: 1067294720
server.memory_physical_size: 834822144

server.initialization_time_ms: P0(nan,46000) P25(nan,46250) P50(nan,46500) P75(nan,46750) P90(nan,46900) P95(nan,46950) P99(nan,46990) P99.5(nan,46995) P99.9(nan,46999) P100(nan,47000)

With Deferred creation

server.memory_allocated: 749908528
server.memory_heap_size: 1056219136
server.memory_physical_size: 853909504

server.initialization_time_ms: P0(nan,36000) P25(nan,36250) P50(nan,36500) P75(nan,36750) P90(nan,36900) P95(nan,36950) P99(nan,36990) P99.5(nan,36995) P99.9(nan,36999) P100(nan,37000)

Concurrency 10 Clusters 3600

Without Deferred creation

server.memory_allocated: 901796592
server.memory_heap_size: 1266057216
server.memory_physical_size: 981704704

server.initialization_time_ms: P0(nan,52000) P25(nan,52250) P50(nan,52500) P75(nan,52750) P90(nan,52900) P95(nan,52950) P99(nan,52990) P99.5(nan,52995) P99.9(nan,52999) P100(nan,53000)

With

server.memory_allocated: 783817120
server.memory_heap_size: 1104920576
server.memory_physical_size: 858816512

server.initialization_time_ms: P0(nan,30000) P25(nan,30250) P50(nan,30500) P75(nan,30750) P90(nan,30900) P95(nan,30950) P99(nan,30990) P99.5(nan,30995) P99.9(nan,30999) P100(nan,31000)

Summary:

We see some improvements both in memory and init times. Out of these 3600 clusters, 400 clusters STRICT_DNS type of clusters and deferred creation does not work for them.

ramaraochavali · 2024-05-02T10:23:52Z

@howardjohn can you please share /stats?usedonly endpoint output of your test when you saw no difference? Please enable all stats of Envoy.

hzxuzhonghu · 2024-05-02T13:32:38Z

please wait if possible. i want to take a look,but i am on holiday this week

ramaraochavali · 2024-05-02T13:45:56Z

sure it is not urgent

hzxuzhonghu · 2024-05-06T04:21:17Z

How does this work in envoy, will it ceate cluster on flight, so will it influence the latency?

ramaraochavali · 2024-05-06T05:25:24Z

Yes. When a request comes on the worker thread, it creates it inline with the already existing object https://github.com/envoyproxy/envoy/blob/8d4cf00e2980aeba11b9b74230a53f92086419bc/source/common/upstream/cluster_manager_impl.cc#L1355. Should not incur lot of latency

costinm

I like this change - and any 'on demand'/'delayed' or scale improving PR - but absolutely not as a default behavior change.

Add an annotation or option or meta - off by default - and it would be great. Gradually we can make it the default - maybe for waypoints first ( no backward compat risks), some classes of gateways - and maybe even sidecars, but over few releases.

kyessenov · 2024-05-06T17:28:24Z

This needs to be gated by a feature flag and reversible. Otherwise, this is probably a good change.

…ed_cluster

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

ramaraochavali · 2024-05-09T10:02:59Z

@kyessenov @costinm added feature flag. PTAL

@howardjohn when you get chance, please share stats of your experiment

howardjohn · 2024-05-09T16:15:31Z

I jsut have #49868 (comment)

howardjohn

LGTM. I am fine with turning it on by default with "turn off for compatibility version". Or we can get more testing then flip it

howardjohn · 2024-05-09T16:16:08Z

releasenotes/notes/deferred_cluster_creation.yaml

+- |
+  **Added** an experimental feature to enable cluster creation on worker threads inline during requests.
+  This will save memory and CPU cycles in cases where there are lots of inactive clusters and > 1 worker thread.
+  This can be enabled by setting ENABLE_DEFERRED_CLUSTER_CREATION to true in Istiod Deployment.


This is actually in the agent not istiod?

Then the feature flag should have its own home not under pilot

Oh yes. I forgot just copy pasted some other flag :-)

Then the feature flag should have its own home not under pilot

We will have to do that refactor

hzxuzhonghu

any latency test result

hzxuzhonghu · 2024-05-10T01:10:45Z

releasenotes/notes/deferred_cluster_creation.yaml

+- |
+  **Added** an experimental feature to enable cluster creation on worker threads inline during requests.
+  This will save memory and CPU cycles in cases where there are lots of inactive clusters and > 1 worker thread.
+  This can be enabled by setting ENABLE_DEFERRED_CLUSTER_CREATION to true in Istiod Deployment.


Then the feature flag should have its own home not under pilot

ramaraochavali · 2024-05-10T04:44:44Z

any latency test result

latency is very negligible based on my measurements

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

ramaraochavali · 2024-05-10T04:57:12Z

compatibility check added.

@costinm can you please review?

wulianglongrd · 2024-05-10T16:21:55Z

releasenotes/notes/deferred_cluster_creation.yaml

+- |
+  **Added** an experimental feature to enable cluster creation on worker threads inline during requests.
+  This will save memory and CPU cycles in cases where there are lots of inactive clusters and > 1 worker thread.
+  This can be disabled by setting ENABLE_DEFERRED_CLUSTER_CREATION to true in agent Deployment.


ENABLE_DEFERRED_CLUSTER_CREATION starts with enable, but true means disable? This confuses me.

sorry. good point, That needs to change after the default has been changed. fixed now

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

hzxuzhonghu · 2024-05-11T03:42:41Z

releasenotes/notes/deferred_cluster_creation.yaml

+- |
+  **Added** an experimental feature to enable cluster creation on worker threads inline during requests.
+  This will save memory and CPU cycles in cases where there are lots of inactive clusters and > 1 worker thread.
+  This can be disabled by setting ENABLE_DEFERRED_CLUSTER_CREATION to false in agent Deployment.


need update, now it is not enabled by default

hzxuzhonghu · 2024-05-11T03:43:25Z

latency is very negligible based on my measurements

I doubt this conclusion

ramaraochavali · 2024-05-11T08:36:28Z

Please see what Envoy is doing when creating cluster in line https://github.com/envoyproxy/envoy/blob/8d4cf00e2980aeba11b9b74230a53f92086419bc/source/common/upstream/cluster_manager_impl.cc#L1355 - I do not think it is doing any significant work on a worker. But feel free to measure with your service

ramaraochavali · 2024-05-14T14:56:45Z

@costinm gentle ping

enable deferred cluster creation

883e9d7

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

ramaraochavali requested a review from a team as a code owner May 1, 2024 10:40

istio-policy-bot added the release-notes-none Indicates a PR that does not require release notes. label May 1, 2024

istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 1, 2024

keithmattix reviewed May 1, 2024

View reviewed changes

howardjohn reviewed May 1, 2024

View reviewed changes

fix bootstrap

702775e

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 2, 2024

ramaraochavali added the do-not-merge Block automatic merging of a PR. label May 2, 2024

costinm requested changes May 6, 2024

View reviewed changes

ramaraochavali added 2 commits May 9, 2024 14:51

Merge remote-tracking branch 'upstream/master' into fix/enable_deferr…

3177699

…ed_cluster

add feature flag

e05099d

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

istio-testing added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 9, 2024

fix yaml

8a1fd92

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

howardjohn approved these changes May 9, 2024

View reviewed changes

hzxuzhonghu reviewed May 10, 2024

View reviewed changes

enable default with compatibility check

8c4c3f3

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

ramaraochavali requested a review from a team as a code owner May 10, 2024 04:56

wulianglongrd reviewed May 10, 2024

View reviewed changes

fix release note

ef7da15

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

hzxuzhonghu reviewed May 11, 2024

View reviewed changes

ramaraochavali removed the do-not-merge Block automatic merging of a PR. label May 14, 2024

costinm approved these changes May 14, 2024

View reviewed changes

istio-testing merged commit 1e73925 into istio:master May 14, 2024
28 checks passed

ramaraochavali deleted the fix/enable_deferred_cluster branch May 15, 2024 05:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable deferred cluster creation #50785

enable deferred cluster creation #50785

ramaraochavali commented May 1, 2024 •

edited by istio-policy-bot

keithmattix left a comment

howardjohn left a comment

ramaraochavali commented May 1, 2024

ramaraochavali commented May 2, 2024 •

edited

ramaraochavali commented May 2, 2024

hzxuzhonghu commented May 2, 2024

ramaraochavali commented May 2, 2024

hzxuzhonghu commented May 6, 2024

ramaraochavali commented May 6, 2024

costinm left a comment

kyessenov commented May 6, 2024

ramaraochavali commented May 9, 2024

howardjohn commented May 9, 2024

howardjohn left a comment

howardjohn May 9, 2024

hzxuzhonghu May 10, 2024

ramaraochavali May 10, 2024

hzxuzhonghu left a comment

hzxuzhonghu May 10, 2024

ramaraochavali commented May 10, 2024

ramaraochavali commented May 10, 2024

wulianglongrd May 10, 2024

ramaraochavali May 11, 2024

hzxuzhonghu May 11, 2024

hzxuzhonghu commented May 11, 2024

ramaraochavali commented May 11, 2024

ramaraochavali commented May 14, 2024

enable deferred cluster creation #50785

enable deferred cluster creation #50785

Conversation

ramaraochavali commented May 1, 2024 • edited by istio-policy-bot

keithmattix left a comment

Choose a reason for hiding this comment

howardjohn left a comment

Choose a reason for hiding this comment

ramaraochavali commented May 1, 2024

ramaraochavali commented May 2, 2024 • edited

ramaraochavali commented May 2, 2024

hzxuzhonghu commented May 2, 2024

ramaraochavali commented May 2, 2024

hzxuzhonghu commented May 6, 2024

ramaraochavali commented May 6, 2024

costinm left a comment

Choose a reason for hiding this comment

kyessenov commented May 6, 2024

ramaraochavali commented May 9, 2024

howardjohn commented May 9, 2024

howardjohn left a comment

Choose a reason for hiding this comment

howardjohn May 9, 2024

Choose a reason for hiding this comment

hzxuzhonghu May 10, 2024

Choose a reason for hiding this comment

ramaraochavali May 10, 2024

Choose a reason for hiding this comment

hzxuzhonghu left a comment

Choose a reason for hiding this comment

hzxuzhonghu May 10, 2024

Choose a reason for hiding this comment

ramaraochavali commented May 10, 2024

ramaraochavali commented May 10, 2024

wulianglongrd May 10, 2024

Choose a reason for hiding this comment

ramaraochavali May 11, 2024

Choose a reason for hiding this comment

hzxuzhonghu May 11, 2024

Choose a reason for hiding this comment

hzxuzhonghu commented May 11, 2024

ramaraochavali commented May 11, 2024

ramaraochavali commented May 14, 2024

ramaraochavali commented May 1, 2024 •

edited by istio-policy-bot

ramaraochavali commented May 2, 2024 •

edited