From a53ad5bba37199047ba20194933e122bf6b0252f Mon Sep 17 00:00:00 2001 From: Nahum Wild Date: Thu, 15 Jan 2009 21:28:10 -0600 Subject: Added in a local per request cache to MemCacheStore. It acts as a buffer to stop unneccessary requests being sent through to memcache [#1653 state:resolved] Signed-off-by: Joshua Peek --- .../lib/active_support/cache/mem_cache_store.rb | 65 ++++++++++- activesupport/test/caching_test.rb | 119 ++++++++++++++++++--- railties/lib/initializer.rb | 3 + 3 files changed, 167 insertions(+), 20 deletions(-) diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index f9a7fb1440..eed9faac6d 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -13,6 +13,7 @@ module ActiveSupport # server goes down, then MemCacheStore will ignore it until it goes back # online. # - Time-based expiry support. See #write and the +:expires_in+ option. + # - Per-request in memory cache for all communication with the MemCache server(s). class MemCacheStore < Store module Response # :nodoc: STORED = "STORED\r\n" @@ -22,6 +23,24 @@ module ActiveSupport DELETED = "DELETED\r\n" end + # this allows caching of the fact that there is nothing in the remote cache + NULL = 'mem_cache_store:null' + + THREAD_LOCAL_KEY = :mem_cache_store_cache + + class LocalCache + def initialize(app) + @app = app + end + + def call(env) + Thread.current[THREAD_LOCAL_KEY] = MemoryStore.new + @app.call(env) + ensure + Thread.current[THREAD_LOCAL_KEY] = nil + end + end + attr_reader :addresses # Creates a new MemCacheStore object, with the given memcached server @@ -42,7 +61,18 @@ module ActiveSupport def read(key, options = nil) # :nodoc: super - @data.get(key, raw?(options)) + + value = local_cache && local_cache.read(key) + if value == NULL + nil + elsif value.nil? + value = @data.get(key, raw?(options)) + local_cache.write(key, value || NULL) if local_cache + value + else + # forcing the value to be immutable + value.dup + end rescue MemCache::MemCacheError => e logger.error("MemCacheError (#{e}): #{e.message}") nil @@ -61,6 +91,7 @@ module ActiveSupport # memcache-client will break the connection if you send it an integer # in raw mode, so we convert it to a string to be sure it continues working. value = value.to_s if raw?(options) + local_cache.write(key, value || NULL) if local_cache response = @data.send(method, key, value, expires_in(options), raw?(options)) response == Response::STORED rescue MemCache::MemCacheError => e @@ -70,6 +101,7 @@ module ActiveSupport def delete(key, options = nil) # :nodoc: super + local_cache.write(key, NULL) if local_cache response = @data.delete(key, expires_in(options)) response == Response::DELETED rescue MemCache::MemCacheError => e @@ -80,14 +112,27 @@ module ActiveSupport def exist?(key, options = nil) # :nodoc: # Doesn't call super, cause exist? in memcache is in fact a read # But who cares? Reading is very fast anyway - !read(key, options).nil? + # Local cache is checked first, if it doesn't know then memcache itself is read from + value = local_cache.read(key) if local_cache + if value == NULL + false + elsif value + true + else + !read(key, options).nil? + end end def increment(key, amount = 1) # :nodoc: log("incrementing", key, amount) response = @data.incr(key, amount) - response == Response::NOT_FOUND ? nil : response + unless response == Response::NOT_FOUND + local_cache.write(key, response.to_s) if local_cache + response + else + nil + end rescue MemCache::MemCacheError nil end @@ -96,17 +141,25 @@ module ActiveSupport log("decrement", key, amount) response = @data.decr(key, amount) - response == Response::NOT_FOUND ? nil : response + unless response == Response::NOT_FOUND + local_cache.write(key, response.to_s) if local_cache + response + else + nil + end rescue MemCache::MemCacheError nil end def delete_matched(matcher, options = nil) # :nodoc: + # don't do any local caching at present, just pass + # through and let the error happen super raise "Not supported by Memcache" end def clear + local_cache.clear if local_cache @data.flush_all end @@ -115,6 +168,10 @@ module ActiveSupport end private + def local_cache + Thread.current[THREAD_LOCAL_KEY] + end + def expires_in(options) (options && options[:expires_in]) || 0 end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index d8506de986..5d220f4403 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -1,18 +1,18 @@ require 'abstract_unit' -class CacheKeyTest < Test::Unit::TestCase +class CacheKeyTest < ActiveSupport::TestCase def test_expand_cache_key assert_equal 'name/1/2/true', ActiveSupport::Cache.expand_cache_key([1, '2', true], :name) end end -class CacheStoreSettingTest < Test::Unit::TestCase +class CacheStoreSettingTest < ActiveSupport::TestCase def test_file_fragment_cache_store store = ActiveSupport::Cache.lookup_store :file_store, "/path/to/cache/directory" assert_kind_of(ActiveSupport::Cache::FileStore, store) assert_equal "/path/to/cache/directory", store.cache_path end - + def test_drb_fragment_cache_store store = ActiveSupport::Cache.lookup_store :drb_store, "druby://localhost:9192" assert_kind_of(ActiveSupport::Cache::DRbStore, store) @@ -24,13 +24,13 @@ class CacheStoreSettingTest < Test::Unit::TestCase assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) assert_equal %w(localhost), store.addresses end - + def test_mem_cache_fragment_cache_store_with_multiple_servers store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1' assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) assert_equal %w(localhost 192.168.1.1), store.addresses end - + def test_mem_cache_fragment_cache_store_with_options store = ActiveSupport::Cache.lookup_store :mem_cache_store, "localhost", '192.168.1.1', :namespace => 'foo' assert_kind_of(ActiveSupport::Cache::MemCacheStore, store) @@ -45,7 +45,7 @@ class CacheStoreSettingTest < Test::Unit::TestCase end end -class CacheStoreTest < Test::Unit::TestCase +class CacheStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:memory_store) end @@ -116,9 +116,15 @@ module CacheStoreBehavior assert_equal 1, @cache.decrement('foo') assert_equal 1, @cache.read('foo', :raw => true).to_i end + + def test_exist + @cache.write('foo', 'bar') + assert @cache.exist?('foo') + assert !@cache.exist?('bar') + end end -class FileStoreTest < Test::Unit::TestCase +class FileStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:file_store, Dir.pwd) end @@ -130,7 +136,7 @@ class FileStoreTest < Test::Unit::TestCase include CacheStoreBehavior end -class MemoryStoreTest < Test::Unit::TestCase +class MemoryStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:memory_store) end @@ -145,28 +151,109 @@ class MemoryStoreTest < Test::Unit::TestCase end uses_memcached 'memcached backed store' do - class MemCacheStoreTest < Test::Unit::TestCase + class MemCacheStoreTest < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:mem_cache_store) + @data = @cache.instance_variable_get(:@data) @cache.clear end include CacheStoreBehavior def test_store_objects_should_be_immutable - @cache.write('foo', 'bar') - @cache.read('foo').gsub!(/.*/, 'baz') - assert_equal 'bar', @cache.read('foo') + with_local_cache do + @cache.write('foo', 'bar') + @cache.read('foo').gsub!(/.*/, 'baz') + assert_equal 'bar', @cache.read('foo') + end end def test_write_should_return_true_on_success - result = @cache.write('foo', 'bar') - assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written - assert result + with_local_cache do + result = @cache.write('foo', 'bar') + assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written + assert result + end + end + + def test_local_writes_are_persistent_on_the_remote_cache + with_local_cache do + @cache.write('foo', 'bar') + end + + assert_equal 'bar', @cache.read('foo') + end + + def test_clear_also_clears_local_cache + with_local_cache do + @cache.write('foo', 'bar') + @cache.clear + assert_nil @cache.read('foo') + end end + + def test_local_cache_of_read_and_write + with_local_cache do + @cache.write('foo', 'bar') + @data.flush_all # Clear remote cache + assert_equal 'bar', @cache.read('foo') + end + end + + def test_local_cache_of_delete + with_local_cache do + @cache.write('foo', 'bar') + @cache.delete('foo') + @data.flush_all # Clear remote cache + assert_nil @cache.read('foo') + end + end + + def test_local_cache_of_exist + with_local_cache do + @cache.write('foo', 'bar') + @cache.instance_variable_set(:@data, nil) + @data.flush_all # Clear remote cache + assert @cache.exist?('foo') + end + end + + def test_local_cache_of_increment + with_local_cache do + @cache.write('foo', 1, :raw => true) + @cache.increment('foo') + @data.flush_all # Clear remote cache + assert_equal 2, @cache.read('foo', :raw => true).to_i + end + end + + def test_local_cache_of_decrement + with_local_cache do + @cache.write('foo', 1, :raw => true) + @cache.decrement('foo') + @data.flush_all # Clear remote cache + assert_equal 0, @cache.read('foo', :raw => true).to_i + end + end + + def test_exist_with_nulls_cached_locally + with_local_cache do + @cache.write('foo', 'bar') + @cache.delete('foo') + assert !@cache.exist?('foo') + end + end + + private + def with_local_cache + Thread.current[ActiveSupport::Cache::MemCacheStore::THREAD_LOCAL_KEY] = ActiveSupport::Cache::MemoryStore.new + yield + ensure + Thread.current[ActiveSupport::Cache::MemCacheStore::THREAD_LOCAL_KEY] = nil + end end - class CompressedMemCacheStore < Test::Unit::TestCase + class CompressedMemCacheStore < ActiveSupport::TestCase def setup @cache = ActiveSupport::Cache.lookup_store(:compressed_mem_cache_store) @cache.clear diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 824d1d6096..b57c46e098 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -414,6 +414,9 @@ Run `rake gems:install` to install the missing gems. def initialize_cache unless defined?(RAILS_CACHE) silence_warnings { Object.const_set "RAILS_CACHE", ActiveSupport::Cache.lookup_store(configuration.cache_store) } + if RAILS_CACHE.class.name == "ActiveSupport::Cache::MemCacheStore" + configuration.middleware.insert_after(:"ActionController::Failsafe", ActiveSupport::Cache::MemCacheStore::LocalCache) + end end end -- cgit v1.2.3