alisaifee / limits

Rate limiting using various strategies and storage backends such as redis & memcached
https://limits.readthedocs.org
MIT License
423 stars 59 forks source link

Fixed window strategy using mongodb storage not working #182

Closed bertytobing closed 9 months ago

bertytobing commented 1 year ago

Fixed window strategy using mongodb storage not working. The expireAt always use moving window strategy.

def incr(
        self, key: str, expiry: int, elastic_expiry: bool = False, amount: int = 1
    ) -> int:
        """
        increments the counter for a given rate limit key

        :param key: the key to increment
        :param expiry: amount in seconds for the key to expire in
        :param amount: the number to increment by
        """
        expiration = datetime.datetime.utcnow() + datetime.timedelta(seconds=expiry)

        return int(
            self.counters.find_one_and_update(
                {"_id": key},
                [
                    {
                        "$set": {
                            "count": {
                                "$cond": {
                                    "if": {"$lt": ["$expireAt", "$$NOW"]},
                                    "then": amount,
                                    "else": {"$add": ["$count", amount]},
                                }
                            },
                            "expireAt": {
                                "$cond": {
                                    "if": {"$lt": ["$expireAt", "$$NOW"]},
                                    "then": expiration,
                                    "else": (
                                        expiration if elastic_expiry else "$expireAt"
                                    ),
                                }
                            },
                        }
                    },
                ],
                upsert=True,
                projection=["count"],
                return_document=self.lib.ReturnDocument.AFTER,
            )["count"]
        )

Expected Behaviour

Use strategy configuration.

Current Behaviour

https://github.com/alisaifee/limits/blob/master/limits/storage/mongodb.py#L104

Always use moving window strategy.

Steps to Reproduce

limiter = Limiter(
    key_func=lambda: get_limiter_key(),
    app=app,
    storage_uri=uri,
    enabled=True,
    strategy="fixed-window",
)

Already setting strategy but not work.

alisaifee commented 1 year ago

I don’t think I understand the issue clearly, could you provide a reproduction step?

bertytobing commented 1 year ago

I don’t think I understand the issue clearly, could you provide a reproduction step?

  1. Use mongodb for storage.
  2. Adopt the fixed-window strategy.
  3. Set the rate limit to 2/1 minute.
  4. Hit the endpoint at 00:00:55 and again at 00:00:59.
  5. Hit the endpoint once more at 00:01:01. You'll receive a 429 error (indicating too many requests).

If I'm understanding correctly, the fixed-window strategy should reset the limit based on the specified period, in this case, every minute (at zero seconds). However, it currently seems to reset the limit based on the second of the first request (moving-window strategy).

alisaifee commented 11 months ago

Yes this is the expected behavior of the fixed window strategy is implemented (regardless of the storage type) - i.e. the window is constructed based on the second of the first hit.

bertytobing commented 11 months ago

Yes this is the expected behavior of the fixed window strategy is implemented (regardless of the storage type) - i.e. the window is constructed based on the second of the first hit.

So If I want to make it reset every month let's say, how to achieve that?

alisaifee commented 11 months ago

So If I want to make it reset every month let's say, how to achieve that?

You could call the reset method on the storage if it supports it using a scheduled job

alisaifee commented 9 months ago

Closing due to inactivity