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

Add cache expiration for Redis #34

Open
GuyPaddock opened this issue Oct 5, 2016 · 5 comments
Open

Add cache expiration for Redis #34

GuyPaddock opened this issue Oct 5, 2016 · 5 comments

Comments

@GuyPaddock
Copy link

It seems like the per-second / per-minute / etc. keys that rack-throttle creates in Redis are permanent. We run a high traffic site, which means that we're going to end up with a lot of pollution and wasted space in Redis. Ideally, there should be some way for these keys to expire.

@GuyPaddock GuyPaddock changed the title Add cache expiration for memcache / Redis providers Add cache expiration for Redis Oct 5, 2016
@GuyPaddock
Copy link
Author

Just reduced the scope. Memcached isn't much of an issue, but Redis is not always configured to be a cache, so it'd be great if rack-throttle could set an appropriate expiration.

@FreekingDean
Copy link
Collaborator

Hmm, one suggestion I would have to do this is to implement your own rack-throttle cache store. The cache is meant to be very flexible you can try something like this:

class ThrottleCache
  def set(key, value)
    redis_client.set(key, value)
    redis_client.expire(key, your_expire_time)
  end

  def get(key)
    redis_client.get(key)
  end
end

rack-throttle is really cache agnostic, and doesn't really do much with the cache besides try to set keys, and retrieve them.

@FreekingDean
Copy link
Collaborator

FreekingDean commented Oct 5, 2016

That example assumes redis_client is setup to your liking in the class in some private function/variable/etc.

You can then setup rack-throttle via:

use Rack::Throttle::Interval, :cache => ThrottleCache.new
#or
config.middleware.use Rack::Throttle::Interval, :cache => ThrottleCache.new

@GuyPaddock
Copy link
Author

I'd considered that, but didn't realize it would be quite that straightforward. I was just working on a PR that would allow RT to detect if the cache supports expiration, so that each provider could provide a recommended expiration, but I think the approach you're recommending is much cleaner.

@FreekingDean
Copy link
Collaborator

Yup! I think until we decide on a "Best Practices" or start implementing true cache specific code it might be best to leave the expiry up to the user. That all being said! I think that it might be good to include this as an example :). I'm going to start working on 'wiki-ifying' the instructions one of these days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants