Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  Evgeniy Dolzhenko April 22nd, 2010 @ 08:18 AM

  Wow that's massive,
  +1

  (btw. why there is the space sometimes after the function name like def expanded_key (key) # :nodoc: ?)

• 

  Pratik April 22nd, 2010 @ 11:41 AM

  • Assigned user set to “Jeremy Kemper”
• 

  Jeremy Kemper April 22nd, 2010 @ 05:39 PM

  • Milestone cleared.
  • State changed from “new” to “open”

  Great patch!

  We use ActiveSupport::Cache.expand_cache_key in our apps a lot. Everything except ENV["RAILS_CACHE_ID"] || ENV["RAILS_APP_VERSION"] is not Rails-specific, and those are for global namespacing. Perhaps ActionController::Caching.expand_cache_key could use AS::Cache.expand_cache_key and pass these env vars along if present.

• You flagged this item as spam.
  

  Brian Durand April 23rd, 2010 @ 03:12 AM

  • Assigned user cleared.

  I have another patch to apply on top of this one. It fixes FileStore and MemCacheStore so that they don't exceed limits on key length (memcached limit is 250 characters and file systems are generally 255).

  FileStore still may have issues on some (Windows mostly) file systems, but I don't see away around that and still keeping the delete_matched method. Personally, I'd be OK with getting rid of delete_matched since it doesn't work on MemCacheStore so it isn't reliably available.

  • active_support_cache_keys_of_any_length.diff 5.3 KB

• You flagged this item as spam.
  

  Daniel Schierbeck April 23rd, 2010 @ 09:59 AM

  This patch looks great! Reading through the diff, I have but a few remarks.

  1. The patch does not apply cleanly on the current Rails master (19cecc907f2c97458519f103cbb967cf8dda5716)
     
  2. The definition of ActiveSupport::Cache::Store#initialize places a space between the method name and the parenthesis. This style appears several places throughout the patch, and doesn't comform with the style guidelines (or normal Ruby style). In particular, cleanup, clear, key_matcher and the rest of the methods in the vicinity are defined like that.

  These are probably easily fixable, so it's an overall +1 from me :-)

• You flagged this item as spam.
  

  Brian Durand April 23rd, 2010 @ 03:31 PM

  • Assigned user set to “Jeremy Kemper”

  oops, accidentally unassigned

• You flagged this item as spam.
  

  Brian Durand April 23rd, 2010 @ 04:52 PM

  I've updated the patch again to remove the offending spaces and rebase to master again.

  The latest version also adds some more synchronization on MemoryStore.

  I also moved delete_matched, increment, and decrement from being implemented by default to being not implemented by default and explicitly documenting that only fetch, write, read, exist?, and delete are supported by all implementations. It adds a bit of duplicate code, but after thinking about it I realized increment and decrement will always be more than just a simple read/write and either need to be synchronized or use an atomic operator in the implementation code.

  • active_support_cache_refactoring_2.diff 86.5 KB

• You flagged this item as spam.
  

  Brian Durand April 23rd, 2010 @ 04:57 PM

  Jeremy, I agree. If there are a lot of uses of expand_cache_key in the wild let's keep it (I don't use it and was only looking in the Rails code base). My concern was only with the Rails specific variables being used.

• You flagged this item as spam.
  

  tonycoco April 23rd, 2010 @ 05:41 PM

  This looks like an excellent patch. I'll have to dig around a bit more in the patch, but it looks to be very concise and has refactored many parts where it was much needed. +1

• You flagged this item as spam.
  

  Brian Durand April 26th, 2010 @ 03:50 PM

  Attaching new patch which removes the deprecation of ActiveSupport::Cache.expand_cache_key.

  • active_support_cache_refactoring_3.diff 82.4 KB
• 

  Jeremy Kemper April 26th, 2010 @ 09:59 PM

  Three issues with MemoryStore:

  Bounds don't work since non-String values may be cached and #size for them is rarely byte-size. Same deal for 1.9 String, have to use #bytesize. Perhaps the bound should be on number of entries.

  LRU pruning is limited by timestamp resolution, so the unit tests will fail on fast machines. They test strict ordering.

  The pruning thread means the cache can't be used on Google App Engine. I'm not sure whether we can detect whether threads are available, but blocking to prune the cache would be acceptable in that case.

