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 <josh@joshpeek.com>

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

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')
+      with_local_cache do
-      @cache.read('foo').gsub!(/.*/, 'baz')
+        @cache.write('foo', 'bar')
-      assert_equal 'bar', @cache.read('foo')
+        @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')
+      with_local_cache do
-      assert_equal 'bar', @cache.read('foo') # make sure 'foo' was written
+        result = @cache.write('foo', 'bar')
-      assert result
+        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

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