Disable client-side rate-limiting when AP&F is enabled

[negz]

What would you like to be added?

I'd like client-side rate limiting to be disabled in client-go when API server-side Priority and Fairness is enabled.

All client-go based Kubernetes clients are configured by default to use a token bucket rate limiter in an attempt to avoid overloading the API server. This is a form of open-loop control, in that the clients don't factor in the load of the API server in deciding whether to rate-limit themselves.

Per the below links the default rate limits for a particular client is currently 5 qps, bursting to 10 qps. Discovery clients are particularly noisy and have thus had their burst raised most recently to 300 qps. I believe this number was picked because it's around the number of API groups that need to be discovered (i.e. HTTP request that need to be made in short order) when @crossplane is installed with all of the "big three" providers (i.e. CRDs representing all AWS, Azure, and GCP APIs).

• https://github.com/kubernetes/client-go/blob/a890e7b/rest/config.go#L44 - Typical clients
• https://github.com/kubernetes/cli-runtime/blob/b48c51e/pkg/genericclioptions/config_flags.go#L452 - Discovery clients

API Priority and Fairness has been enabled by default (as a beta feature) since Kubernetes v1.20. It allows the API server to prioritize and queue requests, and can let a particular client know when it should limit its request rate by returning a response with HTTP status code 429 "Too Many Requests". REST clients appear to respect this status code and will backoff their requests when they encounter it per https://github.com/kubernetes/client-go/blob/a890e7b/rest/urlbackoff.go.

Disabling client-side rate-limiting appears to be a goal of AP&F per its graduation criteria:

PF allows us to disable client-side rate limiting without causing the apiservers to wedge/crash. Note that there is another level of concern that APF does not attempt to address, which is mismatch between the throughput that various controllers can sustain.

There seems to be a general consensus amongst API machinery maintainers that we should just stop worrying and learn to love AP&F per #105520 (comment).

Why is this needed?

The open-loop nature of client-go's rate limiter means that it's possible (indeed likely) that clients will rate limit themselves too much or not enough. The former is particularly painful for CLI tools like kubectl, helm, and kpt where a series of requests taking longer than they need to directly results in a particular CLI command taking longer than it needs to. Again this is an area where discovery is particularly painful - when kubectl's discovery burst was set to 100 qps we were seeing it take more than 5 minutes for some commands to complete, with users seeing the below logs while waiting for their command to finish.

Waited for 1.033772408s due to client-side throttling, not priority and fairness, request: GET:https://api.example.org/apis/pkg.crossplane.io/v1?timeout=32s

To this end several tools have bumped or disabled their client-side rate limits, e.g.:

• Add --burst-limit option for client-side throttling limit configuration helm/helm#10842
• Adjust Discovery throttling kptdev/kpt#3368
• Support discovery cache or up both QPS/BurstQPS tektoncd/cli#1641
• ✨ Default QPS to a saner value in and out of tests kubernetes-sigs/controller-runtime#365

Notably I'm pretty sure @crossplane will be exceeding the 300 qps discovery burst fairly soon, which will result in another round of PRs to bump the limit further if we don't remove it entirely.

[negz]

/sig api-machinery

[negz]

/cc @apelisse @MikeSpreitzer

[negz]

/kind cleanup

Not sure if CI will let me make this change, but it feels more appropriate than "feature".

[MadhavJivrajani]

Related: #109614
/cc @wojtek-t @tkashem

[MikeSpreitzer]

This raises some questions.

1. Since enabling/disabling APF is done independently at each server, what is the desired behavior when the servers are not all the same in this regard?
2. How does the client know whether the servers have APF enabled or disabled?

[MikeSpreitzer]

Here are some suggested answers.

1. For a given request, the client reading the response can tell whether the server has APF enabled by looking for the X-Kubernetes-Pf-Flowschema-Uid and X-Kubernetes-Pf-Prioritylevel-Uid headers.
2. A client that has not gotten any response (yet, or in the last hour) assumes that the servers do not have APF enabled and applies client-side rate limiting.
3. If some responses have been received in the last hour and all of those indicate that the server has APF enabled, then the client stops applying client-side rate-limiting.
4. If some responses have been received in the last hour and any of those indicates that APF is not enabled, then the client applies client-side rate-limiting.

[negz]

How does the client know whether the servers have APF enabled or disabled?

