go icon indicating copy to clipboard operation
go copied to clipboard

x/time/rate: Impossible to use reservation delay

Open geovanisouza92 opened this issue 2 years ago • 3 comments

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

$ go version
go version go1.19.3 darwin/arm64

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
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/geovani/Library/Caches/go-build"
GOENV="/Users/geovani/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/geovani/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/geovani/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.19.3/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.19.3/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.19.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/63/nh001khd1792rxsfl5qbzmt40000gn/T/go-build3264002040=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Playground: https://go.dev/play/p/HLAZYj9BGhx

I was trying to use the rate.(*Limiter).Reserve() to control backpressure and rate.(*Reservation).Delay() to fill a Retry-After http header instead blocking the goroutine.

What did you expect to see?

Nothing in the playground logs.

In my code I was expecting (*Reservation).OK() return false and (*Reservation).Delay() return the amount of time we need to wait to reserve more tokens, allowing to use this value to fill a Retry-After http header, for example.

What did you see instead?

"should not reserve" logged in the playground, while in my code (*Reservation).OK() returns true. Trying to use (*Reservation).Delay() will block the goroutine forever.


Not sure the authors intention, but it seems that if we change the maxDuration (or at least expose it as a param) in this line the code will be able to meet the requirement and documented behavior.

I like the package concept and hope it get promoted to the stdlib with a simpler interface.

geovanisouza92 avatar Dec 21 '22 19:12 geovanisouza92

@rsc not sure who else to suggest look at this.

dr2chase avatar Dec 22 '22 00:12 dr2chase

CC @Sajmani

ianlancetaylor avatar Dec 22 '22 00:12 ianlancetaylor

I believe this is working as intended? https://go.dev/play/p/6ccCWLVKJzJ OK() returns true because it's possible to provide tokens at some point in the future, Delay() works as expected.

seankhliao avatar Dec 24 '22 13:12 seankhliao

@seankhliao is correct. The reservation returns OK because it's possible to act within the rate limit after waiting the delay. It only returns !OK if there's no amount of time you can wait to act within the rate limit.

Sajmani avatar Jan 04 '23 17:01 Sajmani

@Sajmani @seankhliao Thanks for investigating this 🙏

So, your suggestion it to completely ignore .OK() and check .Delay() == time.Duration(0) to know its allowed to proceed?

geovanisouza92 avatar Jan 13 '23 13:01 geovanisouza92