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

x/time/rate: Limit per time period longer than second #49162

Closed
sobczak-m opened this issue Oct 26, 2021 · 4 comments
Closed

x/time/rate: Limit per time period longer than second #49162

sobczak-m opened this issue Oct 26, 2021 · 4 comments

Comments

@sobczak-m
Copy link

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

$ go version
go version go1.16 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GOHOSTARCH="amd64"
GOHOSTOS="darwin"

What did you do?

Library works good with limiting per one second but I would like to limit number of request in time period longer than second. For example allow 10 request in 60 seconds (burst = 10)
Here is example https://play.golang.org/p/OXyj7p3N9X9

What did you expect to see?

Limiter should block/reject after 11 request, because request every half second is too fast.

What did you see instead?

Limiter allow 120 requests in 60 seconds

@gopherbot gopherbot added this to the Unreleased milestone Oct 26, 2021
@D1CED
Copy link

D1CED commented Oct 26, 2021

Your calculateLimitPerSecond function contains a bug. The division is an integer division and the result is 0. You need to convert the operands and not the result.

func calculateLimitPerSecond(limit int, period int) float32 {
	return float32(limit) / float32(period)
}

@sobczak-m
Copy link
Author

@D1CED thank for correction, I made bug preparing demo. Below working code.
https://play.golang.org/p/T0_UbRSdqfc

I have still problem with allowing more request than limit (10 per 60 seconds) In this example, limiter allow overall 38 requests (13 requests in first 58 seconds). Should it limit after 11 request?

Accuracy seems to be not good in big limiters like 300 per 3600 seconds.

@D1CED
Copy link

D1CED commented Oct 26, 2021

I'm not sure why you are using a timer but assuming there are enough tokens in the limiter the request rate per minute is 60s * 1/4.5s = 13.3333 so the 13 req in 58s is correct.

And the total amount of request can be calculated by

f(x) = 10 + x * 10 / 60  # fill function
g(x) = - x * 1/4.5  # drain function

h = f + g

h(x) = 0 => x = 180

So after 180 seconds the pool is drained. With the request frequency of 1/4.5 this results in 40 total requests. I don't know why your result differs by 2 but overall the behavior of rate seems correct.

@seankhliao
Copy link
Member

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For questions please refer to https://github.com/golang/go/wiki/Questions

@golang golang locked and limited conversation to collaborators Oct 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants