Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Per-request in memory cache for all communication with the MemCache server(s)

#1653

In November I released a plugin (http://tinyurl.com/7q5fqh) that enhances MemCacheStore by using a per-request local cache to buffer duplicate memcache reads which can result in halving the number of read requests made. I've been asked to add that functionality to Edge and this is that work.

The description linked to essentially describes two changes, the first to MemCacheStore which this patch applies directly to that class instead of creating a new Store class. And secondly tidy ups to the MemCache Session Store which this patch does not implement. I need to follow up about Joshua's comment on ticket #949 Memcache session does gets twice to see if I can help there.

Reported by Nahum Wild · December 28th, 2008 @ 07:28 AM

State: resolved
Milestone: none
Assigned to: josh josh
Importance: Low

Activity

  1. Nahum Wild
    Nahum Wild

    Ah, it's not thread safe - working on that now.

    December 28th, 2008 @ 08:41 PM

  2. Jeffrey Hardy
    Jeffrey Hardy

    I totally dig the idea of this. It's similar to what Nick Kallen's cache_money plugin does:

    http://github.com/nkallen/cache-...

    The main thing that bothers me about the implementation in this patch is that it knows about Action Controller. The cache store shouldn't know Action Controller even exists. I'd much rather there be a facility to tell the cache store to use the local cache or not, such that we could declare the setting from AC's side.

    January 6th, 2009 @ 05:56 PM

  3. Nahum Wild
    Nahum Wild

    Jeffrey, I totally agree with you on the subject of the class knowing about ActionController. In fact my delay in updating this ticket with a thread safe version is because I've been trying to solve that problem nicely.

    I like simply going:

    config.action_controller.session_store = :mem_cache_store

    to setup memcache in rails. I don't really want people to have to do extra things to get it going when it should just work seamlessly without any effort. At the moment it still depends on AC.

    Doing the local caching actually solves a bug in rails, something I've not directly said above. It stops the problem of entries in memcache changing in between the execution of the action and view code. I've had this happen a number of times, pain in the butt to track down. I probably should have tagged this with bug too maybe...

    Anyway, I've attached an updated patch with my changes being thread safe now plus a minor fix to the 'exist?' method.

    January 8th, 2009 @ 09:19 PM

  4. Nahum Wild
    Nahum Wild

    Just to clariy for those reading, the dependency on ActionController is because MemCacheStore hooks into the Dispatcher before_dispatch callback to clear the local cache before each request is processed.

    January 8th, 2009 @ 09:27 PM

  5. DHH
    DHH
    • Assigned user changed from DHH to josh

    Josh, where should we best put this reset call?

    January 12th, 2009 @ 01:49 PM

  6. josh
    josh
    • Milestone cleared.
    • State changed from new to open

    I think a middleware hook would be most appropriate.

    use ActiveSupport::Cache::MemCacheStore::LocalCache # or Buffering

    You could have the middleware initialize and reset the local cache as well. Otherwise the cache store could function standalone without the local buffer.

    January 12th, 2009 @ 02:58 PM

  7. Repository
    Repository
    • State changed from open to resolved

    (from [a53ad5bba37199047ba20194933e122bf6b0252f]) Added in a local per request cache to MemCacheStore. It acts as a buffer to stop unneccessary requests being sent through to memcache [#1653 Per-request in memory cache for all communication with the MemCache server(s) state:resolved] Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...

    January 16th, 2009 @ 03:25 AM

  8. josh
    josh

    Hey, I just commit it.

    I made some changes, it uses a thread local instead of a lock. Should be much less overhead.

    Please review and make sure it still functions how you want.

    January 16th, 2009 @ 03:26 AM

  9. Nahum Wild
    Nahum Wild

    Will do thanks for doing that. I'll look at it right now. (Clients discovered I had spare time :-(

    January 16th, 2009 @ 04:09 AM

  10. Nahum Wild
    Nahum Wild

    Yep, functions as intended. :-)

    January 16th, 2009 @ 06:29 AM

  11. Nahum Wild
    Nahum Wild

    Oh and thanks for giving it some polish Joshua.

    January 16th, 2009 @ 06:35 AM

  12. josh
    josh

    Interesting comments on github:

    http://github.com/rails/rails/co...

    Maybe it should be a subclass or extension module?

    January 16th, 2009 @ 02:40 PM

  13. Lourens Naudé
    Lourens Naudé

    Josh,

    Initial backend agnostic local cache strategy attached.Introduced an ActiveSupport::Cache::Strategy::* namespace, which may be frowned upon, but may be useful for similar optimizations.

    Original test cases passes.

    January 17th, 2009 @ 03:23 AM

  14. josh
    josh
    • State changed from resolved to open

    @Lourens Thanks

    I think it can be implemented cleaner with extend and super. I'll take care of that though.

    Thanks for the patch, will commit soon.

    January 17th, 2009 @ 03:52 AM

  15. Repository
    Repository
    • State changed from open to resolved

    (from [b08c96887538cf53670bb882e79996582375e6c9]) Decouple the local cache strategy from MemCacheStore for reuse with other remote stores [#1653 Per-request in memory cache for all communication with the MemCache server(s) state:resolved]

    Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...

    January 18th, 2009 @ 12:01 AM

  16. Lourens Naudé
    Lourens Naudé

    @ Josh,

    Am aware of two apps where the immutable snippet in #read raises with TypeError when the value being dup'ed is a special case constant eg. Fixnum, TrueClass etc.

    Here's the reference to the MRI implementation of Object#clone, Object#dup ( object.c ) && the special constant declaration ( ruby.h ) from a 187 checkout :

    http://gist.github.com/54069

    Attached is a patch that implements Object#dup? to better handle such cases.

    Thoughts ?

    • Lourens

    January 28th, 2009 @ 05:25 PM

  17. Lourens Naudé
  18. josh
    josh

    Maybe we could just add dup to Integer and just return self?

    January 28th, 2009 @ 06:35 PM

  19. Lourens Naudé
    Lourens Naudé

    @ Josh,

    Interesting, but isn't it dodgy practice to mask the TypeError in non-framework code VS being able to conditionally guard against it in specific framework areas, like the local cache, to not blow up the whole process when that happens ?

    January 28th, 2009 @ 08:12 PM

  20. Lourens Naudé
    Lourens Naudé

    Josh,

    Nevermind, references :

    "* object.duplicable? returns true if object.dup is safe. False for nil, true, false, symbols, and numbers; true otherwise. #9333 [sur]"

    Patch that piggy backs on that attached.

    January 29th, 2009 @ 02:38 AM

  21. Repository
    Repository

    (from [2e69db18ce2815c25eee64fc2978e7f6b57f6e1f]) Only dup local cache values if duplicable [#1653 Per-request in memory cache for all communication with the MemCache server(s) state:resolved] http://github.com/rails/rails/co...

    January 29th, 2009 @ 03:18 AM

  22. Ryan Bigg
    Ryan Bigg
    • Tag cleared.
    • Importance changed from to Low

    Automatic cleanup of spam.

    October 9th, 2010 @ 09:54 PM