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: "bursts" of the RateLimiter don't work as documented #52618

Closed
remster opened this issue Apr 29, 2022 · 2 comments
Closed

x/time: "bursts" of the RateLimiter don't work as documented #52618

remster opened this issue Apr 29, 2022 · 2 comments

Comments

@remster
Copy link

@remster remster commented Apr 29, 2022

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

$ go version
go version go1.16.6 linux/amd64

Does this issue reproduce with the latest release?

I don't know
x/time at:

golang.org/x/time v0.0.0-20220411224347-583f2d630306 h1:+gHMid33q6pen7kv9xvT+JRinntgeXO2AeZVd0AWD3w=
golang.org/x/time v0.0.0-20220411224347-583f2d630306/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/remek/.cache/go-build"
GOENV="/home/remek/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/remek/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/remek/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.6"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/remek/go/src/github.com/airmap/platform/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3207674770=/tmp/go-build -gno-record-gcc-switches"

What did you do?

wrote this test:

package util

import (
	"fmt"
	"testing"
	"time"

	"github.com/stretchr/testify/require"
	"golang.org/x/time/rate"
)

func TestRateLimiter(t *testing.T) {
	rlUnderTest := rate.NewLimiter(10, 1)
	for i := 0; i < 10; i++ {
		fmt.Printf("i:%d\n", i)
		require.True(t, rlUnderTest.Allow())
		time.Sleep(10 * time.Millisecond)
	}
}

What did you expect to see?

The test to pass.

What did you see instead?

The test failed on the 2nd iteration

=== RUN   TestRateLimiter
i:0
i:1
    /home/remek/go/src/github.com/airmap/platform/airmap/util/ratelimtter_test.go:16:
        	Error Trace:	ratelimtter_test.go:16
        	Error:      	Should be true
        	Test:       	TestRateLimiter
--- FAIL: TestRateLimiter (0.01s)
FAIL
FAIL	github.com/airmap/platform/airmap/util	0.013s

The documentation says: "Burst is the maximum number of tokens that can be consumed in a single call to Allow, Reserve, or Wait, so higher Burst values allow more events to happen at once."
As you can see; at no point I call Allow with anything different than 1.

@gopherbot gopherbot added this to the Unreleased milestone Apr 29, 2022
@seankhliao
Copy link
Member

@seankhliao seankhliao commented Apr 29, 2022

It implements a "token bucket" of size b

you've drained the limiter and it hasn't refilled

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

@remster
Copy link
Author

@remster remster commented Apr 29, 2022

Thanks. That had me connect the dots. Documentation still needs improving:

NewLimiter returns a new Limiter that allows events up to rate r and permits bursts of at most b tokens.

say here that b is also the initial credit of the bucket. b hasn't got a meaning earlier and specifically: It implements a "token bucket" of size b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants