From 5720e9158958d9daad7a3beebf281de18f129859 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Thu, 19 Nov 2015 17:40:02 -0800 Subject: [PATCH 1/2] use normalized keys everywhere to stay close to rails 5 https://github.com/rails/rails/pull/22215 --- .../cache/libmemcached_store.rb | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/active_support/cache/libmemcached_store.rb b/lib/active_support/cache/libmemcached_store.rb index 0a55841..757b96c 100644 --- a/lib/active_support/cache/libmemcached_store.rb +++ b/lib/active_support/cache/libmemcached_store.rb @@ -91,7 +91,7 @@ def fetch(key, options = nil, &block) if options && options[:race_condition_ttl] && options[:expires_in] fetch_with_race_condition_ttl(key, options, &block) else - key = expanded_key(key) + key = normalize_key(key) unless options && options[:force] entry = instrument(:read, key, options) do |payload| read_entry(key, options).tap do |result| @@ -120,7 +120,7 @@ def fetch(key, options = nil, &block) end def read(key, options = nil) - key = expanded_key(key) + key = normalize_key(key) instrument(:read, key, options) do |payload| entry = read_entry(key, options) payload[:hit] = !!entry if payload @@ -129,24 +129,24 @@ def read(key, options = nil) end def write(key, value, options = nil) - key = expanded_key(key) + key = normalize_key(key) instrument(:write, key, options) do |payload| write_entry(key, value, options) end end def delete(key, options = nil) - key = expanded_key(key) + key = normalize_key(key) instrument(:delete, key) do |payload| delete_entry(key, options) end end def exist?(key, options = nil) - key = expanded_key(key) + key = normalize_key(key) instrument(:exist?, key) do |payload| if @cache.respond_to?(:exist) - @cache.exist(escape_and_normalize(key)) + @cache.exist(key) true else read_entry(key, options) != nil @@ -157,9 +157,9 @@ def exist?(key, options = nil) end def increment(key, amount = 1, options = nil) - key = expanded_key(key) + key = normalize_key(key) instrument(:increment, key, amount: amount) do - @cache.incr(escape_and_normalize(key), amount) + @cache.incr(key, amount) end rescue Memcached::NotFound nil @@ -169,9 +169,9 @@ def increment(key, amount = 1, options = nil) end def decrement(key, amount = 1, options = nil) - key = expanded_key(key) + key = normalize_key(key) instrument(:decrement, key, amount: amount) do - @cache.decr(escape_and_normalize(key), amount) + @cache.decr(key, amount) end rescue Memcached::NotFound nil @@ -186,7 +186,7 @@ def read_multi(*names) return {} if names.empty? - mapping = Hash[names.map {|name| [escape_and_normalize(expanded_key(name)), name] }] + mapping = Hash[names.map {|name| [normalize_key(name), name] }] keys = mapping.keys raw_values, flags = instrument(:read_multi, keys, options) do @cache.get(keys, false, true) @@ -216,7 +216,7 @@ def stats def read_entry(key, options = nil) options ||= {} - raw_value, flags = @cache.get(escape_and_normalize(key), false, true) + raw_value, flags = @cache.get(key, false, true) value = deserialize(raw_value, options[:raw], flags) convert_race_condition_entry(value, options) rescue Memcached::NotFound @@ -237,7 +237,7 @@ def write_entry(key, entry, options = nil) flags |= FLAG_COMPRESSED end - @cache.send(method, escape_and_normalize(key), entry, options[:expires_in].to_i, false, flags) + @cache.send(method, key, entry, options[:expires_in].to_i, false, flags) true rescue Memcached::Error => e log_error(e) @@ -245,7 +245,7 @@ def write_entry(key, entry, options = nil) end def delete_entry(key, options = nil) - @cache.delete(escape_and_normalize(key)) + @cache.delete(key) true rescue Memcached::NotFound false @@ -296,6 +296,10 @@ def deserialize(value, raw = false, flags = 0) value end + def normalize_key(key) + escape_and_normalize(expanded_key(key)) + end + def escape_and_normalize(key) key = key.to_s.force_encoding("BINARY").gsub(ESCAPE_KEY_CHARS) { |match| "%#{match.getbyte(0).to_s(16).upcase}" } key_length = key.length From 193a07328b4da6369a18e60fef1a07401a75fb1c Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Thu, 19 Nov 2015 21:43:30 -0800 Subject: [PATCH 2/2] cache at the key level --- .../cache/libmemcached_local_store.rb | 25 +++++++++---------- .../cache/libmemcached_store.rb | 23 +++++++++++------ .../libmemcached_local_store_test.rb | 9 +++++++ 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/lib/active_support/cache/libmemcached_local_store.rb b/lib/active_support/cache/libmemcached_local_store.rb index ec68995..693254d 100644 --- a/lib/active_support/cache/libmemcached_local_store.rb +++ b/lib/active_support/cache/libmemcached_local_store.rb @@ -34,26 +34,23 @@ def read(*args) end # make read multi hit local cache - def read_multi(*names) + def read_multi_entry(keys, options) return super unless cache = local_cache - options = names.extract_options! - - missing_names = [] + missing_keys = [] # We write raw values to the local cache, unlike rails MemcachedStore, so we cannot use local_cache.read_multi. # Once read_multi_entry is available we can switch to that. - results = names.each_with_object({}) do |name, results| - value = local_cache.fetch_entry(name) do - missing_names << name + results = keys.each_with_object({}) do |key, results| + value = local_cache.fetch_entry(key) do + missing_keys << key nil end - results[name] = value unless value.nil? + results[key] = value unless value.nil? end - if missing_names.any? - missing_names << options - missing = super(*missing_names) + if missing_keys.any? + missing = super(missing_keys, options) missing.each { |k,v| cache.write_entry(k, v, nil) } results.merge!(missing) end @@ -94,9 +91,11 @@ def read_entry(key, options) # for memcached writing raw means writing a string and not the actual value def write_entry(key, entry, options) # :nodoc: + return super unless cache = local_cache + written = super - if options && options[:raw] && local_cache && written - local_cache.write_entry(key, Entry.new(entry.to_s), options) + if options && options[:raw] && written + cache.write_entry(key, Entry.new(entry.to_s), options) end written end diff --git a/lib/active_support/cache/libmemcached_store.rb b/lib/active_support/cache/libmemcached_store.rb index 757b96c..22dd30c 100644 --- a/lib/active_support/cache/libmemcached_store.rb +++ b/lib/active_support/cache/libmemcached_store.rb @@ -188,15 +188,13 @@ def read_multi(*names) mapping = Hash[names.map {|name| [normalize_key(name), name] }] keys = mapping.keys - raw_values, flags = instrument(:read_multi, keys, options) do - @cache.get(keys, false, true) - end - - values = {} - raw_values.each do |key, value| - values[mapping[key]] = convert_race_condition_entry(deserialize(value, options[:raw], flags[key])) + instrument(:read_multi, keys, options) do + values = {} + read_multi_entry(keys, options).each do |key, value| + values[mapping[key]] = value + end + values end - values rescue Memcached::Error => e log_error(e) {} @@ -214,6 +212,15 @@ def stats protected + def read_multi_entry(keys, options) + raw_values, flags = @cache.get(keys, false, true) + values = {} + raw_values.each do |key, value| + values[key] = convert_race_condition_entry(deserialize(value, options[:raw], flags[key])) + end + values + end + def read_entry(key, options = nil) options ||= {} raw_value, flags = @cache.get(key, false, true) diff --git a/test/active_support/libmemcached_local_store_test.rb b/test/active_support/libmemcached_local_store_test.rb index e64dfc5..7f911c4 100644 --- a/test/active_support/libmemcached_local_store_test.rb +++ b/test/active_support/libmemcached_local_store_test.rb @@ -127,5 +127,14 @@ def self.it_wawo(description, &block) assert_equal expected, store.read_multi('a', 'b', 'c') assert_equal({}, store.read_multi) end + + it "can read interchangeable keys with read_multi" do + @cache.with_local_cache do + @cache.write 'x', 1 # write plain key + @cache.read_multi('x').must_equal('x' => 1) + @memcache.set 'x', 2 + @cache.read_multi(['x']).must_equal('x' => 1) # reads normalized key + end + end end end