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

Jump to bottom

x/time/rate: Parallel reservations in Limiter.WaitN are served in parallel, violating rate limit #43858

Closed
nbingham1 opened this issue Jan 22, 2021 · 2 comments
Closed

x/time/rate: Parallel reservations in Limiter.WaitN are served in parallel, violating rate limit #43858

nbingham1 opened this issue Jan 22, 2021 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unreleased

Comments

@nbingham1
Copy link

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

go version go1.14.13 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

$ lsb_release -a
Distributor ID:	Ubuntu
Description:	Ubuntu 20.10
Release:	20.10
Codename:	groovy
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/nbingham/go/bin"
GOCACHE="/home/nbingham/.cache/go-build"
GOENV="/home/nbingham/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOOS="linux"
GOPATH="/home/nbingham/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build682703074=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

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

func do_things(ctx context.Context, limiter *rate.Limiter, wg *sync.WaitGroup) {
  for i := 0; i < 10; i++ {
    limiter.WaitN(ctx, 100.0)
  }
  wg.Done()
}

func main() {
  ctx := context.Background()
  burst := 1000000000
  limiter := rate.NewLimiter(rate.Limit(1000.0), burst)
  limiter.AllowN(time.Now(), burst)

  var wg sync.WaitGroup

  startTime := time.Now()
  for i := 0; i < 10; i++ {
    wg.Add(1)
    go do_things(ctx, limiter, &wg)
  }

  wg.Wait()
  endTime := time.Now()

  elapsed := endTime.Sub(startTime).Seconds()
  fmt.Printf("elapsed: %f, rate: %f != %f\n", elapsed, 1000000/elapsed, 1000.0)
}

What did you expect to see?

The rate achieved by the 10 goroutines should not exceed the rate limit.

What did you see instead?

The overall rate is 10x what it should be.

elapsed: 10.000078, rate: 99999.223486 != 1000.000000

Suspected Cause

https://github.com/golang/time/blob/7e3f01d253248a0a5694eb5b7376dfea18b6397e/rate/rate.go#L345

r.timeToAct = now.Add(waitDuration)

should be

r.timeToAct = lim.lastEvent.Add(waitDuration)
@gopherbot gopherbot added this to the Unreleased milestone Jan 22, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 22, 2021
@seankhliao seankhliao changed the title x/time: Parallel reservations in rate/Limiter.WaitN are served in parallel, violating rate limit x/time/rate: Parallel reservations in Limiter.WaitN are served in parallel, violating rate limit Jan 22, 2021
@seankhliao
Copy link
Member

I don't see a problem here other than an error in your output: 1000000/elapsed?
10 goroutines x 10 iterations x 100 = 10000
10000 / 10 seconds for program execution = 1000 rps == the configured rate limiter

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 22, 2021
@nbingham1
Copy link
Author

I'm silly, it was a pointer issue in my other code.

@golang golang locked and limited conversation to collaborators Jan 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@gopherbot @nbingham1 @seankhliao