arthurnn / memcached

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

ActiveSupport Cache::MemCacheStore#write always returns false when using Memcached::Rails #18

Closed drobbecke closed 14 years ago

drobbecke commented 14 years ago

ActiveSupport Cache::MemCacheStore#write always returns false when using Memcached::Rails

The MemCacheStore#write method calls either the Memcached::Rails #set or #add method depending on whether or not the "unless_exist" option is specified. The #set method returns nil and #add returns true or false based upon if the value was written or not.

The MemCacheStore #write method appears to expect a value from a predefined set, checking explicitly for Response::Stored, returned from these method calls.

code snippet from MemCacheStore#write: method = options && options[:unless_exist] ? :add : :set value = value.to_s if raw?(options) response = @data.send(method, key, value, expires_in(options), raw?(options)) response == Response::STORED

Currently - since nil, true, or false is always returned this evaluation is always resulting in false.

Defined Response values in ActiveSupport Cache::MemCacheStore:

module Response # :nodoc:
  STORED      = "STORED\r\n"
  NOT_STORED  = "NOT_STORED\r\n"
  EXISTS      = "EXISTS\r\n"
  NOT_FOUND   = "NOT_FOUND\r\n"
  DELETED     = "DELETED\r\n"
end

We have an application that utilizes memcached and since it has multiple threads across multiple Ruby processes that may be attempting to update the objects in the cache, it is also relying on memcache as a locking mechanism. This mechanism depends on knowing if the write of a lock key succeeded or not. The lock is set as follows:

Rails.cache.write( self.lock_key( key ), lock_object, {:unless_exist => true, :expires_in => duration_seconds} )

Previously this would return true or false based upon the write succeeding. Now, since this always returns false, our application behaves as though it can never write and thus obtain a lock.

We are hosted by Heroku and want to support their upgrade to the latest version of memcached but are unable to do so until we can resolve this issue.

ghost commented 14 years ago

You will have to reopen or subclass the Memcached::Rails class in your app to support this legacy API (or reopen and fix MemCacheStore). We're not going to return strings for all calls.

Writes always succeed and return true unless there is an exception, in which case they raise it. There is no need to let request serialization details bubble up to the level of the caller.

drobbecke commented 14 years ago

Thanks for the feedback. I went ahead and reopened the Memcached::Rails class to modify the return values where appropriate to values that ActiveSupport Cache::MemCacheStore seems to expect.

Including the changes I made below in case any others run into the same issue.

# Wraps Memcached#set.
#
# Return Response::STORED rather than nil
def set(key, value, ttl=@default_ttl, raw=false)
  super(key, value, ttl, !raw)
  return ActiveSupport::Cache::MemCacheStore::Response::STORED
end

# Wraps Memcached#add so that it doesn't raise.
#
# Return Response::STORED/NOT_STORED rather than true or false
def add(key, value, ttl=@default_ttl, raw=false)
  super(key, value, ttl, !raw)
  return ActiveSupport::Cache::MemCacheStore::Response::STORED
rescue NotStored
  return ActiveSupport::Cache::MemCacheStore::Response::NOT_STORED
end

# Wraps Memcached#delete so that it doesn't raise.
#
#Return Response::DELETED or NOT_FOUND
def delete(key, expiry=0)
  super(key)
  return ActiveSupport::Cache::MemCacheStore::Response::DELETED
rescue NotFound
  return ActiveSupport::Cache::MemCacheStore::Response::NOT_FOUND
end