Merge pull request #648 from yankaindustries/master

Add Split::Cache to reduce load on Redis

Author: andrehjr

README.md

@@ -756,6 +756,21 @@ split_config = YAML.load_file(Rails.root.join('config', 'split.yml'))
 Split.redis = split_config[Rails.env]
 ```
 
+### Redis Caching
+
+In some high-volume usage scenarios, Redis load can be incurred by repeated 
+fetches for fairly static data.  Enabling caching will reduce this load, but 
+require a restart for changes to experiment definitions to take effect.
+
+ ```ruby
+Split.configuration.cache = true
+````
+
+This currently caches:
+  - `Split::ExperimentCatalog.find`
+  - `Split::Experiment.start_time`
+  - `Split::Experiment.winner`
+
 ## Namespaces
 
 If you're running multiple, separate instances of Split you may want

lib/split.rb

@@ -6,6 +6,7 @@
 require 'split/algorithms/weighted_sample'
 require 'split/algorithms/whiplash'
 require 'split/alternative'
+require 'split/cache'
 require 'split/configuration'
 require 'split/encapsulated_helper'
 require 'split/exceptions'
@@ -65,6 +66,10 @@ def configure
     self.configuration ||= Configuration.new
     yield(configuration)
   end
+
+  def cache(namespace, key, &block)
+    Split::Cache.fetch(namespace, key, &block)
+  end
 end
 
 # Check to see if being run in a Rails application.  If so, wait until before_initialize to run configuration so Gems that create ENV variables have the chance to initialize first.

lib/split/cache.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module Split
+  class Cache
+
+    def self.clear
+      @cache = nil
+    end
+
+    def self.fetch(namespace, key)
+      return yield unless Split.configuration.cache
+
+      @cache ||= {}
+      @cache[namespace] ||= {}
+
+      value = @cache[namespace][key]
+      return value if value
+
+      @cache[namespace][key] = yield
+    end
+  end
+end

lib/split/configuration.rb

@@ -28,6 +28,7 @@ class Configuration
     attr_accessor :winning_alternative_recalculation_interval
     attr_accessor :redis
     attr_accessor :dashboard_pagination_default_per_page
+    attr_accessor :cache
 
     attr_reader :experiments
 

lib/split/experiment.rb

@@ -128,11 +128,13 @@ def alternatives=(alts)
     end
 
     def winner
-      experiment_winner = redis.hget(:experiment_winner, name)
-      if experiment_winner
-        Split::Alternative.new(experiment_winner, name)
-      else
-        nil
+      Split.cache(:experiment_winner, name) do
+        experiment_winner = redis.hget(:experiment_winner, name)
+        if experiment_winner
+          Split::Alternative.new(experiment_winner, name)
+        else
+          nil
+        end
       end
     end
 
@@ -165,13 +167,15 @@ def start
     end
 
     def start_time
-      t = redis.hget(:experiment_start_times, @name)
-      if t
-        # Check if stored time is an integer
-        if t =~ /^[-+]?[0-9]+$/
-          Time.at(t.to_i)
-        else
-          Time.parse(t)
+      Split.cache(:experiment_start_times, @name) do
+        t = redis.hget(:experiment_start_times, @name)
+        if t
+          # Check if stored time is an integer
+          if t =~ /^[-+]?[0-9]+$/
+            Time.at(t.to_i)
+          else
+            Time.parse(t)
+          end
         end
       end
     end

lib/split/experiment_catalog.rb

@@ -14,8 +14,10 @@ def self.all_active_first
     end
 
     def self.find(name)
-      return unless Split.redis.exists?(name)
-      Experiment.new(name).tap { |exp| exp.load_from_redis }
+      Split.cache(:experiment_catalog, name) do
+        return unless Split.redis.exists?(name)
+        Experiment.new(name).tap { |exp| exp.load_from_redis }
+      end
     end
 
     def self.find_or_initialize(metric_descriptor, control = nil, *alternatives)
@@ -47,6 +49,5 @@ def self.normalize_experiment(metric_descriptor)
       return experiment_name, goals
     end
     private_class_method :normalize_experiment
-
   end
 end

spec/cache_spec.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Split::Cache do
+
+  let(:namespace) { :test_namespace }
+  let(:key) { :test_key }
+  let(:now) { 1606189017 }
+
+  before { allow(Time).to receive(:now).and_return(now) }
+
+  describe 'clear' do
+
+    before { Split.configuration.cache = true }
+
+    it 'clears the cache' do
+      expect(Time).to receive(:now).and_return(now).exactly(2).times
+      Split::Cache.fetch(namespace, key) { Time.now }
+      Split::Cache.clear
+      Split::Cache.fetch(namespace, key) { Time.now }
+    end
+  end
+
+  describe 'fetch' do
+
+    subject { Split::Cache.fetch(namespace, key) { Time.now } }
+
+    context 'when cache disabled' do
+
+      before { Split.configuration.cache = false }
+
+      it 'returns the yield' do
+        expect(subject).to eql(now)
+      end
+
+      it 'yields every time' do
+        expect(Time).to receive(:now).and_return(now).exactly(2).times
+        Split::Cache.fetch(namespace, key) { Time.now }
+        Split::Cache.fetch(namespace, key) { Time.now }
+      end
+    end
+
+    context 'when cache enabled' do
+
+      before { Split.configuration.cache = true }
+
+      it 'returns the yield' do
+        expect(subject).to eql(now)
+      end
+
+      it 'yields once' do
+        expect(Time).to receive(:now).and_return(now).once
+        Split::Cache.fetch(namespace, key) { Time.now }
+        Split::Cache.fetch(namespace, key) { Time.now }
+      end
+
+      it 'honors namespace' do
+        expect(Split::Cache.fetch(:a, key) { :a }).to eql(:a)
+        expect(Split::Cache.fetch(:b, key) { :b }).to eql(:b)
+
+        expect(Split::Cache.fetch(:a, key) { :a }).to eql(:a)
+        expect(Split::Cache.fetch(:b, key) { :b }).to eql(:b)
+      end
+
+      it 'honors key' do
+        expect(Split::Cache.fetch(namespace, :a) { :a }).to eql(:a)
+        expect(Split::Cache.fetch(namespace, :b) { :b }).to eql(:b)
+
+        expect(Split::Cache.fetch(namespace, :a) { :a }).to eql(:a)
+        expect(Split::Cache.fetch(namespace, :b) { :b }).to eql(:b)
+      end
+    end
+  end
+end

spec/dashboard_spec.rb

@@ -188,7 +188,7 @@ def link(color)
 
     it "calls disable of cohorting when action is disable" do
       post "/update_cohorting?experiment=#{experiment.name}", { "cohorting_action": "disable" }
-      
+
       expect(experiment.cohorting_disabled?).to eq true
     end
 

spec/spec_helper.rb

@@ -22,6 +22,7 @@ module GlobalSharedContext
     Split.redis = Redis.new
     Split.redis.select(10)
     Split.redis.flushdb
+    Split::Cache.clear
     @ab_user = mock_user
     params = nil
   end