search

rtomayko / rack-cache

Real HTTP Caching for Ruby Web Apps
http://rtomayko.github.io/rack-cache/
Other
822 stars 124 forks source link

Support for ConnectionPool (with Dalli) #105

Open ollie opened 9 years ago

ollie commented 9 years ago

Hello,

I think it would be a good idea to support the ConnectionPool (same author as Dalli) as the storage instance. I was pointed to this https://github.com/rtomayko/rack-cache/blob/master/lib/rack/cache/storage.rb#L41-L52 so I hacked around and added another if branch and it looks like it works. I had to try this on the 1.2 version because I was not getting any messages on the master branch. Would you like a pull request? It looks like this gem is has not been updated in a while though. :-(

Thanks.

# hack in support for passing a Dalli::Client or Memcached object
# as the storage URI.
case
when defined?(::Dalli) && uri.kind_of?(::Dalli::Client) ||
     defined?(::Dalli) && defined?(::ConnectionPool) && uri.kind_of?(::ConnectionPool)
  type.const_get(:Dalli).resolve(uri)
# ...
end
s-andringa commented 8 years ago

+1; Support for connection pools could be beneficial when running in a multi-threaded webserver such as Puma for the same reasons it is recommended to use a connection pool when using Dalli for the Rails cache. Please correct me if this cannot be compared with using Dalli for Rack::Cache and this is somehow not an issue.

I don't see how the suggested solution would work though. When looking at .resolve you can see that a new Dalli::Client is created, which is passed in the connection pool as an argument (because the connection pool doesnt respond to #stats). However, Dalli::Client instances should be initialized with an array of servers, not a connection pool, so I'm really surprised this works for you. Maybe this has changed since you filed the issue.

I would opt for a more generic solution, since ConnectionPool is very generic in itself and not tied to Dalli, and could therefore potentially also be used to wrap a bunch of Memcached clients.

ollie commented 8 years ago

Hello, thanks for the reply. Sadly it has been almost two years and I don't really remember much about this anymore. I can't look into it in the near future so feel free to close this issue. :)

s-andringa commented 8 years ago

If you keep it open maybe someone else will pick it up... I'd be happy to give it a shot some time.

grosser commented 8 years ago

FYI a funky issue that would also need fixing is that we always write and read from the cache instead of only writing.

ollie commented 8 years ago

Okey, no probs, I'll reopen it. :)