ratelimiting starts too early

[komuw]

https://github.com/komuw/ong/blob/720b28e32cb586f482631294b69db67d2d690a13/.github/workflows/integration_test_coverage.sh#L87-L101 Has 3 failures which were rate limited yet request rate is 20/s hence should not have any failures.

[komuw]

goimports -w .;gofumpt -extra -w .;gofmt -w -s .;go mod tidy;go run -race ./example/...

echo "GET https://localhost:65081/check/67" | vegeta attack -duration=10s -rate=90/s -workers=1 -max-workers=1 | tee results.text | vegeta report

Status Codes  [code:count]               200:892  429:8
Error Set:
429 Too Many Requests

892 succeeded whereas 8 were rate limited.

[komuw]

I think it is okay. Ratelimiting is not supposed to be exact, it should be approximate; https://github.com/komuw/ong/blob/aaae923ae7324056e56bad4d908b507b8bf30c5d/middleware/ratelimiter_test.go#L157

[komuw]

https://github.com/uber-go/ratelimit

[komuw]

type mutexLimiter struct {
    mu sync.Mutex // protects last and sleepFor
    // +checklocks:mu
    last time.Time
    // +checklocks:mu
    sleepFor time.Duration

    perRequest time.Duration
    maxSlack   time.Duration
}

// newMutexBased returns a new atomic based limiter.
func newMutexBased(rate int) *mutexLimiter {
    var (
        slack int           = 10
        per   time.Duration = 1 * time.Second
    )

    perRequest := per / time.Duration(rate)
    fmt.Println("\t perRequest: ", perRequest)

    l := &mutexLimiter{
        perRequest: perRequest,
        maxSlack:   -1 * time.Duration(slack) * perRequest,
    }
    return l
}

// Take blocks to ensure that the time spent between multiple
// Take calls is on average per/rate.
func (t *mutexLimiter) Take() bool {
    t.mu.Lock()
    defer t.mu.Unlock()

    now := time.Now()

    alw := true

    if t.last.IsZero() {
        // this is first request, allow it.
        t.last = now
        return alw
    }

    // sleepFor calculates how much time we should sleep based on
    // the perRequest budget and how long the last request took.
    // Since the request may take longer than the budget, this number
    // can get negative, and is summed across requests.
    t.sleepFor += t.perRequest - now.Sub(t.last)

    // We shouldn't allow sleepFor to get too negative, since it would mean that
    // a service that slowed down a lot for a short period of time would get
    // a much higher RPS following that.
    if t.sleepFor < t.maxSlack {
        t.sleepFor = t.maxSlack
    }

    // If sleepFor is positive, then we should sleep now.
    // fmt.Println("\t t.sleepFor: ", t.sleepFor)
    if t.sleepFor > 0 {
        time.Sleep(t.sleepFor)
        t.last = now.Add(t.sleepFor)
        t.sleepFor = 0
        alw = false
    } else {
        t.last = now
    }

    return alw
}