Clarify thread-safety

[tuukkamustonen]

Is the library thread-safe? Docs don't mention, but I would assume it is, because it's being used in contexts that are normally multi-threaded (e.g. Flask-Limiter).

However, when I was using the library in multi-threaded context (with high concurrency), I randomly hit into:

  ...
  File "/home/musttu/.pyenv/versions/his/lib/python3.5/site-packages/limits/strategies.py", line 130, in hit
    self.storage().incr(item.key_for(*identifiers), item.get_expiry())
  File "/home/musttu/.pyenv/versions/his/lib/python3.5/site-packages/limits/storage.py", line 160, in incr
    self.__schedule_expiry()
  File "/home/musttu/.pyenv/versions/his/lib/python3.5/site-packages/limits/storage.py", line 148, in __schedule_expiry
    self.timer.start()
  File "/home/musttu/.pyenv/versions/3.5.5/lib/python3.5/threading.py", line 840, in start
    raise RuntimeError("threads can only be started once")
RuntimeError: threads can only be started once

Looking at https://github.com/alisaifee/limits/blob/master/limits/storage.py#L62 there seems to be a default lock object, but it doesn't seem to be ever used? In this case, https://github.com/alisaifee/limits/blob/master/limits/storage.py#L146-L148 is not locking anything so above can happen.

What is the library's take on thread-safety? I didn't go deep into code, so I may have gotten something wrong.

[alisaifee]

Some minor improvements have been made in the thread safety of the memory storage - however, it's still basically a toy implementation for local development or tests. ~~When using~~

[tuukkamustonen]

@alisaifee The second the paragraph in your response above cuts a bit short. Are you saying that memory storage should not be used in production in multi-thread environments?

[alisaifee]

@alisaifee The second the paragraph in your response above cuts a bit short. Are you saying that memory storage should not be used in production in multi-thread environments?

@tuukkamustonen I should clarify: I had never intended the memory storage to be used in production and it was meant to be:

1. a reference implementation
2. a "better" mock for tests and a lightweight implementation for development environments
3. a worst case, short window fallback in for production environments if the rate limit memcached or redis was temporarily unavailable

The main issue with memory storage is, however, that it is in memory (:smile:) and it is very unlikely there is a scenario where a single python process would be sufficient for a production setup.

Having said that, other than one reports like this providing some feedback, I have never used or benchmarked the memory storage at any relevant scale to advertise it as a production option. I'm now curious what usecases there are for it that I hadn't considered.

[tuukkamustonen]

Well... at least I am using it in production 🤔 Though, my use case is that it's an internal endpoint, which is called by other internal systems. So the number of unique clients that it sees is a few 10s at maximum.

I am running the application in multiple processes (over multiple servers), so it's not really "reliable" in that sense, but it's sufficient (if some calling backend goes haywire, it will block the excess calls from it). Having a Redis only for the rate-limit storage would be overkill for us.

[alisaifee]

Closing due to inactivity