Skip to content
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

Many services in a single namespace leads to assorted problems. #8498

Closed
mattmoor opened this issue Jun 27, 2020 · 12 comments
Closed

Many services in a single namespace leads to assorted problems. #8498

mattmoor opened this issue Jun 27, 2020 · 12 comments
Labels
area/API API objects and controllers kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@mattmoor
Copy link
Member

mattmoor commented Jun 27, 2020

/area API
/area autoscale

What version of Knative?

HEAD

Description of the problem.

I wanted to push on our limits a bit, and so I wrote the very innovative (patent pending 🤣 ) script below. I plotted the latency between creationTimestamp and status.conditions[Ready].lastTransitionTime here.

A few observations in this context:

  1. Revision creation latency creeps up over time.
  2. (anecdotally by curling) the cold start latency of ALL ksvcs creeps up over time (from 2-3s to 10-12s, with all time being spent between "Container create" and "Container start")
  3. After 1198 services were deployed, I started seeing deployments fail with:
RevisionFailed: Revision "foo-1200-sjwyy-1" failed with message: Container failed with: standard_init_linux.go:211: exec user process caused "argument list too long
  1. When the above happened, we stop being able to cold start new services (they crash loop with the same message)!

Here's where it gets interesting... On a whim, I tried picking back up in a second namespace, and things work! Not only do they work, but cold start latency for the new services is back down!

Steps to Reproduce the Problem

I needed a GKE cluster with at least 10 nodes (post-master resize) to tolerate the number of services this creates. I was playing with this in the context of mink, but there's no reason that would affect what I'm seeing.

#!/bin/bash -e

for i in $(seq 1 1500); do
  kn service create foo-$i --image=gcr.io/knative-samples/autoscale-go:0.1
  sleep 10
done

I gathered the latencies as CSV with:

kubectl get ksvc -ojsonpath='{range .items[*]}{.metadata.name},{.metadata.creationTimestamp},{.status.conditions[?(@.type=="Ready")].lastTransitionTime}{"\n"}{end}' | pbcopy
@mattmoor mattmoor added the kind/bug Categorizes issue or PR as related to a bug. label Jun 27, 2020
@mattmoor mattmoor added this to the 0.17.x milestone Jun 27, 2020
@knative-prow-robot knative-prow-robot added the area/API API objects and controllers label Jun 27, 2020
@mattmoor
Copy link
Member Author

Given the sheer volume of services we create, I have the sneaking suspicion that this is related to the service link environment variables injected by k8s. @dprotaso has a change to allow us to use enableServiceLinks: false here.

I am going to try pulling that in and see if it alleviates these problems. If it does, then I think we have our compelling reason to make this default configurable.

cc @vagababov @markusthoemmes @duglin

@vagababov
Copy link
Contributor

Yeah Dave's change should help with this

@tshak
Copy link

tshak commented Jun 27, 2020

enableServiceLinks: false still feels like the right default to me, especially if it's causing issues like this. I really can't imagine anyone using knative relying on Docker links behavior. If they do we have a switch now where it can be enabled in the PodSpec. Lest us not forget...

Built by codifying the best practices shared by successful real-world implementations...

@mattmoor
Copy link
Member Author

@tshak Ack, when this was raised previously we didn't really have tangible evidence that this could cause correctness or performance problems at scale. I am running a test now that should confirm/deny that impact, and then we can revisit in the WG call. However, I think the sentiment from the previous WG call stands: we need to handle this like a change to "stable" behavior.

Assuming we have a smoking gun...

I'd suggest:

  1. Make the default configurable,
  2. Announce the intent to change the default (and why), and advise folks interested to specify an explicit default (and how).
  3. Wait until the release specified in 2.
  4. Change the default and advise folks how to restore the previous behavior.

I don't see a super compelling reason to remove the configurable default (since it enables "K8s compat mode"), but I think these issues would be enough to sway my view on the default behavior.

I'll report back in a few hundred more services 😉

@mattmoor
Copy link
Member Author

Alright, I still need to confirm that we can push beyond 1200 services without the standard_init_linux.go problem, but I think we have enough to declare a smoking gun.

This is the before picture (effectively enableServiceLinks: true):
image

This is the after picture (enableServiceLinks: false):
image

On top of that, not only is the cold-start latency for services better, but it seems like it is a noticeable improvement over where we are at HEAD today with nothing on the cluster 🤩

time curl http://foo-xdsps.default.ducktagon.io?sleep=100
Slept for 100.13 milliseconds.
real    0m2.016s
user    0m0.007s
sys     0m0.011s


time curl http://foo-z74zf.default.ducktagon.io?sleep=100
Slept for 100.25 milliseconds.
real    0m1.895s
user    0m0.008s
sys     0m0.009s

I deployed a ksvc with the original method on top of these 1000 ksvcs (it was slow!):

kn service create foo-svc-links --image=gcr.io/knative-samples/autoscale-go:0.1
Creating service 'foo-svc-links' in namespace 'default':

  0.119s Configuration "foo-svc-links" is waiting for a Revision to become ready.
  0.131s unsuccessfully observed a new generation
  0.154s Configuration "foo-svc-links" is waiting for a Revision to become ready.
 22.734s ...
 22.861s Ingress has not yet been reconciled.
 23.549s Waiting for load balancer to be ready
 25.077s Ready to serve.

Service 'foo-svc-links' created with latest revision 'foo-svc-links-fxfmm-1' and URL:
http://foo-svc-links.default.ducktagon.io

I waited for this to scale to zero:

time curl http://foo-svc-links.default.ducktagon.io?sleep=100
Slept for 100.14 milliseconds.

real    0m13.042s
user    0m0.008s
sys     0m0.009s

My next step is to keep cranking out services and see if I can push past 1200 (where foo-svc-links should fail) and see if the ksvcs without service links keep working.

@mattmoor
Copy link
Member Author

Alright, I have confirmed the last piece of this. Above 1200 services, we fail to cold start with service links, but without we still see pretty exceptional cold starts...

time curl http://foo-z9gs2.default.ducktagon.io?sleep=100
Slept for 100.12 milliseconds.

real    0m1.698s
user    0m0.008s
sys     0m0.009s

@mattmoor
Copy link
Member Author

cc @markusthoemmes

@mattmoor
Copy link
Member Author

Overnight I did a fresh run creating 1500 ksvc (with kn as above, but with service links disabled by default thanks to @vagababov), and the Revision creation latency is completely flat:
image

... bear in mind that the original graph was only 1000 ksvc.

So I think we should start a discussion around how we want to change this default behavior. Thoughts?

@mattmoor
Copy link
Member Author

Closing in favor of #8563

@duglin
Copy link

duglin commented Oct 17, 2022

@mattmoor do you happen to remember if this issue was strictly due to K8s Services env vars being injected, or whether it would have happened even if the user defined the same number of env vars manually?

@mattmoor
Copy link
Member Author

@duglin I'd guess it'd happen with either

@duglin
Copy link

duglin commented Oct 17, 2022

ok thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

5 participants