Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it possible to use a Redis connection pool? #77

Closed
pieroliviermarquis opened this issue Sep 14, 2021 · 20 comments
Closed

Is it possible to use a Redis connection pool? #77

pieroliviermarquis opened this issue Sep 14, 2021 · 20 comments
Assignees

Comments

@pieroliviermarquis
Copy link

Connecting to a connection pool would be very useful. I have an issue with my application reaching the maximum number of Redis clients.

@VeNoMouS
Copy link

VeNoMouS commented Oct 4, 2021

if you running this under gunicorn etc... the short answer is no, each thread will be launched under its own application context, so it wont share a GIL ...

What I personally ended up doing , was using nutcracker and having python connect to that, to minimize the impact to the redis server

Edit:

ahhh sorry i came back to edit this, i just remember, due to limits package running "script" cmds to redis i couldnt run limiter through nutcracker

Edit 2:

I haven't tested it yet... but you could try https://github.com/RedisLabs/redis-cluster-proxy

@alisaifee
Copy link
Owner

When you say connection pool, do you mean being able to do something like this:

from limits.storage import RedisStorage
from limits.strategies import MovingWindowRateLimiter
import redis
pool = redis.ConnectionPool(host='localhost', port=6379, db=0)
storage = RedisStorage(connection_pool = pool)
limiter = MovingWindowRateLimiter(storage)

Unfortunately not at the moment - since the redis implementation delegates the construction of the redis client entirely to the redis.Redis.from_url function.

Having said it does seem reasonable to reuse a shared connection pool in your application and there have been other use-cases where letting the application provide a "Storage" that has been pre-constructed.

@alisaifee
Copy link
Owner

This issue should be in the limits repository, so I'll transfer it there.

@alisaifee alisaifee transferred this issue from alisaifee/flask-limiter Dec 15, 2021
@VeNoMouS
Copy link

VeNoMouS commented Dec 15, 2021

Well the problem comes when your using flask / gunicorn for example under green threads.. gunicorn isnt going to share the connection / connection pool because of the green thread...

So each thread is going to open up its own connection pool and when you have a cluster of servers it really hits the redis... when you doing thousands of requests a second

you can put nutcracker on the local system as a "middleware" to use just one connection pool from the server itself to the redis server , but due to the exact queries your code is running, it isnt supported in nutcracker

https://github.com/twitter/twemproxy

@alisaifee
Copy link
Owner

I think you might be able to use redis-proxy in envoy (which supports lua scripting afaik) reference

@VeNoMouS
Copy link

VeNoMouS commented Mar 8, 2022

When you say connection pool, do you mean being able to do something like this:

from limits.storage import RedisStorage
from limits.strategies import MovingWindowRateLimiter
import redis
pool = redis.ConnectionPool(host='localhost', port=6379, db=0)
storage = RedisStorage(connection_pool = pool)
limiter = MovingWindowRateLimiter(storage)

Unfortunately not at the moment - since the redis implementation delegates the construction of the redis client entirely to the redis.Redis.from_url function.

Having said it does seem reasonable to reuse a shared connection pool in your application and there have been other use-cases where letting the application provide a "Storage" that has been pre-constructed.

HI @alisaifee,

just circling back around to this, to see if this would be possible ? as if you using gunicorn etc, using a redis BlockingConnectionPool and monkey patching gevent can limit the number / rape connections being opened

would be awesome if we could just pass a redis instance to limiter rather than a URI, that way we can share an already created pool instance with the rest of the application and not create an entire new pool...

@VeNoMouS
Copy link

VeNoMouS commented Mar 9, 2022

could

redis = self.dependencies["redis"]
uri = uri.replace("redis+unix", "unix")
self.storage = redis.from_url(uri, **options)

be replaced with something along like the follwoing...

    redis = self.dependencies["redis"]
    if isinstance(uri, redis.client.Redis):
        self.storage = uri
    else:
        uri = uri.replace("redis+unix", "unix")
        self.storage = redis.from_url(uri, **options)

@alisaifee
Copy link
Owner

@VeNoMouS could you take a look at #110 to see if that'll work for you?

@alisaifee alisaifee self-assigned this Mar 9, 2022
@VeNoMouS
Copy link

VeNoMouS commented Mar 10, 2022

Will test today for you, i'll have to hack your flask limiter up first to allow the new param

mmmm i'll make some simple test code and test outside of flask first to see if it uses pool connection

@alisaifee
Copy link
Owner

You shouldn't need to hack anything in flask-limiter, this should work:

from flask_limiter import Limiter
pool = ....
limiter = Limiter(....., storage_uri="redis://", storage_options={"connection_pool": pool})

@VeNoMouS
Copy link

You shouldn't need to hack anything in flask-limiter, this should work:

from flask_limiter import Limiter
pool = ....
limiter = Limiter(....., storage_uri="redis://", storage_options={"connection_pool": pool})

ok cheers will test out in a bit and let you know

@alisaifee
Copy link
Owner

🙇

@alisaifee alisaifee reopened this Mar 11, 2022
@VeNoMouS
Copy link

VeNoMouS commented Mar 11, 2022

Since my stuff is in a docker... this is a dirty trick to pull the code base with the PR as a zip for requirements.txt incase you didnt know...

https://api.github.com/repos/alisaifee/limits/zipball/pull/110/head

