Admission controllers can cause unnecessary significant load on apiserver

[wojtek-t]

Recently we observed high latencies on POST pods on our scalability tests.
For more context see: #22340

It appeared to be throttling related, e.g.:

W0302 21:38:28.190204       5 request.go:627] Throttling request took 1.281100127s, request: GET:http://127.0.0.1:8080/api/v1/namespaces/e2e-tests-load-6cqci/limitranges
I0302 21:38:28.191235       5 handlers.go:152] GET /api/v1/namespaces/e2e-tests-load-6cqci/limitranges: (853.53µs) 200 [[kube-apiserver/v1.2.0 (linux/amd64) kubernetes/2cf9157] 127.0.0.1:58515]
I0302 21:38:28.193396       5 trace.go:57] Trace "Create /api/v1/namespaces/e2e-tests-load-6cqci/pods" (started 2016-03-02 21:38:26.908795629 +0000 UTC):
[20.412µs] [20.412µs] About to convert to expected version
[239.558µs] [219.146µs] Conversion done
[1.282815499s] [1.282575941s] About to store object in database
[1.284377959s] [1.56246ms] Object stored in database
[1.284389884s] [11.925µs] Self-link added
[1.284548334s] [158.45µs] END

As an example look at resource quota admission controller.
In #20446 there was some fallback added to use lru cache in case of on objects.
However, not what happens if we are sending multiple POST pods at the same time to apiserver.
What happens in Admit() if there are no results in Indexer, we list ResourceQuotas.
But if there are multiple calls at the same time, there can be multiple LISTs before the cache is updated:
https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/resourcequota/admission.go#L131
But this can cause throttling of these requests.

What we should do is: if there is List() in flight, we should wait until it is finished and lookupCache is updated.

@derekwaynecarr

[derekwaynecarr]

@wojtek-t - thanks for the analysis. @deads2k - fyi, this is an interesting side-effect of the LRU cache pattern, not sure if we have adopted it in other places as well, but good to keep in mind.

[derekwaynecarr]

To clarify, it was adopted in LimitRange and ResourceQuota admission plug-ins in separate PRs.

[wojtek-t]

@derekwaynecarr - it took me some time, but was useful experience. Thanks for assigning yourself.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle rotten
/remove-lifecycle stale

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[aimuz]

/reopen

[k8s-ci-robot]

@aimuz: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.