ulule / limiter

Dead simple rate limit middleware for Go.
MIT License
2.06k stars 147 forks source link

Limiters with momery store are not working as expected #230

Closed mohamedabozeid closed 1 year ago

mohamedabozeid commented 1 year ago

I was using v2 and upon update to v3, I found the weird behaviour

Steps:

-- limit is 30 After 1 increments for Key: 1HourKey, Remaining: 29 -- correct After 2 increments for Key: 1HourKey, Remaining: 28 -- correct After 3 increments for Key: 1HourKey, Remaining: 27 -- correct After 4 increments for Key: 1HourKey, Remaining: 26 -- correct After 5 increments for Key: 1HourKey, Remaining: 29 -- wrong , should be 25 After 6 increments for Key: 1HourKey, Remaining: 28 -- wrong , should be 24 After 7 increments for Key: 1HourKey, Remaining: 27 -- wrong , should be 23 After 8 increments for Key: 1HourKey, Remaining: 26 -- wrong , should be 22 After 9 increments for Key: 1HourKey, Remaining: 29 -- wrong , should be 21 After 10 increments for Key: 1HourKey, Remaining: 28 -- wrong , should be 20

package main

import (
    "context"
    "fmt"
    "time"

    "github.com/ulule/limiter/v3"
    "github.com/ulule/limiter/v3/drivers/store/memory"
)

const (
    oneSecKey  = "1SecondKey"
    oneHourKey = "1HourKey"
)

func main() {
    oneSecRate := limiter.Rate{
        Period: 1 * time.Second,
        Limit:  10,
    }

    oneHourRate := limiter.Rate{
        Period: 1 * time.Hour,
        Limit:  30,
    }

    store := memory.NewStore()

    oneSecLimiter := limiter.New(store, oneSecRate)
    oneHourLimiter := limiter.New(store, oneHourRate)

    for i := 1; i <= 10; i++ {
        ctx := context.Background()
        increment(ctx, oneSecLimiter, oneHourLimiter)
        //printStatus(ctx, oneSecLimiter, oneSecKey)
        printStatus(ctx, oneHourLimiter, oneHourKey, i)
    }
}

func printStatus(ctx context.Context, l *limiter.Limiter, key string, incrementCount int) {
    lmCtx, err := l.Peek(ctx, key)
    if err != nil {
        panic(err)
    }

    fmt.Println(fmt.Sprintf("After %d increments for Key: %v, Remaining: %v", incrementCount, key, lmCtx.Remaining))
}

func increment(ctx context.Context, oneSecLimiter, oneHourLimiter *limiter.Limiter) {
    _, err := oneSecLimiter.Get(ctx, oneSecKey)
    if err != nil {
        panic(err)
    }

    _, err = oneHourLimiter.Get(ctx, oneHourKey)
    if err != nil {
        panic(err)
    }
}
novln commented 1 year ago

Hello :wave:

It should be fixed with the latest release: https://github.com/ulule/limiter/releases/tag/v3.11.2

Thank you for bringing this up.

Have a nice day :slightly_smiling_face:

novln commented 1 year ago

@mohamedabozeid Could you confirm if the new version fixed the issue that you noticed on your workload ?

mohamedabozeid commented 1 year ago

Hey @novln , Yes it is fixed with the new version, thank you, much apperciated