arthurnn / memcached

A Ruby interface to the libmemcached C client
Academic Free License v3.0
432 stars 125 forks source link

Increment on invalid value #110

Closed minad closed 7 years ago

minad commented 11 years ago

Hi Evan,

from the last issue #109 I thought that you already looked at my code. Is it correct that memcached returns 0 if increment is applied on an invalid value?

I want to throw an exception in such a case, see:

https://github.com/minad/moneta/blob/master/lib/moneta/adapters/memcached/native.rb#L56

Thx!

Daniel

evan commented 11 years ago

I believe it destructively changes the value to the ASCII string "0", thus letting you increment it correctly in the future.

I'm not sure if throwing an error is correct as one could be incrementing and decrementing and arrive at 0 legitimately.

minad commented 11 years ago

Hi evan, I think you misunderstood the issue a bit. Memcached doesn't destructivily change the ascii string to 0. But it always returns 0 in such a case. I would say this is incorrect behaviour, memcached should throw an error since currently you can not distinguish valid 0 and invalid 0.

Dalli and redis for example throw an error.

minad commented 11 years ago

Another possibility would be to return nil or the ascii string, at least it shouldn't return 0. An exception would be the best imho. Reopen?

minad commented 11 years ago

Ok, I investigated this a bit and it seems to be a problem of the c memcached library. The library returns 0 in the case of an invalid increment.

server.set('counter', 'invalid')
server.get('counter') # => invalid
server.increment('counter')  # => 0
server.get('counter') # => invalid
evan commented 11 years ago

Oh, ok. Well, I will reopen. Can you submit a patch for the bundled C library?

minad commented 11 years ago

Hmm, better for the upstream library. But I am not in the source there. Will do if I have time, ok?

evan commented 11 years ago

We're already far diverged from the original upstream package, so you'll have to just do it here.

Sounds good.

minad commented 11 years ago

ok, why is that? That is bad.

evan commented 11 years ago

Upstream performance degraded unacceptably. There are a few tickets discussing the problem.