There seems to be some prior art for this in kubernetes-sigs/cli-utils#584 (CC @karlkfi)

[MikeSpreitzer]

The technique in kubernetes-sigs/cli-utils#584 :

1. Looks at a response header like I suggested.
2. Adds an extra request per client.
3. Assumes that all the servers have the same configuration and it does not change over time.

[karlkfi]

FWIW, the impl I wrote seems to work pretty well in kpt when used by humans, but when using something similar in Config Sync (a multi-instance GitOps operator) we've been noticing that the load produced is very high, especially with many instances in parallel, and the default P&F config doesn't seem configured to be able to mitigate the load, forcing users to make their own P&F configs, which they generally don't know how to do. So I'm not sure this approach or P&F are actually mature enough or easy enough to use to propagate this all K8s clients. You might end up with a lot of broken clusters.

That said, I'd love for someone to pursue it and report issues to help improve P&F with this kind of dogfooding.

[negz]

I'd love for someone to pursue it and report issues to help improve P&F with this kind of dogfooding.

Yeah, that's my underlying motive here - to discover what needs to be done to feel comfortable removing these rate limits.

[negz]

we've been noticing that the load produced is very high, especially with many instances in parallel, and the default P&F config doesn't seem configured to be able to mitigate the load, forcing users to make their own P&F configs, which they generally don't know how to do.

@karlkfi Can you elaborate on what you see when the load gets high? Just reading through the docs I'm guessing all Config Sync instances are getting put into the workload-low priority level. Is the issue that this priority level isn't granular enough and that other instances at the same level are experiencing issues (time spent in queues? 429s?)? Are you seeing stuff in workload-low affecting other (lower?) priority levels?

[karlkfi]

Unfortunately, the problem is only partially diagnosed and a customer issue, so I can't share the exact details. I haven't had a chance to reproduce it myself yet, due to a number of other related load causing issues. But part of the issue is with thousands of clients all hammering the apiserver with the same P&F config, due to the multi-instance way Config Sync handles namespace tenancy.

[MikeSpreitzer]

If you are finding APF is not working well then that is very interesting indeed. Let me see if I understand the bad symptom. It is the apiserver(s) using a lot of CPU?

Thousands of the same sort of client is normal in a large cluster. The self-protection is based on the number of requests that the server actually works on at once, with the rest queued or rejected, so simply increasing the number of some sort of client should not defeat that.

What do the offending requests do? Any insight into the pathology, or even how to reproduce it, would be great.

[leilajal]

/triage accepted

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[negz]

/remove-lifecycle stale

[taliastocks]

Is this change still planned? It doesn't look like there has been movement in over a year.

[Jefftree]

@negz: Aggregated Discovery (kubernetes/enhancements#3352) has been in beta since 1.27 and effectively reduces the number of discovery requests from an unbounded amount to 2. Is this still an issue anymore for crossplane?

[karlkfi]

For what it's worth, we implemented this a year and a half ago in Config Sync v1.12.0 and it's seems to work fine. It detects server-side throttling and disables client-side throttling.

Other than one initial customer complaint that seemed to be caused by a severely overloaded and underpowered apiserver, I haven't had any more customer complaints.

One notable downside tho is that it punts configuration of the server-side throttling config to the cluster admin, and most have no experience configuring it. The defaults are probably ok for many, but we haven't done the extensive testing required to figure out what edge cases require configuration. This shifts some responsibility from tenant to admin in multi-tenant use cases.

Another minor downside is that the detection requires making an api call, which can fail, needs its own recovery mechanism, and has to be run before other calls, which complicates app startup. Delaying detection is also challenging due to the way config-runtime and client-go have different ways of constructing clients from config. So there might still be some value to having this detection built-in.

https://github.com/GoogleContainerTools/kpt-config-sync/blob/1cf82eaa4c27016c38fa9ed3f4316aa2d8b52074/pkg/client/restconfig/restconfig.go#L102

[negz]

Is this still an issue anymore for crossplane?

It's not currently a big issue for Crossplane, but I'd still advocate for doing it.

[bboreham]

currently 5 qps, bursting to 10 qps.

Nitpick: the unit for burst is "q", not per second.

Since requests are sent one at a time, this limit only comes into effect after an idle period. E.g. with 5qps rate limit, if you send no requests for 2 seconds then the burst lets you send 10 as fast as you can before limiting again.