ngmoco / cache-money

A Write-Through Cacheing Library for ActiveRecord
Apache License 2.0
161 stars 31 forks source link

Cash::Lock#acquire_lock Causes NoMethodError #26

Closed sshaw closed 12 years ago

sshaw commented 12 years ago

Cash::Lock#acquire_lock calls get_server_for_key on the cache adapter: https://github.com/ngmoco/cache-money/blob/master/lib/cash/lock.rb#L36. This method is specific to memcache-client, when memcached is being used a NoMethodError is raised.

My pull request contains a fix for this and for the Redis adapter. The test cases for this error were already there, they just need to be modified to check for the right exception.

There is still an issue if memcache-client is being used. In this case the call to get_server_for_key can raise an unhandled exception. But, due to the current state of said lib and my uncertainty about this project's direction i.e., will memcache-client still be supported here, will you support Dalli in addition to or instead of it, Redis etc... I left it alone.

ashleym1972 commented 12 years ago

The method was only in the exception building so it's not critical to add it to the adapters. I also found a bug in exponential sleep. Thanks for your help on this.

I'm going to close this pull request. Please create another one to add yourself to the Acknowledgments and gemspec.

sshaw commented 12 years ago

The method was only in the exception building so it's not critical to add it to the adapters.

Though in environments with several nodes it can be useful to know what node was affected. I found this bug because memcached was down.

Also, any idea why the 2 test cases are failing in Cache::Finders under memcached?

ashleym1972 commented 12 years ago

I think that's why I put it in, so that I could tell which node was having problems. However, it's really not necessary since you should be able to tell what's going on as soon as you pop into the console. I might add it back, though.

The test cases are failing because cache_money's implementation is fundamentally flawed in those areas. So far we have not found a fix that doesn't break anything else.

ashleym1972 commented 12 years ago

One other thing. The wrappers really take care of the exceptions so if something does go wrong you are really going to be hitting the logs to find out exactly what went wrong and the logs should have everything you need.