mennanov / limiters

Golang rate limiters for distributed applications
https://godoc.org/github.com/mennanov/limiters
MIT License
411 stars 47 forks source link

Redis backend with race check bug #29

Closed oskarwojciski closed 9 months ago

oskarwojciski commented 9 months ago

After the fix from #22 I found a bug: With a small amount of Takes from the bucket, where Backend is a Redis with raceCheck=true then sometimes the expected version is higher than the actual one.

To replicate the issue I created a simple test:

func (s *LimitersTestSuite) TestTokenBucketRealClockRaceCheckRedisProblem() {
    ttl := time.Second
    limit := int64(2)
    refillEvery := time.Duration(int64(ttl) / limit)
    locker := l.NewLockNoop()
    backend := l.NewTokenBucketRedis(s.redisClient, uuid.New().String(), ttl, true)

    clk := l.NewSystemClock()
    ctx := context.Background()

    bucket := l.NewTokenBucket(
        limit,
        refillEvery,
        locker,
        backend,
        clk,
        nil,
    )

    for i := 0; i < 4; i++ {
        wait, err := bucket.Limit(ctx)
        if err != nil {
            assert.ErrorIs(s.T(), err, l.ErrLimitExhausted)
        }
        if wait.Seconds() > 0 {
            clk.Sleep(time.Second) // after a second all keys expired and the initial state was returned, but lastVersion is not cleared
        }
    }
}

checkResponseFromRedis returns error:

got [1 1 OK OK] from redis, expected [3 1 OK OK]
oskarwojciski commented 9 months ago

I have created a PR with proposed solution for that: #30

oskarwojciski commented 9 months ago

Resolved with #30