Proposal: More reliable resource quota enforcement #20113
Conversation
k8s-github-robot
added
kind/design
| any corresponding reservation made by that object will be removed | ||
| since its consumption will properly be tracked in usage. If a | ||
| reservation has expired, it will be removed from the quota document. | ||
| A default expirationTime could be applied to sufficiently mitigate |
There was a problem hiding this comment.
Will apiserever.T6 fail if the corresponding quota expires?
| to allow for optimistic locking. If the quota usage | ||
| was stale, the admission control logic runs again | ||
| to validate the request is still valid with the latest | ||
| quota document. |
There was a problem hiding this comment.
What happens if the creation fails for other reasons afterwards?
|
Is the design with the quota controller intended to be permanent? Or are we making the best of a bad situation here? |
|
WRT the one question directed at me: Generating the uid earlier seems ok. We could create an interface for that so the resthandler could know whether a resource had uids and whether a Create actually created a resource (e.g., not a subresource). |
|
cc @mml |
|
@derekwaynecarr I see this is sitting in my queue. What would you like to happen next on this? |
|
I am closing this now, we can revisit it at a later time as its not yet a major issue. I do want to send a separate PR though to allow for UID to generate earlier in the flow as we have discussed previously that can be stashed on a context. |
derekwaynecarr
changed the title
|
Reopening this as its been a source of recent discussion at Red Hat. I may look to make some additional tweaks on this in the near term. |
|
@smarterclayton @deads2k - I think the reservation system discussed here fixes the N problem we discussed today, and avoids anything additional in admission control, but I could be wrong ;-) |
| incremental create occurs in the time that it recalculated usage | ||
| it will be told it has stale data and recalculate. | ||
|
|
||
| ### ResourceQuota reservations |
There was a problem hiding this comment.
@deads2k - this should solve the latent list scenario unless I am mistaken. the replenishment controller would not release the reservation, so off by N scenarios should not happen in HA.
derekwaynecarr
force-pushed
the
quota_reservation
branch
from
15ce2c4
to
f91a75c
Compare
derekwaynecarr
changed the title
|
@smarterclayton @deads2k - Given our discussions around race windows in quota, I cleaned up this design proposal. I think it addresses the core problem around replenishment working against a latent API server (also raised here: #24494). I am not committing implementation of this for Kubernetes 1.3, but it would be nice to have an agreement on a plan of attack given our experiments with a |
derekwaynecarr
force-pushed
the
quota_reservation
branch
from
f91a75c
to
1e28e70
Compare
|
cc @davidopp |
|
cc @davidopp |
I think this closes the "off by N" hole. The actual structure may work move effectively as maps if we're trying to efficiently handle the "scale to 1000 instantly" case. 1000 is big enough to feel in an n-squared search loop ("is this one in the list, is this one in the list, etc"). |
|
cc @wojtek-t re. performance |
| and the requesting object is allowed to be created. | ||
| Quota usage is incremented using compare-and-swap | ||
| to allow for optimistic locking. If the quota usage | ||
| was stale, the admission control logic runs again |
There was a problem hiding this comment.
I assume "stale" means version number conflict? You should probably mention that explicitly.
| ## Data Model Impact | ||
|
|
||
| ``` | ||
| type ResourceQuotaReservation struct { |
There was a problem hiding this comment.
We need to document hidden state in etcd somewhere.
There was a problem hiding this comment.
@bgrant0607 - i know its been awhile, but what did you mean by hidden state? is the transient reservation concept hidden state in your view?
k8s-github-robot
added
retest-not-required-docs-only
do-not-merge
apelisse
removed
the
do-not-merge
|
GCE e2e build/test passed for commit 1e28e70. |
|
Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test". This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry. |
k8s-github-robot
added
kind/design
|
Adding label:do-not-merge because PR changes docs prohibited to auto merge |
k8s-github-robot
added
the
do-not-merge
|
This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review. You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days |
This document describes how we can get more reliable quota enforcement.
@erictune - this addresses a question you had around multi-object transactions here:
#19761 (comment)
@bgrant0607 @lavalamp @smarterclayton - this calls out the need to populate object meta system fields before calling admission controllers. most notably, a
UIDwould be really valuable in this context.FYI:
@kubernetes/rh-cluster-infra
This change is