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: When limit higher burst, Allow() will return true higher burst #39970

Closed
yankooo opened this issue Jul 1, 2020 · 1 comment
Closed
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@yankooo
Copy link

yankooo commented Jul 1, 2020

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

$ go version
go version go1.12.17 darwin/amd64

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

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/Library/Caches/go-build"
GOEXE=""
GOFLAGS=" -mod="
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/go"
GOPROXY="direct"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/workspace/time/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/cs/47hjwvp92qqb75_p9fxxm3v40000gn/T/go-build201946329=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run the following on amd64:

package main

import (
	"fmt"
	"golang.org/x/time/rate"
	"sync"
	"sync/atomic"
	"time"
)

func main() {
	now := time.Now()
	t := time.After(time.Second)

	limit := rate.Limit(100)
	burst := 10
	l := rate.NewLimiter(limit, burst)

	var count int
	for {
		select {
		case <-t:
			fmt.Printf("%ds, %d\n", time.Now().Sub(now).Nanoseconds()/1e9, count)
			return
		default:
			if l.Allow() {
				count++
			}
		}
	}
}

What did you expect to see?

1s, 10

What did you see instead?

1s, 110

I confuse about count is not equals burst size. When I set Limit=100, burst=10, it's should Allow() for max calls is burst.

@gopherbot gopherbot added this to the Unreleased milestone Jul 1, 2020
@D1CED
Copy link

D1CED commented Jul 1, 2020

Lets go through this to clear up your confusion.

You start with a full bucket of size 10.
So 10 tokens can be retrieved effectively immediately.
During the second you wait the bucket gets refilled at 100/s.
So every 100/s of a second Allow returns true.
So all in all you should see 110±1 tokens being drawn.

Maybe what you actually want is to have a bucket size of 100 and a refill rate of 10 (and start with an empty bucket)?

As there seems to be nothing wrong here please consider closing this issue.

@dmitshur dmitshur added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 1, 2020
@yankooo yankooo closed this as completed Jul 2, 2020
@golang golang locked and limited conversation to collaborators Jul 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants