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

proposal: x/time/rate: expose number of available tokens for metrics/testing #18233

Open
liggitt opened this issue Dec 7, 2016 · 4 comments
Open

Comments

@liggitt
Copy link
Contributor

liggitt commented Dec 7, 2016

What version of Go are you using (go version)?

1.6

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

Using golang.org/x/time/rate#Limiter in kubernetes, we have a need to monitor ratelimiters for saturation. To do that, we need the capacity (available via Burst()) and the current availability. The latter is not exposed in the Limiter API.

What did you expect to see?

A thread-safe Available() method that returns (but does not reserve) the number of currently available tokens.

Something like this, perhaps:

// Available returns the number of tokens currently available, but does not
// guarantee a subsequent call to AllowN, WaitN, or ReserveN will succeed.
// If consumers are waiting for tokens, the number can be negative.
// It is primarily for metrics reporting and debugging.
func (lim *Limiter) Available() int {
	lim.mu.Lock()
	defer lim.mu.Unlock()
	_, _, tokens := lim.advance(time.Now())
	return int(tokens)
}

What did you see instead?

No visibility to the number of available tokens at a given point in time

@krousey
Copy link

krousey commented Dec 7, 2016

@liggitt see https://go-review.googlesource.com/#/c/29958/ for my attempt. The discussion on that change basically boils down to Kubernetes' queue saturation metric being a fundamentally broken calculation.

@liggitt
Copy link
Contributor Author

liggitt commented Dec 7, 2016

ah, didn't know you'd already pursued that... I just opened https://go-review.googlesource.com/#/c/34112/

@bradfitz
Copy link
Contributor

bradfitz commented Dec 7, 2016

Sorry for letting this slip through the cracks. @dsnet, can you either own this or figure out who does own x/time/rate? And then have the owner review the CLs above?

@dsnet dsnet removed their assignment Dec 6, 2017
@ALTree ALTree changed the title x/time/rate: Expose number of available tokens for metrics/testing x/time/rate: expose number of available tokens for metrics/testing Sep 22, 2018
@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 22, 2018
@liggitt liggitt closed this as completed Jun 13, 2019
@golang golang locked and limited conversation to collaborators Jun 12, 2020
@dsnet
Copy link
Member

dsnet commented Jun 29, 2023

I'd like to re-open this issue. In addition to metrics, this is also useful for computing a time duration for a request to retry after.

@dsnet dsnet reopened this Jun 29, 2023
@dsnet dsnet changed the title x/time/rate: expose number of available tokens for metrics/testing proposal: x/time/rate: expose number of available tokens for metrics/testing Jun 29, 2023
@gopherbot gopherbot added Proposal and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Incoming
Development

No branches or pull requests

6 participants