Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All The Get Alls #298

Merged
merged 23 commits into from
Nov 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1140288
Ensure that get_all is only adapter call for memoizer
jnunemaker Oct 31, 2017
e930381
Define get_all for memory
jnunemaker Oct 31, 2017
f51f761
Change memoizable to really call get_all
jnunemaker Oct 31, 2017
e8f34bf
Add spec for memoizing cached adapters
jnunemaker Oct 31, 2017
7c23da5
Make rubo happy
jnunemaker Oct 31, 2017
332cea4
Add specs for memoized get_all and how it interacts with memoized ada…
jnunemaker Oct 31, 2017
fefadae
Only store all features once in memory for memoized
jnunemaker Oct 31, 2017
83c3645
Remove some duplication in memory adapter
jnunemaker Oct 31, 2017
752c503
Implement get_all for active record
jnunemaker Oct 31, 2017
55d674d
Do not require valid data set for sequel when declaring models
jnunemaker Oct 31, 2017
c063b81
Ensure that memoizable returns default config for any unknown features
jnunemaker Oct 31, 2017
8b3699d
Make all cache adapters work the same
jnunemaker Oct 31, 2017
30c7fbd
Make rubo happy
jnunemaker Oct 31, 2017
fad3a99
Fix activerecord joins syntax
jnunemaker Oct 31, 2017
704bf08
Return nil for not memoizing
jnunemaker Oct 31, 2017
384db77
Add get multi/all to read only
jnunemaker Nov 3, 2017
beaeaf0
Add get all/multi to pstore
jnunemaker Nov 3, 2017
88d14f5
Add get_all to sequel
jnunemaker Nov 3, 2017
367ad90
Add get_all to redis
jnunemaker Nov 3, 2017
44aecdf
Add get_all to mongo
jnunemaker Nov 3, 2017
e74f1a5
Fix rubocop
jnunemaker Nov 4, 2017
8b6e2cb
Finally make rubo happy
jnunemaker Nov 4, 2017
bc9971f
Use dalli cache store which does support unless_exist
jnunemaker Nov 4, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,6 @@ Style/TrailingCommaInLiteral:

RSpec/InstanceVariable:
Enabled: false

Style/AccessorMethodName:
Enabled: false
15 changes: 15 additions & 0 deletions lib/flipper/adapters/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,21 @@ def get_multi(features)
result
end

def get_all
joins = <<-EOJ
INNER JOIN #{@feature_class.table_name}
ON #{@feature_class.table_name}.key = #{@gate_class.table_name}.feature_key
EOJ
db_gates = @gate_class.joins(joins).all.to_a
grouped_db_gates = db_gates.group_by(&:feature_key)
result = Hash.new { |hash, key| hash[key] = default_config }
features = grouped_db_gates.keys.map { |key| Flipper::Feature.new(key, self) }
features.each do |feature|
result[feature.key] = result_for_feature(feature, grouped_db_gates[feature.key])
end
result
end

# Public: Enables a gate for a given thing.
#
# feature - The Flipper::Feature for the gate.
Expand Down
52 changes: 41 additions & 11 deletions lib/flipper/adapters/active_support_cache_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class ActiveSupportCacheStore
Version = 'v1'.freeze
Namespace = "flipper/#{Version}".freeze
FeaturesKey = "#{Namespace}/features".freeze
GetAllKey = "#{Namespace}/get_all".freeze

# Private
def self.key_for(key)
Expand All @@ -27,14 +28,12 @@ def initialize(adapter, cache, expires_in: nil)
@name = :active_support_cache_store
@cache = cache
@write_options = {}
@write_options.merge!(expires_in: expires_in) if expires_in
@write_options[:expires_in] = expires_in if expires_in
end

# Public
def features
@cache.fetch(FeaturesKey, @write_options) do
@adapter.features
end
read_feature_keys
end

# Public
Expand Down Expand Up @@ -67,17 +66,21 @@ def get(feature)
end

def get_multi(features)
keys = features.map { |feature| key_for(feature.key) }
result = @cache.read_multi(keys)
uncached_features = features.reject { |feature| result[feature.key] }
if uncached_features.any?
response = @adapter.get_multi(uncached_features)
read_many_features(features)
end

def get_all
if @cache.write(GetAllKey, Time.now.to_i, @write_options.merge(unless_exist: true))
response = @adapter.get_all
response.each do |key, value|
@cache.write(key_for(key), value, @write_options)
result[key] = value
end
@cache.write(FeaturesKey, response.keys.to_set, @write_options)
response
else
features = read_feature_keys.map { |key| Flipper::Feature.new(key, self) }
read_many_features(features)
end
result
end

## Public
Expand All @@ -99,6 +102,33 @@ def disable(feature, gate, thing)
def key_for(key)
self.class.key_for(key)
end

# Internal: Returns an array of the known feature keys.
def read_feature_keys
@cache.fetch(FeaturesKey, @write_options) { @adapter.features }
end

# Internal: Given an array of features, attempts to read through cache in
# as few network calls as possible.
def read_many_features(features)
keys = features.map { |feature| key_for(feature.key) }
cache_result = @cache.read_multi(*keys)
uncached_features = features.reject { |feature| cache_result[key_for(feature)] }

if uncached_features.any?
response = @adapter.get_multi(uncached_features)
response.each do |key, value|
@cache.write(key_for(key), value, @write_options)
cache_result[key_for(key)] = value
end
end

