x/time/rate: convert Limit to an Interface

[deefdragon]

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

$ go version
go version go1.15.5 linux/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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOOS="linux"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build868185860=/tmp/go-build -gno-record-gcc-switches"

What did you do?

examined using rate.Limiter to interact with an API

What did you expect to see?

A way to interact with an API where the api implements token bucket rate limiting, or The ability to create an implementation of the rate limit interface to do so.

What did you see instead?

rate.Limiter is not an interface.

Limiter should likely be an interface providing the programmer the options to choose between a token bucket or a leaky bucket implementations, if not sliding and fixed windows as well.

Looking at the API, Allow, Wait and Reserve (and their N counterparts) would all be part of the interface, while get/set limit and get/set burst would likely be specific to only some implementations, and so not part of the interface.

[deefdragon]

I just threw together a quick migration to an interface to play with here. I had to make the reservation returned by Reserve() an interface as well when thinking forward about implementation of other rate limit algorithms.

I think the name of the Reservation interface should be kept to Reservation, breaking the 'er' rule, but it just works better than anything else I could find.

The only other changes to the API would be the addition of NewTokenBucket(). NewLimiter() then just calls NewTokenBucket(), which should preserve all existing functionality.

I wouldn't have jumped the gun on the proposal process, but I need a sliding window implementation soon, and if i'm writing it anyway, I might as well do so in a manner than benefits everyone.

[cespare]

The x/time/rate package only has one implementation, so why does it need an interface? If you need an abstraction across multiple types of rate limiters, that can live in your code.

To quote the Code Review Comments page:

Go interfaces generally belong in the package that uses values of the interface type, not the package that implements those values. The implementing package should return concrete (usually pointer or struct) types: that way, new methods can be added to implementations without requiring extensive refactoring.

[cespare]

(In any case, it can't be changed now in a backward-compatible way.)