This project is archived and is in readonly mode.
Increment/decrement semantics vary between Rails.cache stores
Reported by Doug Barth | October 7th, 2008 @ 05:28 PM
This issue was discussed in more detail on the Rails Core mailing list. http://groups.google.com/group/r...
Essentially, each cache store has different semantics for incrementing & decrementing counters in the cache. The desired semantics can be expressed with the following test cases.
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')
assert_equal 3, @cache.increment('foo')
assert_equal 3, @cache.read('foo')
end
def test_decrement
@cache.write('foo', 3)
assert_equal 3, @cache.read('foo')
assert_equal 2, @cache.decrement('foo')
assert_equal 2, @cache.read('foo')
assert_equal 1, @cache.decrement('foo')
assert_equal 1, @cache.read('foo')
end
Unfortunately, due to MemCache's requirement that counter data be stored in raw format, that syntax needs to be modified to following if the MemCache store is to be supported. This style, while ugly, will still work across all stores.
def test_increment
@cache.write('foo', 1, :raw => true)
assert_equal 1, @cache.read('foo', :raw => true).to_i
assert_equal 2, @cache.increment('foo')
assert_equal 2, @cache.read('foo', :raw => true).to_i
assert_equal 3, @cache.increment('foo')
assert_equal 3, @cache.read('foo', :raw => true).to_i
end
def test_decrement
@cache.write('foo', 3, :raw => true)
assert_equal 3, @cache.read('foo', :raw => true).to_i
assert_equal 2, @cache.decrement('foo')
assert_equal 2, @cache.read('foo', :raw => true).to_i
assert_equal 1, @cache.decrement('foo')
assert_equal 1, @cache.read('foo', :raw => true).to_i
end
Comments and changes to this ticket
-
Michael Koziarski October 9th, 2008 @ 11:01 AM
- Assigned user set to “Michael Koziarski”
Could you give us a version of this patch which doesn't touch the memcache stuff, but just tidies up the semantics of the other stores?
Then we'll need to get the memcache stuff fixed upstream, it's also broken in the MemCache gem from evan weaver.
-
Doug Barth October 9th, 2008 @ 03:43 PM
Michael, I will work on that change, but I'm a bit confused as to why the current patch can't be applied as is. I didn't make any modifications to the vendor-ized memcache-client library included with ActiveSupport. All changes were in the ActiveSupport::Cache::Store subclasses, which, as far as I know, are only maintained in the ActiveSupport codebase.
-
Michael Koziarski October 9th, 2008 @ 03:47 PM
I'm pretty confident that we can and will apply it, I just want to have that patch seperately so we can use it when talking with the various memcache driver maintainers. e.g. we can say "This is the problem, does this fix look right" without having a bunch of other changes in there to confuse them
-
Doug Barth October 9th, 2008 @ 04:30 PM
Thanks for the clarification Michael. In that case, I will split this patch into two. One to fix all the cache stores minus MemCacheStore & CompressedMemCache and the second will fix the remaining stores.
On a related note, do you know what process needs to be spun up to successfully use the DRbStore?
On Oct 9, 2008, at 9:47 AM, Lighthouse wrote:
-
Doug Barth October 9th, 2008 @ 05:11 PM
- no changes were found...
-
Doug Barth October 9th, 2008 @ 05:12 PM
- no changes were found...
-
Michael Koziarski October 10th, 2008 @ 04:09 PM
I have no idea, I believe it's just a drb script which exposes a hash...
No one uses it and it should probably be removed post 2.2.
-
Michael Koziarski October 10th, 2008 @ 04:52 PM
The raw fix doesn't seem to help matters, as you can't read the value again once it's been created due to marshalling errors
Seems that (ironically) we can't support incr/decr for memcache at all?
-
Doug Barth October 10th, 2008 @ 05:00 PM
Michael, do you have a test case that exposes the error you're seeing? Everything worked fine for me so long as you also read the value with :raw => true.
What are your thoughts on creating separate methods for reading/ writing counter values? That would allow each store to hide it's own peculiarities for counters behind those methods.
-
Michael Koziarski October 12th, 2008 @ 05:37 PM
Yeah, sorry, I messed up the testing. Never test from the railway station :)
However could you clarify one thing. This line is to work around the memcache-client bug right:
+ value = value.to_s if raw?(options) -
Michael Koziarski October 12th, 2008 @ 05:39 PM
- Milestone cleared.
-
-
Michael Koziarski October 14th, 2008 @ 08:59 PM
OK, I've got this applied locally, the only catch is it shouldn't test the memcache store tests when there's no memcache connection available, but currently it does. So if there's no memcached process running, I get errors.
Can you address this in another patch, then I think we're good to go.
Thanks for the good work!
-
-
Pratik October 17th, 2008 @ 05:24 PM
- State changed from “new” to “resolved”
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Doug Barth
Michael Koziarski
Pratik