@VeNoMouS
Copy link

Appears to be working @alisaifee

@VeNoMouS
Copy link

Testing it now on a test production server

@alisaifee
Copy link
Owner

I've cut a 2.4.0 release if you want to give that a shot as well!

@VeNoMouS
Copy link

Sure , give me ~hour and ill roll out another test

@VeNoMouS
Copy link

2.4.0 appears to be working

@alisaifee
Copy link
Owner

Awesome! Thanks for helping verify and also the idea!

@VeNoMouS
Copy link

thanks for patching ❤️

netbsd-srcmastr referenced this issue in NetBSD/pkgsrc Oct 21, 2022
v2.7.1
------
Release Date: 2022-10-20

* Compatibility Updates

  * Increase pymemcached dependency range to in include 4.x
  * Add python 3.11 rc2 to CI


v2.7.0
------
Release Date: 2022-07-16

* Compatibility Updates

  * Update :pypi:`coredis` requirements to include 4.x versions
  * Remove CI / support for redis < 6.0
  * Remove python 3.7 from CI
  * Add redis 7.0 in CI

v2.6.3
------
Release Date: 2022-06-05

* Chores

  * Update development dependencies
  * Add CI for python 3.11
  * Increase test coverage for redis sentinel

v2.6.2
------
Release Date: 2022-05-12

* Compatibility Updates

  * Update :pypi:`motor` requirements to include 3.x version
  * Update async redis sentinel implementation to remove use of deprecated methods.
  * Fix compatibility issue with asyncio redis ``reset`` method in cluster mode
    when used with :pypi:`coredis` versions >= 3.5.0

v2.6.1
------
Release Date: 2022-04-25

* Bug Fix

  * Fix typing regression with strategy constructors `Issue 88 <https://github.com/alisaifee/limits/issues/88>`_


v2.6.0
------
Release Date: 2022-04-25

* Deprecation

  * Removed tests for rediscluster using the :pypi:`redis-py-cluster` library

* Bug Fix

  * Fix incorrect ``__slots__`` declaration in :class:`limits.RateLimitItem`
    and it's subclasses

v2.5.4
------
Release Date: 2022-04-25

* Bug Fix

  * Fix typing regression with strategy constructors `Issue 88 <https://github.com/alisaifee/limits/issues/88>`_

v2.5.3
------
Release Date: 2022-04-22

* Chore

  * Automate Github releases

v2.5.2
------
Release Date: 2022-04-17

* Chore

  * Increase strictness of type checking and annotations
  * Ensure installations from source distributions are PEP-561
    compliant

v2.5.1
------
Release Date: 2022-04-15

* Chore

  * Ensure storage reset methods have consistent signature

v2.5.0
------
Release Date: 2022-04-13

* Feature

  * Add support for using redis cluster via the official redis client
  * Update coredis dependency to use 3.x

* Deprecations

  * Deprecate using redis-py-cluster

* Chores

  * Remove beta tags for async support
  * Update code base to remove legacy syntax
  * Tighten up CI test dependencies

v2.4.0
------
Release Date: 2022-03-10

* Feature

  * Allow passing an explicit connection pool to redis storage.
    Addresses `Issue 77 <https://github.com/alisaifee/limits/issues/77>`_

v2.3.3
------
Release Date: 2022-02-03

* Feature

  * Add support for dns seed list when using mongodb

v2.3.2
------
Release Date: 2022-01-30

* Chores

  * Improve authentication tests for redis
  * Update documentation theme
  * Pin pip version for CI

v2.3.1
------
Release Date: 2022-01-21

* Bug fix

  * Fix backward incompatible change that separated sentinel
    and connection args for redis sentinel (introduced in 2.1.0).
    Addresses `Issue 97 <https://github.com/alisaifee/limits/issues/97>`_


v2.3.0
------
Release Date: 2022-01-15

* Feature

  * Add support for custom cost per hit

* Bug fix

  * Fix installation issues with missing setuptools

v2.2.0
------
Release Date: 2022-01-05

* Feature

  * Enable async redis for python 3.10 via coredis

* Chore

  * Fix typing issue with strategy constructors

v2.1.1
------
Release Date: 2022-01-02

* Feature

  * Enable async memcache for python 3.10

* Bug fix

  * Ensure window expiry is reported in local time for mongodb
  * Fix inconsistent expiry for fixed window with memcached

* Chore

  * Improve strategy tests

v2.1.0
------
Release Date: 2021-12-22

* Feature

  * Add beta asyncio support
  * Add beta mongodb support
  * Add option to install with extras for different storages

* Bug fix

  * Fix custom option for cluster client in memcached
  * Fix separation of sentinel & connection args in :class:`limits.storage.RedisSentinelStorage`

* Deprecation

  * Deprecate GAEMemcached support
  * Remove use of unused `no_add` argument in :meth:`limits.storage.MovingWindowSupport.acquire_entry`

* Chore

  * Documentation theme upgrades
  * Code linting
  * Add compatibility CI workflow



v2.0.3
------
Release Date: 2021-11-28

* Chore

  * Ensure package is marked PEP-561 compliant

v2.0.1
------
Release Date: 2021-11-28

* Chore

  * Added type annotations

v2.0.0
------
Release Date: 2021-11-27

* Chore

  * Drop support for python < 3.7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants