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

Improve quota replenishment #24527

Closed
deads2k opened this issue Apr 20, 2016 · 9 comments
Closed

Improve quota replenishment #24527

deads2k opened this issue Apr 20, 2016 · 9 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@deads2k
Copy link
Contributor

deads2k commented Apr 20, 2016

Quota replenishment can be slow and irksome in the case of rejected requests. One common example: create with a conflicting name. Quota charges you, but doesn't "uncharge" until it refreshes again.

If we don't need quota to be perfectly, this would be heuristically resolved by registering a time-based recheck on create against a cache of the resource to see if it exists in the store/index based on the reflector. If you check for that particular resource existing in 500 millliseconds, you'll usually make the correct choice since you're basically talking about etcd latency at that point. It possible that you'll be wrong, but a user can't force the system in their favor to gain advantage and the reconciliation of the controller will eventually get it right anyway.

In openshift, this would usually be zero cost, since we run core controllers in-process. In kube, this is non-zero cost (you'd have another cache). I'd like to see the quota admission plugin updated to at least have this as an option or some other solution that can uncharge on failed creates. I see this or a RESTHandler post-hook that can never fail (someone will abuse this) as the best options.

@derekwaynecarr @kubernetes/rh-cluster-infra

@derekwaynecarr derekwaynecarr self-assigned this Apr 20, 2016
@derekwaynecarr
Copy link
Member

I am aware of the conflicting name problem we hit with deployments, we also just have this problem generally when you attempt to create invalid objects that fail validation. Per our chat, I am in favor of a RESTHandler post-hook to support refunding quota in this case of a POST that failed validation.

The post-hook cannot depend on look-up of a resource by name since name generation happens after the fact, so in general, the hook would need to probably just know did the request fail or not, and know what to refund based on shared state, so we may want to expose Context on admission attributes. I will hack something up.

Also related: #20113

@derekwaynecarr derekwaynecarr added this to the next-candidate milestone Apr 20, 2016
@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label May 31, 2017
@0xmichalis
Copy link
Contributor

/sig node
/sig api-machinery

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Jun 20, 2017
@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jun 20, 2017
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 29, 2017
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 28, 2018
@nikhita
Copy link
Member

nikhita commented Jan 28, 2018

/remove-lifecycle rotten
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Jan 28, 2018
@utkarsh2102
Copy link

Hey,

This hasn't been looked up in ~5 years and has a related PR, which has been closed, too. So I'd be inclined to closing this one. Should you feel differently, please let us know?

/kind feature
/close

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 24, 2021
@k8s-ci-robot
Copy link
Contributor

@utkarsh2102: You can't close an active issue/PR unless you authored it or you are a collaborator.

In response to this:

Hey,

This hasn't been looked up in ~5 years and has a related PR, which has been closed, too. So I'd be inclined to closing this one. Should you feel differently, please let us know?

/kind feature
/close

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.

@ehashman
Copy link
Member

/remove-lifecycle frozen
/close

@k8s-ci-robot
Copy link
Contributor

@ehashman: Closing this issue.

In response to this:

/remove-lifecycle frozen
/close

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.

@k8s-ci-robot k8s-ci-robot removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

No branches or pull requests