diff options
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | activesupport/CHANGELOG.md | 2 | ||||
| -rw-r--r-- | activesupport/lib/active_support/cache.rb | 17 | ||||
| -rw-r--r-- | activesupport/lib/active_support/cache/mem_cache_store.rb | 13 | ||||
| -rw-r--r-- | activesupport/lib/active_support/cache/redis_cache_store.rb | 40 | ||||
| -rw-r--r-- | activesupport/test/cache/behaviors/connection_pool_behavior.rb | 7 | ||||
| -rw-r--r-- | activesupport/test/cache/stores/redis_cache_store_test.rb | 45 |
7 files changed, 101 insertions, 25 deletions
@@ -52,7 +52,7 @@ end gem "dalli", ">= 2.2.1" gem "listen", ">= 3.0.5", "< 3.2", require: false gem "libxml-ruby", platforms: :ruby -gem "connection_pool" +gem "connection_pool", require: false # for railties app_generator_test gem "bootsnap", ">= 1.1.0", require: false diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index c9cf63f7b5..d2e7b0a31a 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,5 @@ +* Add support for connection pooling on RedisCacheStore. + *fatkodima* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/activesupport/CHANGELOG.md) for previous changes. diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 2d038dba77..1ea2d0bbf2 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -160,6 +160,23 @@ module ActiveSupport attr_reader :silence, :options alias :silence? :silence + class << self + private + def retrieve_pool_options(options) + {}.tap do |pool_options| + pool_options[:size] = options.delete(:pool_size) if options[:pool_size] + pool_options[:timeout] = options.delete(:pool_timeout) if options[:pool_timeout] + end + end + + def ensure_connection_pool_added! + require "connection_pool" + rescue LoadError => e + $stderr.puts "You don't have connection_pool installed in your application. Please add it to your Gemfile and run bundle install" + raise e + end + end + # Creates a new cache. The options will be passed to any write method calls # except for <tt>:namespace</tt> which can be used to set the global # namespace for the cache. diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index cae0d44e7d..2840781dde 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -63,21 +63,12 @@ module ActiveSupport addresses = addresses.flatten options = addresses.extract_options! addresses = ["localhost:11211"] if addresses.empty? - - pool_options = {} - pool_options[:size] = options[:pool_size] if options[:pool_size] - pool_options[:timeout] = options[:pool_timeout] if options[:pool_timeout] + pool_options = retrieve_pool_options(options) if pool_options.empty? Dalli::Client.new(addresses, options) else - begin - require "connection_pool" - rescue LoadError => e - $stderr.puts "You don't have connection_pool installed in your application. Please add it to your Gemfile and run bundle install" - raise e - end - + ensure_connection_pool_added! ConnectionPool.new(pool_options) { Dalli::Client.new(addresses, options.merge(threadsafe: false)) } end end diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb index 1de98dcd6c..b8227359a5 100644 --- a/activesupport/lib/active_support/cache/redis_cache_store.rb +++ b/activesupport/lib/active_support/cache/redis_cache_store.rb @@ -20,6 +20,15 @@ require "active_support/core_ext/marshal" module ActiveSupport module Cache + module ConnectionPoolLike + def with + yield self + end + end + + ::Redis.include(ConnectionPoolLike) + ::Redis::Distributed.include(ConnectionPoolLike) + # Redis cache store. # # Deployment note: Take care to use a *dedicated Redis cache* rather @@ -172,7 +181,16 @@ module ActiveSupport end def redis - @redis ||= self.class.build_redis(**redis_options) + @redis ||= begin + pool_options = self.class.send(:retrieve_pool_options, redis_options) + + if pool_options.any? + self.class.send(:ensure_connection_pool_added!) + ::ConnectionPool.new(pool_options) { self.class.build_redis(**redis_options) } + else + self.class.build_redis(**redis_options) + end + end end def inspect @@ -211,7 +229,7 @@ module ActiveSupport instrument :delete_matched, matcher do case matcher when String - redis.eval DELETE_GLOB_LUA, [], [namespace_key(matcher, options)] + redis.with { |c| c.eval DELETE_GLOB_LUA, [], [namespace_key(matcher, options)] } else raise ArgumentError, "Only Redis glob strings are supported: #{matcher.inspect}" end @@ -229,7 +247,7 @@ module ActiveSupport def increment(name, amount = 1, options = nil) instrument :increment, name, amount: amount do failsafe :increment do - redis.incrby normalize_key(name, options), amount + redis.with { |c| c.incrby normalize_key(name, options), amount } end end end @@ -245,7 +263,7 @@ module ActiveSupport def decrement(name, amount = 1, options = nil) instrument :decrement, name, amount: amount do failsafe :decrement do - redis.decrby normalize_key(name, options), amount + redis.with { |c| c.decrby normalize_key(name, options), amount } end end end @@ -267,7 +285,7 @@ module ActiveSupport if namespace = merged_options(options)[namespace] delete_matched "*", namespace: namespace else - redis.flushdb + redis.with { |c| c.flushdb } end end end @@ -298,7 +316,7 @@ module ActiveSupport # Read an entry from the cache. def read_entry(key, options = nil) failsafe :read_entry do - deserialize_entry redis.get(key) + deserialize_entry redis.with { |c| c.get(key) } end end @@ -309,7 +327,7 @@ module ActiveSupport keys = names.map { |name| normalize_key(name, options) } values = failsafe(:read_multi_mget, returning: {}) do - redis.mget(*keys) + redis.with { |c| c.mget(*keys) } end names.zip(values).each_with_object({}) do |(name, value), results| @@ -341,9 +359,9 @@ module ActiveSupport modifiers[:nx] = unless_exist modifiers[:px] = (1000 * expires_in.to_f).ceil if expires_in - redis.set key, value, modifiers + redis.with { |c| c.set key, value, modifiers } else - redis.set key, value + redis.with { |c| c.set key, value } end end end @@ -351,7 +369,7 @@ module ActiveSupport # Delete an entry from the cache. def delete_entry(key, options) failsafe :delete_entry, returning: false do - redis.del key + redis.with { |c| c.del key } end end @@ -360,7 +378,7 @@ module ActiveSupport if entries.any? if mset_capable? && expires_in.nil? failsafe :write_multi_entries do - redis.mapped_mset(entries) + redis.with { |c| c.mapped_mset(entries) } end else super diff --git a/activesupport/test/cache/behaviors/connection_pool_behavior.rb b/activesupport/test/cache/behaviors/connection_pool_behavior.rb index 0d46f88552..a0dcc8199c 100644 --- a/activesupport/test/cache/behaviors/connection_pool_behavior.rb +++ b/activesupport/test/cache/behaviors/connection_pool_behavior.rb @@ -6,7 +6,7 @@ module ConnectionPoolBehavior emulating_latency do begin - cache = ActiveSupport::Cache.lookup_store(store, pool_size: 2, pool_timeout: 1) + cache = ActiveSupport::Cache.lookup_store(store, { pool_size: 2, pool_timeout: 1 }.merge(store_options)) cache.clear threads = [] @@ -33,7 +33,7 @@ module ConnectionPoolBehavior def test_no_connection_pool emulating_latency do begin - cache = ActiveSupport::Cache.lookup_store(store) + cache = ActiveSupport::Cache.lookup_store(store, store_options) cache.clear threads = [] @@ -54,4 +54,7 @@ module ConnectionPoolBehavior end end end + + private + def store_options; {}; end end diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index 62752d2c65..a2bb4253f0 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -5,6 +5,24 @@ require "active_support/cache" require "active_support/cache/redis_cache_store" require_relative "../behaviors" +driver_name = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis" +driver = Object.const_get("Redis::Connection::#{driver_name.camelize}") + +Redis::Connection.drivers.clear +Redis::Connection.drivers.append(driver) + +# Emulates a latency on Redis's back-end for the key latency to facilitate +# connection pool testing. +class SlowRedis < Redis + def get(key, options = {}) + if key =~ /latency/ + sleep 3 + else + super + end + end +end + module ActiveSupport::Cache::RedisCacheStoreTests DRIVER = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis" @@ -110,6 +128,33 @@ module ActiveSupport::Cache::RedisCacheStoreTests include AutoloadingCacheBehavior end + class ConnectionPoolBehaviourTest < StoreTest + include ConnectionPoolBehavior + + private + + def store + :redis_cache_store + end + + def emulating_latency + old_redis = Object.send(:remove_const, :Redis) + Object.const_set(:Redis, SlowRedis) + + yield + ensure + Object.send(:remove_const, :Redis) + Object.const_set(:Redis, old_redis) + end + end + + class RedisDistributedConnectionPoolBehaviourTest < ConnectionPoolBehaviourTest + private + def store_options + { url: %w[ redis://localhost:6379/0 redis://localhost:6379/0 ] } + end + end + # Separate test class so we can omit the namespace which causes expected, # appropriate complaints about incompatible string encodings. class KeyEncodingSafetyTest < StoreTest |