Skip to content

Commit

Permalink
fix: memory gcra limiter inconsistency when rate < cost < burst (#177)
Browse files Browse the repository at this point in the history
Co-authored-by: Francesco Casula <fracasula@users.noreply.github.com>
  • Loading branch information
atzoum and fracasula authored Oct 27, 2023
1 parent 88ee064 commit cda0d64
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
4 changes: 3 additions & 1 deletion throttling/memory_gcra.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ func (g *gcra) getLimiter(key string, burst, rate, period int64) (*throttled.GCR
return nil, fmt.Errorf("could not create rate limiter: %w", err)
}
rl.SetMaxCASAttemptsLimit(defaultMaxCASAttemptsLimit)
g.store.Put(key, rl, time.Duration(period)*time.Second)
// rate limiter should be cached for (burst/rate)*period seconds
// e.g. if burst is 100 and rate is 10/sec, then the rate limiter should be cached for 10 seconds
g.store.Put(key, rl, time.Duration((burst/rate)*period)*time.Second)
}

return rl, nil
Expand Down
35 changes: 35 additions & 0 deletions throttling/memory_gcra_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package throttling

import (
"context"
"testing"
"time"

"github.com/stretchr/testify/require"
)

func TestMemoryGCRA(t *testing.T) {
t.Run("burst and cost greater than rate", func(t *testing.T) {
l := &gcra{}

burst := int64(5)
rate := int64(1)
period := int64(1)

limit, err := l.limit(context.Background(), "key", burst+rate, burst, rate, period)
require.NoError(t, err)
require.True(t, limit, "it should be able to fill the bucket (burst)")

// next request should be allowed after 5 seconds
start := time.Now()
var allowed bool

require.Eventually(t, func() bool {
allowed, err = l.limit(context.Background(), "key", burst, burst, rate, period)
require.NoError(t, err)
return allowed
}, 10*time.Second, 1*time.Second, "next request should be eventually allowed")

require.GreaterOrEqual(t, time.Since(start).Seconds(), 5.0, "next request should be allowed after 5 seconds")
})
}

0 comments on commit cda0d64

Please sign in to comment.