Hello,
We ran into an issue where the semantics for incrementing or decrementing a value in the Rails generic cache vary based on which store you are using. I'd like to work up a patch, but I want to solicit some opinions on the best way to implement the changes.
We were attempting to implement the delete by namespace pattern that memcached recommends [1]. In short, you keep a counter in a namespace key and use the value in your target cache keys. When you want to delete the namespace, you increment the key, effectively forgetting the previous versions.
We found that implementing this pattern required custom code for each store.
The MemoryStore will pass the following test case and is what I assume is the expected behavior.
def test_increment @cache.write('foo', 1) assert_equal 1, @cache.read('foo') assert_equal 2, @cache.increment('foo') assert_equal 2, @cache.read('foo') end
The FileStore will fail the above test because increment always returns 1, not the new value. Is increment expected to return the new value?
The MemCacheStore requires the test case to be written completely differently because memcached's atomic increment and decrement methods require raw values to be written. The version of memcache-client that is bundled with activesupport will hang if number is supplied for a raw write. This is a known bug [2] and is fixed in a number of alternative memcache-clients [3]. The following test will work around the issues but is completely different from the memory store version, defeating the purpose of the facade.
def test_increment @cache.write('foo', '1', :raw => true) assert_equal '1', @cache.read('foo', :raw => true) assert_equal 2, @cache.increment('foo') assert_equal '2', @cache.read('foo', :raw => true) end
Finally, the CompressedMemCacheStore completely breaks the increment and decrement functionality. Upon trying to read an incremented value, the following exception is thrown.
Zlib::GzipFile::Error: not in gzip format ./test/../lib/active_support/gzip.rb:13:in `initialize' ./test/../lib/active_support/gzip.rb:13:in `new' ./test/../lib/active_support/gzip.rb:13:in `decompress' ./test/../lib/active_support/cache/compressed_mem_cache_store.rb: 6:in `read' test/caching_test.rb:195:in `test_increment' /Library/Ruby/Gems/1.8/gems/mocha-0.9.0/lib/mocha/ test_case_adapter.rb:69:in `__send__' /Library/Ruby/Gems/1.8/gems/mocha-0.9.0/lib/mocha/ test_case_adapter.rb:69:in `run'
What are the expected semantics for the cache store facade? I can work up a patch with test cases to ensure that the current implementations fall in line. My own suggestion would be to mimic the memory store test case, with the exception that :raw => true should be supplied (and presumably ignored by every impl other than memcache).
def test_increment @cache.write('foo', 1, :raw => true) assert_equal 1, @cache.read('foo', :raw => true) assert_equal 2, @cache.increment('foo') assert_equal 2, @cache.read('foo', :raw => true) end
Alternatively, we could create special counter methods (counter_init, counter_read, counter_write) that one would need to call to interact with counters in the cache.
Thoughts?
[1] http://www.socialtext.net/memcached/index.cgi?faq#deleting_by_namespace [2] http://rubyforge.org/tracker/index.php?func=detail&aid=13721&group_id=1513&atid=5921 [3] http://blog.fiveruns.com/2008/6/11/fiveruns-and-seattle-rb-s-memcache-client