result = {}
features.each do |feature|
result[feature.key] = cache_result[key_for(feature.key)]
end
result
end
end
end
end
52 changes: 41 additions & 11 deletions lib/flipper/adapters/dalli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class Dalli
Version = 'v1'.freeze
Namespace = "flipper/#{Version}".freeze
FeaturesKey = "#{Namespace}/features".freeze
GetAllKey = "#{Namespace}/get_all".freeze

# Private
def self.key_for(key)
Expand All @@ -22,6 +23,9 @@ def self.key_for(key)
# Public: The name of the adapter.
attr_reader :name

# Public: The ttl for all cached data.
attr_reader :ttl

# Public
def initialize(adapter, cache, ttl = 0)
@adapter = adapter
Expand All @@ -32,9 +36,7 @@ def initialize(adapter, cache, ttl = 0)

# Public
def features
@cache.fetch(FeaturesKey, @ttl) do
@adapter.features
end
read_feature_keys
end

# Public
Expand Down Expand Up @@ -67,19 +69,21 @@ def get(feature)
end

def get_multi(features)
keys = features.map { |feature| key_for(feature.key) }
result = @cache.get_multi(keys)
uncached_features = features.reject { |feature| result[key_for(feature.key)] }
read_many_features(features)
end

if uncached_features.any?
response = @adapter.get_multi(uncached_features)
def get_all
if @cache.add(GetAllKey, Time.now.to_i, @ttl)
response = @adapter.get_all
response.each do |key, value|
@cache.set(key_for(key), value, @ttl)
result[key] = value
end
@cache.set(FeaturesKey, response.keys.to_set, @ttl)
response
else
features = read_feature_keys.map { |key| Flipper::Feature.new(key, self) }
read_many_features(features)
end

result
end

# Public
Expand All @@ -101,6 +105,32 @@ def disable(feature, gate, thing)
def key_for(key)
self.class.key_for(key)
end

def read_feature_keys
@cache.fetch(FeaturesKey, @ttl) { @adapter.features }
end

# Internal: Given an array of features, attempts to read through cache in
# as few network calls as possible.
def read_many_features(features)
keys = features.map { |feature| key_for(feature.key) }
cache_result = @cache.get_multi(keys)
uncached_features = features.reject { |feature| cache_result[key_for(feature.key)] }

if uncached_features.any?
response = @adapter.get_multi(uncached_features)
response.each do |key, value|
@cache.set(key_for(key), value, @ttl)
cache_result[key_for(key)] = value
end
end

result = {}
features.each do |feature|
result[feature.key] = cache_result[key_for(feature.key)]
end
result
end
end
end
end
49 changes: 31 additions & 18 deletions lib/flipper/adapters/memoizable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class Memoizable < SimpleDelegator
include ::Flipper::Adapter

FeaturesKey = :flipper_features
GetAllKey = :all_memoized

# Internal
attr_reader :cache
Expand All @@ -19,14 +20,18 @@ class Memoizable < SimpleDelegator
# Internal: The adapter this adapter is wrapping.
attr_reader :adapter

# Private
def self.key_for(key)
"feature/#{key}"
end

# Public
def initialize(adapter, cache = nil)
super(adapter)
@adapter = adapter
@name = :memoizable
@cache = cache || {}
@memoize = false
@all_fetched = false
end

# Public
Expand Down Expand Up @@ -63,7 +68,7 @@ def clear(feature)
# Public
def get(feature)
if memoizing?
cache.fetch(feature.key) { cache[feature.key] = @adapter.get(feature) }
cache.fetch(key_for(feature.key)) { cache[key_for(feature.key)] = @adapter.get(feature) }
else
@adapter.get(feature)
end
Expand All @@ -72,18 +77,18 @@ def get(feature)
# Public
def get_multi(features)
if memoizing?
uncached_features = features.reject { |feature| cache[feature.key] }
uncached_features = features.reject { |feature| cache[key_for(feature.key)] }

if uncached_features.any?
response = @adapter.get_multi(uncached_features)
response.each do |key, hash|
cache[key] = hash
cache[key_for(key)] = hash
end
end

result = {}
features.each do |feature|
result[feature.key] = cache[feature.key]
result[feature.key] = cache[key_for(feature.key)]
end
result
else
Expand All @@ -93,20 +98,25 @@ def get_multi(features)

def get_all
if memoizing?
unless @all_fetched
result = @adapter.get_all
result.each do |key, value|
cache[key] = value
response = nil
if cache[GetAllKey]
response = {}
cache[FeaturesKey].each do |key|
response[key] = cache[key_for(key)]
end
cache[FeaturesKey] = result.keys.to_set
@all_fetched = true
else
response = @adapter.get_all
response.each do |key, value|
cache[key_for(key)] = value
end
cache[FeaturesKey] = response.keys.to_set
cache[GetAllKey] = true
end

result = {}
features.each do |key|
result[key] = cache[key]
end
result
# Ensures that looking up other features that do not exist doesn't
# result in N+1 adapter calls.
response.default_proc = ->(memo, key) { memo[key] = default_config }
response
else
@adapter.get_all
end
Expand All @@ -130,7 +140,6 @@ def disable(feature, gate, thing)
#
# value - The Boolean that decides if local caching is on.
def memoize=(value)
@all_fetched = false
cache.clear
@memoize = value
end
Expand All @@ -142,8 +151,12 @@ def memoizing?

private

def key_for(key)
self.class.key_for(key)
end

def expire_feature(feature)
cache.delete(feature.key) if memoizing?
cache.delete(key_for(feature.key)) if memoizing?
end

def expire_features_set
Expand Down
Loading