• You flagged this item as spam.
  

  Brian Durand April 27th, 2010 @ 05:24 PM

  Another patch to fix MemoryStore.

  1. Made the logic for Entry#size more sophisticated so that if the object responds to :bytesize, use that, otherwise Marshal.dump the object and get the bytesize. This adds a bit of overhead for more complex objects but not much (~1.5ms for a large array in my tests). For strings, numbers, or hashes of strings and numbers the overhead is negligible.

  2. Added sleep(0.001) statements to pruning tests to throttle fast CPUs.

  3. Removed pruning thread in favor of a max pruning time (default 2 seconds). The intent of the Thread was to not tie up request threads pruning a large cache. This will provide an throttle without the need for a separate thread.

  4. Added Entry#create method so it is possible for implementations to recreate entries from a native format if desired.

  • active_support_cache_refactoring_4.diff 85.5 KB
• 

  Repository April 27th, 2010 @ 07:20 PM

  • State changed from “open” to “committed”

  (from [ee51b51b60f9e6cce9babed2c8a65a14d87790c8]) ActiveSupport::Cache refactoring

  All Caches

  • Add default options to initializer that will be sent to all read, write, fetch, exist?, increment, and decrement
  • Add support for the :expires_in option to fetch and write for all caches. Cache entries are stored with the create timestamp and a ttl so that expiration can be handled independently of the implementation.
  • Add support for a :namespace option. This can be used to set a global prefix for cache entries.
  • Deprecate expand_cache_key on ActiveSupport::Cache and move it to ActionController::Caching and ActionDispatch::Http::Cache since the logic in the method used some Rails specific environment variables and was only used by ActionPack classes. Not very DRY but there didn't seem to be a good shared spot and ActiveSupport really shouldn't be Rails specific.
  • Add support for :race_condition_ttl to fetch. This setting can prevent race conditions on fetch calls where several processes try to regenerate a recently expired entry at once.
  • Add support for :compress option to fetch and write which will compress any data over a configurable threshold.
  • Nil values can now be stored in the cache and are distinct from cache misses for fetch.
  • Easier API to create new implementations. Just need to implement the methods read_entry, write_entry, and delete_entry instead of overwriting existing methods.
  • Since all cache implementations support storing objects, update the docs to state that ActiveCache::Cache::Store implementations should store objects. Keys, however, must be strings since some implementations require that.
  • Increase test coverage.
  • Document methods which are provided as convenience but which may not be universally available.

  MemoryStore

  • MemoryStore can now safely be used as the cache for single server sites.
  • Make thread safe so that the default cache implementation used by Rails is thread safe. The overhead is minimal and it is still the fastest store available.
  • Provide :size initialization option indicating the maximum size of the cache in memory (defaults to 32Mb).
  • Add prune logic that removes the least recently used cache entries to keep the cache size from exceeding the max.
  • Deprecated SynchronizedMemoryStore since it isn't needed anymore.

  FileStore

  • Escape key values so they will work as file names on all file systems, be consistent, and case sensitive
  • Use a hash algorithm to segment the cache into sub directories so that a large cache doesn't exceed file system limits.
  • FileStore can be slow so implement the LocalCache strategy to cache reads for the duration of a request.
  • Add cleanup method to keep the disk from filling up with expired entries.
  • Fix increment and decrement to use file system locks so they are consistent between processes.

  MemCacheStore

  • Support all keys. Previously keys with spaces in them would fail
  • Deprecate CompressedMemCacheStore since it isn't needed anymore (use :compress => true)

  [#4452 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/ee51b51b60f9e6cce9babed2c8a65a...

• 

  Ryan Bigg April 28th, 2010 @ 12:13 AM

  Wow, what a patch! Fantastic work Brian!

• You flagged this item as spam.
  

  Lawrence Pit May 19th, 2010 @ 05:03 AM

  Great patch indeed. Can I ask you guys to look at this one as well? :

  https://rails.lighthouseapp.com/projects/8994/tickets/4588-cache-in...

  It has some minor doc fixes to this patch + makes increment/decrement consistent for all cache stores, particularly Memcache + adds the ability to provide a block to increment/decrement similar to how +fetch+ works.

  Thanks,
  Lawrence

• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”
  • Importance changed from “” to “Low”

  [bulk edit]

• You flagged this item as spam.
  

  Tomasz Mazur December 7th, 2010 @ 11:04 AM

  +1 Highly recommended patch