From 832cc802f5c8cdf1f25b212992361cd25fd3b387 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Mon, 30 Jun 2014 14:00:02 -0700 Subject: [PATCH 01/22] add elb health check to exclusion list --- lib/split/configuration.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index b0cc3d3b..41922c55 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -85,6 +85,7 @@ def bots 'DigitalPersona Fingerprint Software' => 'HP Fingerprint scanner', 'ShowyouBot' => 'Showyou iOS app spider', 'ZyBorg' => 'Zyborg? Hmmm....', + 'ELB-HealthChecker' => 'ELB Health Check' } end From 0a67b907c0574ed34de56616c6c360ab2af8fd13 Mon Sep 17 00:00:00 2001 From: Marc Cull Date: Wed, 30 Sep 2015 11:58:30 -0700 Subject: [PATCH 02/22] +connection pool --- Appraisals | 5 +- gemfiles/3.0.gemfile | 4 +- gemfiles/3.0.gemfile.lock | 70 +++++++++--- gemfiles/3.1.gemfile | 5 +- gemfiles/3.2.gemfile | 5 +- gemfiles/4.0.gemfile | 5 +- lib/split.rb | 9 +- lib/split/alternative.rb | 46 +++++--- lib/split/experiment.rb | 150 +++++++++++++++---------- lib/split/experiment_catalog.rb | 15 ++- lib/split/metric.rb | 30 +++-- lib/split/persistence/redis_adapter.rb | 16 ++- spec/alternative_spec.rb | 4 +- spec/dashboard_spec.rb | 5 +- spec/experiment_spec.rb | 25 +++-- spec/spec_helper.rb | 4 +- split.gemspec | 1 + 17 files changed, 261 insertions(+), 138 deletions(-) diff --git a/Appraisals b/Appraisals index a67b688b..92690716 100644 --- a/Appraisals +++ b/Appraisals @@ -1,19 +1,22 @@ appraise "3.0" do gem "rails", "~> 3.0.20" - gem "split", :path => "../" + gem "connection_pool" end appraise "3.1" do gem "rails", "~> 3.1.12" + gem "connection_pool" gem "split", :path => "../" end appraise "3.2" do gem "rails", "~> 3.2.13" + gem "connection_pool" gem "split", :path => "../" end appraise "4.0" do gem "rails", "4.0.0.rc1" + gem "connection_pool" gem "split", :path => "../" end \ No newline at end of file diff --git a/gemfiles/3.0.gemfile b/gemfiles/3.0.gemfile index b79d01a8..e138155c 100644 --- a/gemfiles/3.0.gemfile +++ b/gemfiles/3.0.gemfile @@ -4,6 +4,6 @@ source "https://rubygems.org" gem "appraisal" gem "rails", "~> 3.0.20" -gem "split", :path=>"../" +gem "connection_pool" -gemspec :path=>"../" \ No newline at end of file +gemspec :path => "../" diff --git a/gemfiles/3.0.gemfile.lock b/gemfiles/3.0.gemfile.lock index c067da73..09111ad8 100644 --- a/gemfiles/3.0.gemfile.lock +++ b/gemfiles/3.0.gemfile.lock @@ -1,7 +1,8 @@ PATH - remote: /Users/andrew/code/split + remote: ../ specs: - split (0.6.1) + split (0.7.2) + connection_pool redis (>= 2.1) redis-namespace (>= 1.1.0) simple-random @@ -41,10 +42,23 @@ GEM bundler rake arel (2.0.10) + backports (3.6.6) builder (2.1.2) - diff-lcs (1.2.2) + connection_pool (2.2.0) + coveralls (0.7.1) + multi_json (~> 1.3) + rest-client + simplecov (>= 0.7) + term-ansicolor + thor + diff-lcs (1.2.5) + docile (1.1.5) + domain_name (0.5.24) + unf (>= 0.0.5, < 1.0.0) erubis (2.6.6) abstract (>= 1.0.0) + http-cookie (1.0.2) + domain_name (~> 0.5) i18n (0.5.0) json (1.7.7) mail (2.2.19) @@ -53,6 +67,8 @@ GEM mime-types (~> 1.16) treetop (~> 1.4.8) mime-types (1.23) + multi_json (1.11.2) + netrc (0.10.3) polyglot (0.3.3) rack (1.2.7) rack-mount (0.6.14) @@ -76,27 +92,43 @@ GEM rake (10.0.4) rdoc (3.12.2) json (~> 1.4) - redis (3.0.3) - redis-namespace (1.2.1) - redis (~> 3.0.0) - rspec (2.13.0) - rspec-core (~> 2.13.0) - rspec-expectations (~> 2.13.0) - rspec-mocks (~> 2.13.0) - rspec-core (2.13.1) - rspec-expectations (2.13.0) + redis (3.2.1) + redis-namespace (1.5.2) + redis (~> 3.0, >= 3.0.4) + rest-client (1.8.0) + http-cookie (>= 1.0.2, < 2.0) + mime-types (>= 1.16, < 3.0) + netrc (~> 0.7) + rspec (2.99.0) + rspec-core (~> 2.99.0) + rspec-expectations (~> 2.99.0) + rspec-mocks (~> 2.99.0) + rspec-core (2.99.2) + rspec-expectations (2.99.2) diff-lcs (>= 1.1.3, < 2.0) - rspec-mocks (2.13.0) - simple-random (0.9.3) - sinatra (1.2.8) - rack (~> 1.1) + rspec-mocks (2.99.4) + simple-random (1.0.1) + simplecov (0.9.2) + docile (~> 1.1.0) + multi_json (~> 1.0) + simplecov-html (~> 0.9.0) + simplecov-html (0.9.0) + sinatra (1.2.9) + backports + rack (~> 1.1, < 1.5) tilt (>= 1.2.2, < 2.0) + term-ansicolor (1.3.2) + tins (~> 1.0) thor (0.14.6) - tilt (1.3.7) + tilt (1.4.1) + tins (1.6.0) treetop (1.4.12) polyglot polyglot (>= 0.3.1) tzinfo (0.3.37) + unf (0.1.4) + unf_ext + unf_ext (0.0.7.1) PLATFORMS ruby @@ -104,8 +136,10 @@ PLATFORMS DEPENDENCIES appraisal bundler (~> 1.3) + connection_pool + coveralls rack-test (>= 0.5.7) rails (~> 3.0.20) rake - rspec (~> 2.12) + rspec (~> 2.14) split! diff --git a/gemfiles/3.1.gemfile b/gemfiles/3.1.gemfile index 6fcbecfe..86fb2f04 100644 --- a/gemfiles/3.1.gemfile +++ b/gemfiles/3.1.gemfile @@ -4,6 +4,7 @@ source "https://rubygems.org" gem "appraisal" gem "rails", "~> 3.1.12" -gem "split", :path=>"../" +gem "split", :path => "../../" +gem "connection_pool" -gemspec :path=>"../" \ No newline at end of file +gemspec :path => "../" diff --git a/gemfiles/3.2.gemfile b/gemfiles/3.2.gemfile index 54efc9da..6c7c20ad 100644 --- a/gemfiles/3.2.gemfile +++ b/gemfiles/3.2.gemfile @@ -4,6 +4,7 @@ source "https://rubygems.org" gem "appraisal" gem "rails", "~> 3.2.13" -gem "split", :path=>"../" +gem "split", :path => "../../" +gem "connection_pool" -gemspec :path=>"../" \ No newline at end of file +gemspec :path => "../" diff --git a/gemfiles/4.0.gemfile b/gemfiles/4.0.gemfile index f901bbd9..f0c5d2c9 100644 --- a/gemfiles/4.0.gemfile +++ b/gemfiles/4.0.gemfile @@ -4,6 +4,7 @@ source "https://rubygems.org" gem "appraisal" gem "rails", "4.0.0.rc1" -gem "split", :path=>"../" +gem "split", :path => "../../" +gem "connection_pool" -gemspec :path=>"../" \ No newline at end of file +gemspec :path => "../" diff --git a/lib/split.rb b/lib/split.rb index aaa14f23..1309fe21 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -32,16 +32,19 @@ module Split def redis=(server) if server.respond_to? :split if server["redis://"] - redis = Redis.connect(:url => server, :thread_safe => true) + redis = Redis.connect(:url => server, :thread_safe => true) else server, namespace = server.split('/', 2) host, port, db = server.split(':') redis = Redis.new(:host => host, :port => port, - :thread_safe => true, :db => db) + :thread_safe => true, :db => db) + end namespace ||= :split - @redis = Redis::Namespace.new(namespace, :redis => redis) + @redis = ConnectionPool.new(size: 10, timeout: 5) { + Redis::Namespace.new(namespace, :redis => redis) + } elsif server.respond_to? :namespace= @redis = server else diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index fd3362da..6d007281 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -30,16 +30,22 @@ def goals end def participant_count - Split.redis.hget(key, 'participant_count').to_i + Split.redis.with do |conn| + conn.hget(key, 'participant_count').to_i + end end def participant_count=(count) - Split.redis.hset(key, 'participant_count', count.to_i) + Split.redis.with do |conn| + conn.hset(key, 'participant_count', count.to_i) + end end def completed_count(goal = nil) field = set_field(goal) - Split.redis.hget(key, field).to_i + Split.redis.with do |conn| + conn.hget(key, field).to_i + end end def all_completed_count @@ -64,16 +70,22 @@ def set_field(goal) def set_completed_count (count, goal = nil) field = set_field(goal) - Split.redis.hset(key, field, count.to_i) + Split.redis.with do |conn| + conn.hset(key, field, count.to_i) + end end def increment_participation - Split.redis.hincrby key, 'participant_count', 1 + Split.redis.with do |conn| + conn.hincrby key, 'participant_count', 1 + end end def increment_completion(goal = nil) field = set_field(goal) - Split.redis.hincrby(key, field, 1) + Split.redis.with do |conn| + conn.hincrby(key, field, 1) + end end def control? @@ -110,8 +122,10 @@ def z_score(goal = nil) end def save - Split.redis.hsetnx key, 'participant_count', 0 - Split.redis.hsetnx key, 'completed_count', 0 + Split.redis.with do |conn| + conn.hsetnx key, 'participant_count', 0 + conn.hsetnx key, 'completed_count', 0 + end end def validate! @@ -121,17 +135,21 @@ def validate! end def reset - Split.redis.hmset key, 'participant_count', 0, 'completed_count', 0 - unless goals.empty? - goals.each do |g| - field = "completed_count:#{g}" - Split.redis.hset key, field, 0 + Split.redis.with do |conn| + conn.hmset key, 'participant_count', 0, 'completed_count', 0 + unless goals.empty? + goals.each do |g| + field = "completed_count:#{g}" + conn.hset key, field, 0 + end end end end def delete - Split.redis.del(key) + Split.redis.with do |conn| + conn.del(key) + end end private diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 85acc591..fc148649 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -72,27 +72,30 @@ def self.find_or_create(label, *alternatives) def save validate! - - if new_record? - Split.redis.sadd(:experiments, name) - start unless Split.configuration.start_manually - @alternatives.reverse.each {|a| Split.redis.lpush(name, a.name)} - @goals.reverse.each {|a| Split.redis.lpush(goals_key, a)} unless @goals.nil? - else - existing_alternatives = load_alternatives_from_redis - existing_goals = load_goals_from_redis - unless existing_alternatives == @alternatives.map(&:name) && existing_goals == @goals - reset - @alternatives.each(&:delete) - delete_goals - Split.redis.del(@name) - @alternatives.reverse.each {|a| Split.redis.lpush(name, a.name)} - @goals.reverse.each {|a| Split.redis.lpush(goals_key, a)} unless @goals.nil? + Split.redis.with do |conn| + conn.pipelined do + if new_record? + conn.sadd(:experiments, name) + start unless Split.configuration.start_manually + @alternatives.reverse.each {|a| conn.lpush(name, a.name)} + @goals.reverse.each {|a| conn.lpush(goals_key, a)} unless @goals.nil? + else + existing_alternatives = load_alternatives_from_redis + existing_goals = load_goals_from_redis + unless existing_alternatives == @alternatives.map(&:name) && existing_goals == @goals + reset + @alternatives.each(&:delete) + delete_goals + conn.del(@name) + @alternatives.reverse.each {|a| conn.lpush(name, a.name)} + @goals.reverse.each {|a| conn.lpush(goals_key, a)} unless @goals.nil? + end + end + + conn.hset(experiment_config_key, :resettable, resettable) + conn.hset(experiment_config_key, :algorithm, algorithm.to_s) end end - - Split.redis.hset(experiment_config_key, :resettable, resettable) - Split.redis.hset(experiment_config_key, :algorithm, algorithm.to_s) self end @@ -107,7 +110,9 @@ def validate! end def new_record? - !Split.redis.exists(name) + Split.redis.with do |conn| + !conn.exists(name) + end end def ==(obj) @@ -141,10 +146,12 @@ def alternatives=(alts) end def winner - if w = Split.redis.hget(:experiment_winner, name) - Split::Alternative.new(w, name) - else - nil + Split.redis.with do |conn| + if w = conn.hget(:experiment_winner, name) + Split::Alternative.new(w, name) + else + nil + end end end @@ -153,7 +160,9 @@ def has_winner? end def winner=(winner_name) - Split.redis.hset(:experiment_winner, name, winner_name.to_s) + Split.redis.with do |conn| + conn.hset(:experiment_winner, name, winner_name.to_s) + end end def participant_count @@ -165,21 +174,27 @@ def control end def reset_winner - Split.redis.hdel(:experiment_winner, name) + Split.redis.with do |conn| + conn.hdel(:experiment_winner, name) + end end def start - Split.redis.hset(:experiment_start_times, @name, Time.now.to_i) + Split.redis.with do |conn| + conn.hset(:experiment_start_times, @name, Time.now.to_i) + end end def start_time - t = Split.redis.hget(:experiment_start_times, @name) - if t - # Check if stored time is an integer - if t =~ /^[-+]?[0-9]+$/ - t = Time.at(t.to_i) - else - t = Time.parse(t) + Split.redis.with do |conn| + t = conn.hget(:experiment_start_times, @name) + if t + # Check if stored time is an integer + if t =~ /^[-+]?[0-9]+$/ + t = Time.at(t.to_i) + else + t = Time.parse(t) + end end end end @@ -197,11 +212,15 @@ def random_alternative end def version - @version ||= (Split.redis.get("#{name.to_s}:version").to_i || 0) + Split.redis.with do |conn| + @version ||= (conn.get("#{name.to_s}:version").to_i || 0) + end end def increment_version - @version = Split.redis.incr("#{name}:version") + Split.redis.with do |conn| + @version = conn.incr("#{name}:version") + end end def key @@ -232,25 +251,33 @@ def reset end def delete - alternatives.each(&:delete) - reset_winner - Split.redis.srem(:experiments, name) - Split.redis.del(name) - delete_goals - Split.configuration.on_experiment_delete.call(self) - increment_version + Split.redis.with do |conn| + conn.pipelined do + alternatives.each(&:delete) + reset_winner + conn.srem(:experiments, name) + conn.del(name) + delete_goals + Split.configuration.on_experiment_delete.call(self) + increment_version + end + end end def delete_goals - Split.redis.del(goals_key) + Split.redis.with do |conn| + conn.del(goals_key) + end end def load_from_redis - exp_config = Split.redis.hgetall(experiment_config_key) - self.resettable = exp_config['resettable'] - self.algorithm = exp_config['algorithm'] - self.alternatives = load_alternatives_from_redis - self.goals = load_goals_from_redis + Split.redis.with do |conn| + exp_config = conn.hgetall(experiment_config_key) + self.resettable = exp_config['resettable'] + self.algorithm = exp_config['algorithm'] + self.alternatives = load_alternatives_from_redis + self.goals = load_goals_from_redis + end end protected @@ -269,7 +296,9 @@ def load_goals_from_configuration end def load_goals_from_redis - Split.redis.lrange(goals_key, 0, -1) + Split.redis.with do |conn| + conn.lrange(goals_key, 0, -1) + end end def load_alternatives_from_configuration @@ -283,16 +312,19 @@ def load_alternatives_from_configuration end def load_alternatives_from_redis - case Split.redis.type(@name) - when 'set' # convert legacy sets to lists - alts = Split.redis.smembers(@name) - Split.redis.del(@name) - alts.reverse.each {|a| Split.redis.lpush(@name, a) } - Split.redis.lrange(@name, 0, -1) - else - Split.redis.lrange(@name, 0, -1) + Split.redis.with do |conn| + conn.pipelined do + case conn.type(@name) + when 'set' # convert legacy sets to lists + alts = conn.smembers(@name) + conn.del(@name) + alts.reverse.each {|a| conn.lpush(@name, a) } + conn.lrange(@name, 0, -1) + else + conn.lrange(@name, 0, -1) + end + end end end - end end diff --git a/lib/split/experiment_catalog.rb b/lib/split/experiment_catalog.rb index 92fc3fc8..1545d10a 100644 --- a/lib/split/experiment_catalog.rb +++ b/lib/split/experiment_catalog.rb @@ -3,7 +3,9 @@ class ExperimentCatalog # Return all experiments def self.all # Call compact to prevent nil experiments from being returned -- seems to happen during gem upgrades - Split.redis.smembers(:experiments).map {|e| find(e)}.compact + Split.redis.with do |conn| + conn.smembers(:experiments).map {|e| find(e)}.compact + end end # Return experiments without a winner (considered "active") first @@ -12,11 +14,12 @@ def self.all_active_first end def self.find(name) - if Split.redis.exists(name) - obj = Experiment.new name - obj.load_from_redis - else - obj = nil + obj = nil + Split.redis.with do |conn| + if conn.exists(name) + obj = Experiment.new name + obj.load_from_redis + end end obj end diff --git a/lib/split/metric.rb b/lib/split/metric.rb index 2a3211e4..e7c2734c 100644 --- a/lib/split/metric.rb +++ b/lib/split/metric.rb @@ -12,17 +12,19 @@ def initialize(attrs = {}) end def self.load_from_redis(name) - metric = Split.redis.hget(:metrics, name) - if metric - experiment_names = metric.split(',') + Split.redis.with do |conn| + metric = conn.hget(:metrics, name) + if metric + experiment_names = metric.split(',') - experiments = experiment_names.collect do |experiment_name| - Split::Experiment.find(experiment_name) - end + experiments = experiment_names.collect do |experiment_name| + Split::Experiment.find(experiment_name) + end - Split::Metric.new(:name => name, :experiments => experiments) - else - nil + Split::Metric.new(:name => name, :experiments => experiments) + else + nil + end end end @@ -52,8 +54,10 @@ def self.find_or_create(attrs) end def self.all - redis_metrics = Split.redis.hgetall(:metrics).collect do |key, value| - find(key) + Split.redis.with do |conn| + redis_metrics = conn.hgetall(:metrics).collect do |key, value| + find(key) + end end configuration_metrics = Split.configuration.metrics.collect do |key, value| new(name: key, experiments: value) @@ -75,7 +79,9 @@ def self.possible_experiments(metric_name) end def save - Split.redis.hset(:metrics, name, experiments.map(&:name).join(',')) + Split.redis.with do |conn| + conn.hset(:metrics, name, experiments.map(&:name).join(',')) + end end def complete! diff --git a/lib/split/persistence/redis_adapter.rb b/lib/split/persistence/redis_adapter.rb index 7cab544d..42bfc924 100644 --- a/lib/split/persistence/redis_adapter.rb +++ b/lib/split/persistence/redis_adapter.rb @@ -19,19 +19,27 @@ def initialize(context) end def [](field) - Split.redis.hget(redis_key, field) + Split.redis.with do |conn| + conn.hget(redis_key, field) + end end def []=(field, value) - Split.redis.hset(redis_key, field, value) + Split.redis.with do |conn| + conn.hset(redis_key, field, value) + end end def delete(field) - Split.redis.hdel(redis_key, field) + Split.redis.with do |conn| + conn.hdel(redis_key, field) + end end def keys - Split.redis.hkeys(redis_key) + Split.redis.with do |conn| + conn.hkeys(redis_key) + end end def self.with_config(options={}) diff --git a/spec/alternative_spec.rb b/spec/alternative_spec.rb index a9614a83..cb20e362 100644 --- a/spec/alternative_spec.rb +++ b/spec/alternative_spec.rb @@ -125,7 +125,9 @@ it "should save to redis" do alternative.save - Split.redis.exists('basket_text:Basket').should be true + Split.redis.with do |conn| + conn.exists('basket_text:Basket').should be true + end end it "should increment participation count" do diff --git a/spec/dashboard_spec.rb b/spec/dashboard_spec.rb index b22fa7e4..2ad32437 100644 --- a/spec/dashboard_spec.rb +++ b/spec/dashboard_spec.rb @@ -163,8 +163,9 @@ def link(color) it "should handle experiments without a start date" do experiment_start_time = Time.parse('2011-07-07') Time.stub(:now => experiment_start_time) - Split.redis.hdel(:experiment_start_times, experiment.name) - + Split.redis.with do |conn| + conn.hdel(:experiment_start_times, experiment.name) + end get '/' last_response.body.should include('Unknown') diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index ee43496f..a75ccd9b 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -40,7 +40,9 @@ def alternative(color) it "should save to redis" do experiment.save - Split.redis.exists('basket_text').should be true + Split.redis.with do |conn| + conn.exists('basket_text').should be true + end end it "should save the start time to redis" do @@ -70,8 +72,9 @@ def alternative(color) experiment_start_time = Time.parse("Sat Mar 03 14:01:03") Time.stub(:now => experiment_start_time) experiment.save - Split.redis.hset(:experiment_start_times, experiment.name, experiment_start_time) - + Split.redis.with do |conn| + conn.hset(:experiment_start_times, experiment.name, experiment_start_time) + end Split::Experiment.find('basket_text').start_time.should == experiment_start_time end @@ -79,17 +82,19 @@ def alternative(color) experiment_start_time = Time.parse("Sat Mar 03 14:01:03") Time.stub(:now => experiment_start_time) experiment.save - - Split.redis.hdel(:experiment_start_times, experiment.name) - + Split.redis.with do |conn| + conn.hdel(:experiment_start_times, experiment.name) + end Split::Experiment.find('basket_text').start_time.should == nil end it "should not create duplicates when saving multiple times" do experiment.save experiment.save - Split.redis.exists('basket_text').should be true - Split.redis.lrange('basket_text', 0, -1).should eql(['Basket', "Cart"]) + Split.redis.with do |conn| + conn.exists('basket_text').should be true + conn.lrange('basket_text', 0, -1).should eql(['Basket', "Cart"]) + end end describe 'new record?' do @@ -172,7 +177,9 @@ def alternative(color) experiment.save experiment.delete - Split.redis.exists('link_color').should be false + Split.redis.with do |conn| + conn.exists('link_color').should be false + end Split::Experiment.find('link_color').should be_nil end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 02a049ac..9d28207e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -17,7 +17,9 @@ config.order = 'random' config.before(:each) do Split.configuration = Split::Configuration.new - Split.redis.flushall + Split.redis.with do |conn| + conn.flushall + end @ab_user = {} params = nil end diff --git a/split.gemspec b/split.gemspec index 83ee3823..1418a321 100644 --- a/split.gemspec +++ b/split.gemspec @@ -22,6 +22,7 @@ Gem::Specification.new do |s| s.add_dependency 'redis-namespace', '>= 1.1.0' s.add_dependency 'sinatra', '>= 1.2.6' s.add_dependency 'simple-random' + s.add_dependency 'connection_pool' # Ruby 1.8 doesn't include JSON in the std lib if RUBY_VERSION < "1.9" From a423d7ac8b3a894ab4dea56aa65f4a15cb2abbd0 Mon Sep 17 00:00:00 2001 From: Marc Cull Date: Wed, 30 Sep 2015 15:41:14 -0700 Subject: [PATCH 03/22] connection pool --- lib/split.rb | 26 +++++++++++-------- lib/split/experiment.rb | 56 ++++++++++++++++++++--------------------- 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/lib/split.rb b/lib/split.rb index 1309fe21..d398a6da 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -17,6 +17,7 @@ require 'split/engine' if defined?(Rails) && Rails::VERSION::MAJOR >= 3 require 'redis/namespace' +require 'connection_pool' module Split extend self @@ -30,25 +31,28 @@ module Split # 5. An instance of `Redis`, `Redis::Client`, `Redis::DistRedis`, # or `Redis::Namespace`. def redis=(server) + if server.respond_to? :split if server["redis://"] - redis = Redis.connect(:url => server, :thread_safe => true) + namespace ||= :split + @redis = ConnectionPool.new(size: 10, timeout: 5) { + Redis::Namespace.new(namespace, :redis => Redis.connect(:url => server, :thread_safe => true)) + } else server, namespace = server.split('/', 2) + namespace ||= :split host, port, db = server.split(':') - redis = Redis.new(:host => host, :port => port, - :thread_safe => true, :db => db) - + @redis = ConnectionPool.new(size: 10, timeout: 5) { + Redis::Namespace.new(namespace, :redis => Redis.new(:host => host, :port => port, + :thread_safe => true, :db => db)) + } end - namespace ||= :split - - @redis = ConnectionPool.new(size: 10, timeout: 5) { - Redis::Namespace.new(namespace, :redis => redis) - } elsif server.respond_to? :namespace= - @redis = server + @redis = ConnectionPool.new(size: 10, timeout: 5) { server } else - @redis = Redis::Namespace.new(:split, :redis => server) + @redis = ConnectionPool.new(size: 10, timeout: 5) { + Redis::Namespace.new(:split, :redis => server) + } end end diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index fc148649..27d4873f 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -73,23 +73,23 @@ def self.find_or_create(label, *alternatives) def save validate! Split.redis.with do |conn| - conn.pipelined do - if new_record? - conn.sadd(:experiments, name) - start unless Split.configuration.start_manually + if new_record? + conn.sadd(:experiments, name) + start unless Split.configuration.start_manually + @alternatives.reverse.each {|a| conn.lpush(name, a.name) } + @goals.reverse.each {|a| conn.lpush(goals_key, a) } unless @goals.nil? + else + existing_alternatives = load_alternatives_from_redis + existing_goals = load_goals_from_redis + unless existing_alternatives == @alternatives.map(&:name) && existing_goals == @goals + reset + @alternatives.each(&:delete) + delete_goals + conn.del(@name) + @alternatives.reverse.each {|a| conn.lpush(name, a.name)} - @goals.reverse.each {|a| conn.lpush(goals_key, a)} unless @goals.nil? - else - existing_alternatives = load_alternatives_from_redis - existing_goals = load_goals_from_redis - unless existing_alternatives == @alternatives.map(&:name) && existing_goals == @goals - reset - @alternatives.each(&:delete) - delete_goals - conn.del(@name) - @alternatives.reverse.each {|a| conn.lpush(name, a.name)} - @goals.reverse.each {|a| conn.lpush(goals_key, a)} unless @goals.nil? - end + + @goals.reverse.each {|a| conn.lpush(goals_key, a) } unless @goals.nil? end conn.hset(experiment_config_key, :resettable, resettable) @@ -252,7 +252,7 @@ def reset def delete Split.redis.with do |conn| - conn.pipelined do + #conn.pipelined do alternatives.each(&:delete) reset_winner conn.srem(:experiments, name) @@ -260,7 +260,7 @@ def delete delete_goals Split.configuration.on_experiment_delete.call(self) increment_version - end + #end end end @@ -313,18 +313,18 @@ def load_alternatives_from_configuration def load_alternatives_from_redis Split.redis.with do |conn| - conn.pipelined do - case conn.type(@name) - when 'set' # convert legacy sets to lists - alts = conn.smembers(@name) - conn.del(@name) - alts.reverse.each {|a| conn.lpush(@name, a) } - conn.lrange(@name, 0, -1) - else - conn.lrange(@name, 0, -1) - end + + case conn.type(@name) + when 'set' # convert legacy sets to lists + alts = conn.smembers(@name) + conn.del(@name) + alts.reverse.each {|a| conn.lpush(@name, a) } + conn.lrange(@name, 0, -1) + else + conn.lrange(@name, 0, -1) end end end + end end From 7af17295c5e9d87bdf7b96c7a15e03f69aa91f27 Mon Sep 17 00:00:00 2001 From: Marc Cull Date: Wed, 30 Sep 2015 15:53:34 -0700 Subject: [PATCH 04/22] fixing tests --- lib/split/experiment.rb | 5 ++--- lib/split/metric.rb | 9 +++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 27d4873f..479f706e 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -91,10 +91,9 @@ def save @goals.reverse.each {|a| conn.lpush(goals_key, a) } unless @goals.nil? end - - conn.hset(experiment_config_key, :resettable, resettable) - conn.hset(experiment_config_key, :algorithm, algorithm.to_s) end + conn.hset(experiment_config_key, :resettable, resettable) + conn.hset(experiment_config_key, :algorithm, algorithm.to_s) end self end diff --git a/lib/split/metric.rb b/lib/split/metric.rb index e7c2734c..05e52aeb 100644 --- a/lib/split/metric.rb +++ b/lib/split/metric.rb @@ -58,11 +58,12 @@ def self.all redis_metrics = conn.hgetall(:metrics).collect do |key, value| find(key) end + + configuration_metrics = Split.configuration.metrics.collect do |key, value| + new(name: key, experiments: value) + end + redis_metrics | configuration_metrics end - configuration_metrics = Split.configuration.metrics.collect do |key, value| - new(name: key, experiments: value) - end - redis_metrics | configuration_metrics end def self.possible_experiments(metric_name) From c6622eedb7a74cc86024b784c91e7023e9d6cc97 Mon Sep 17 00:00:00 2001 From: Marc Cull Date: Mon, 5 Oct 2015 11:39:18 -0700 Subject: [PATCH 05/22] guard alternative scoring against bad data, like negative values. --- lib/split/alternative.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index 6d007281..d46bd9d7 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -117,8 +117,12 @@ def z_score(goal = nil) n_a = alternative.participant_count n_c = control.participant_count - - z_score = Split::Zscore.calculate(p_a, n_a, p_c, n_c) + + begin + z_score = Split::Zscore.calculate(p_a, n_a, p_c, n_c) + rescue + return 'N/A' + end end def save From 15bf8fb820c1d7ace7d772ca5011b6d397332b6d Mon Sep 17 00:00:00 2001 From: Wen Shaw Date: Mon, 29 Feb 2016 12:02:49 -0800 Subject: [PATCH 06/22] make alternative.completed_count always return an integer --- lib/split/alternative.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index d46bd9d7..028f949d 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -43,9 +43,11 @@ def participant_count=(count) def completed_count(goal = nil) field = set_field(goal) - Split.redis.with do |conn| + ret = Split.redis.with do |conn| conn.hget(key, field).to_i end + # the return value should always be an integer + return (ret == [] ? 0 : ret ) end def all_completed_count From b8df2028892a7b6288f9c49c860249daedd2f031 Mon Sep 17 00:00:00 2001 From: Wen Shaw Date: Thu, 6 Apr 2017 14:00:06 -0700 Subject: [PATCH 07/22] removed rescue around Split::Zscore.calculate --- lib/split/alternative.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index 3a76922d..7fceb4f6 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -217,11 +217,7 @@ def z_score(goal = nil) n_a = alternative.participant_count n_c = control.participant_count - begin - z_score = Split::Zscore.calculate(p_a, n_a, p_c, n_c) - rescue - return 'N/A' - end + z_score = Split::Zscore.calculate(p_a, n_a, p_c, n_c) end def log_normal_probability_better_than_control(goal = nil) From 5515162028dc8abbc908944e1525efe90cde68b8 Mon Sep 17 00:00:00 2001 From: Wen Shaw Date: Wed, 19 Apr 2017 17:23:35 -0700 Subject: [PATCH 08/22] require simple-random --- lib/split/alternative.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index 7fceb4f6..e10bc208 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -1,5 +1,6 @@ require 'split/zscore' require 'active_support/all' +require 'simple-random' # TODO - take out require and implement using file paths? module Split From 052dde34c4871b88f3605e3b0d07fd41db893382 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Wed, 5 Jul 2017 18:41:57 -0700 Subject: [PATCH 09/22] 1. don't think we need to keep track of unique vs non-unique values; don't add on unique completion method 2. we should "flatten" values once winner is selected so we don't keep the entire list of values, just the average (caveat: the statistical significance calculations are no longer possible without the distribution) --- lib/split/alternative.rb | 32 +++++++++++++++++++++++++++++--- lib/split/experiment.rb | 1 + 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index e10bc208..a471475e 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -176,9 +176,6 @@ def increment_completion(goal = nil, value = nil) def increment_unique_completion(goal = nil, value = nil) Split.redis.with do |conn| field = set_field(goal, true) - if value - conn.lpush(key + set_value_field(goal, true), value) - end conn.hincrby(key, field, 1) end end @@ -468,6 +465,35 @@ def reset def delete Split.redis.with do |conn| conn.del(key) + unless goals.empty? + goals.each do |g| + field = "completed_count:#{g}" + value_field = set_value_field(g) + conn.del(key + value_field) + conn.del(key + field) + + field = "unique_completed_count:#{g}" + value_field = set_value_field(g, true) + conn.del(key + value_field) + conn.del(key + field) + end + end + end + end + + def flatten_values + Split.redis.with do |conn| + unless goals.empty? + goals.each do |g| + value_field = set_value_field(g) + avg = completed_value(g) + conn.del(key + value_field) + conn.lpush(key + value_field, avg) + + value_field = set_value_field(g, true) + conn.del(key + value_field) + end + end end end diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 066fc4b7..8d0c75f6 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -217,6 +217,7 @@ def winner=(winner_name) set_end_time delete_participants delete_finished + alternatives.each(&:flatten_values) end def participant_count From f344a4f7c2c59742b4b9e2b02e2f86781faffb9d Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 14:29:59 -0700 Subject: [PATCH 10/22] when deleting massive lists like participants and finished, use a garbage collection methodology: 1. rename the key to something deleted. 2. trim the list until it's empty. 3. delete it. steps 2+3 we could do in a background job instead... --- lib/split/experiment.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 8d0c75f6..0295e6b2 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -507,7 +507,7 @@ def delete def delete_participants Split.redis.with do |conn| goals.each do |goal| - conn.del("#{self.key}:participants") + conn.rename("#{self.key}:participants", "gc:lists:#{conn.incr("gc:index")}") end end end @@ -517,9 +517,9 @@ def delete_finished key = "#{self.key}:finished" conn.del(key) (goals).each do |goal| - conn.del("#{key}:#{goal}") + conn.rename("#{key}:#{goal}", "gc:lists:#{conn.incr("gc:index")}") end - conn.del("#{key}") + conn.rename("#{key}", "gc:lists:#{conn.incr("gc:index")}") end end From b82ca95c6a34dc1a89ae9badfd3b6efc2386c78c Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 14:39:34 -0700 Subject: [PATCH 11/22] trigger garbage collection callback on productssite --- lib/split/experiment.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 0295e6b2..7e9cbf39 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -507,7 +507,9 @@ def delete def delete_participants Split.redis.with do |conn| goals.each do |goal| - conn.rename("#{self.key}:participants", "gc:lists:#{conn.incr("gc:index")}") + new_key = "gc:lists:#{conn.incr("gc:index")}" + conn.rename("#{self.key}:participants", ) + Split.configuration.on_garbage_collection.call(new_key) end end end @@ -517,9 +519,13 @@ def delete_finished key = "#{self.key}:finished" conn.del(key) (goals).each do |goal| - conn.rename("#{key}:#{goal}", "gc:lists:#{conn.incr("gc:index")}") + new_key = "gc:lists:#{conn.incr("gc:index")}" + conn.rename("#{key}:#{goal}", new_key) + Split.configuration.on_garbage_collection.call(new_key) end - conn.rename("#{key}", "gc:lists:#{conn.incr("gc:index")}") + new_key = "gc:lists:#{conn.incr("gc:index")}" + conn.rename("#{key}", new_key) + Split.configuration.on_garbage_collection.call(new_key) end end From c3421311edf5c5faedbec6d0a96d76c419cdea4a Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 14:42:04 -0700 Subject: [PATCH 12/22] bug fix --- lib/split/configuration.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index 507bd91a..d07742f1 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -17,6 +17,7 @@ class Configuration attr_accessor :on_experiment_delete attr_accessor :on_experiment_max_out attr_accessor :on_experiment_end + attr_accessor :on_garbage_collection attr_accessor :include_rails_helper attr_accessor :pipeline_size @@ -204,6 +205,7 @@ def initialize @on_experiment_max_out = proc{|experiment|} @on_experiment_delete = proc{|experiment|} @on_experiment_end = proc{|experiment|} + @on_garbage_collection = proc{|experiment|} @db_failover_allow_parameter_override = false @enabled = true @experiments = {} From fdac8f725d127911dc858cbed2370be5c71c172e Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 14:56:18 -0700 Subject: [PATCH 13/22] bug fix --- lib/split/experiment.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 7e9cbf39..a537cb80 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -517,15 +517,14 @@ def delete_participants def delete_finished Split.redis.with do |conn| key = "#{self.key}:finished" - conn.del(key) + new_key = "gc:lists:#{conn.incr("gc:index")}" + conn.rename(key, new_key) + Split.configuration.on_garbage_collection.call(new_key) (goals).each do |goal| new_key = "gc:lists:#{conn.incr("gc:index")}" conn.rename("#{key}:#{goal}", new_key) Split.configuration.on_garbage_collection.call(new_key) end - new_key = "gc:lists:#{conn.incr("gc:index")}" - conn.rename("#{key}", new_key) - Split.configuration.on_garbage_collection.call(new_key) end end From b52d0fcf3f72ac3b4ece1420127e1cff234085a0 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 15:09:26 -0700 Subject: [PATCH 14/22] bug fix --- lib/split/experiment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index a537cb80..b7f74144 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -508,7 +508,7 @@ def delete_participants Split.redis.with do |conn| goals.each do |goal| new_key = "gc:lists:#{conn.incr("gc:index")}" - conn.rename("#{self.key}:participants", ) + conn.rename("#{self.key}:participants", new_key) Split.configuration.on_garbage_collection.call(new_key) end end From f1459639e46155e2fbd9ade9fd95fd61661c2183 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 15:10:51 -0700 Subject: [PATCH 15/22] don't need to loop goals to delete participants --- lib/split/experiment.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index b7f74144..61258000 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -506,11 +506,9 @@ def delete def delete_participants Split.redis.with do |conn| - goals.each do |goal| - new_key = "gc:lists:#{conn.incr("gc:index")}" - conn.rename("#{self.key}:participants", new_key) - Split.configuration.on_garbage_collection.call(new_key) - end + new_key = "gc:lists:#{conn.incr("gc:index")}" + conn.rename("#{self.key}:participants", new_key) + Split.configuration.on_garbage_collection.call(new_key) end end From 69fcdda41aea971a65cd47c2491ed303193805cf Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 15:17:22 -0700 Subject: [PATCH 16/22] they're actually sets not lists --- lib/split/experiment.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 61258000..b274ba9e 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -506,7 +506,7 @@ def delete def delete_participants Split.redis.with do |conn| - new_key = "gc:lists:#{conn.incr("gc:index")}" + new_key = "gc:sets:#{conn.incr("gc:index")}" conn.rename("#{self.key}:participants", new_key) Split.configuration.on_garbage_collection.call(new_key) end @@ -515,11 +515,11 @@ def delete_participants def delete_finished Split.redis.with do |conn| key = "#{self.key}:finished" - new_key = "gc:lists:#{conn.incr("gc:index")}" + new_key = "gc:sets:#{conn.incr("gc:index")}" conn.rename(key, new_key) Split.configuration.on_garbage_collection.call(new_key) (goals).each do |goal| - new_key = "gc:lists:#{conn.incr("gc:index")}" + new_key = "gc:sets:#{conn.incr("gc:index")}" conn.rename("#{key}:#{goal}", new_key) Split.configuration.on_garbage_collection.call(new_key) end From 28b6000ce11da034a1424fb7b00e658355d9b9b6 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 16:30:56 -0700 Subject: [PATCH 17/22] DRY --- lib/split/experiment.rb | 12 +++--------- lib/split/persistence/redis_adapter.rb | 8 ++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index b274ba9e..80287e82 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -506,22 +506,16 @@ def delete def delete_participants Split.redis.with do |conn| - new_key = "gc:sets:#{conn.incr("gc:index")}" - conn.rename("#{self.key}:participants", new_key) - Split.configuration.on_garbage_collection.call(new_key) + Split::Persistence.adapter.garbage_collect("sets", "#{self.key}:participants") end end def delete_finished Split.redis.with do |conn| key = "#{self.key}:finished" - new_key = "gc:sets:#{conn.incr("gc:index")}" - conn.rename(key, new_key) - Split.configuration.on_garbage_collection.call(new_key) + Split::Persistence.adapter.garbage_collect("sets", key) (goals).each do |goal| - new_key = "gc:sets:#{conn.incr("gc:index")}" - conn.rename("#{key}:#{goal}", new_key) - Split.configuration.on_garbage_collection.call(new_key) + Split::Persistence.adapter.garbage_collect("sets", "#{key}:#{goal}") end end end diff --git a/lib/split/persistence/redis_adapter.rb b/lib/split/persistence/redis_adapter.rb index fc15a7d3..6f456284 100644 --- a/lib/split/persistence/redis_adapter.rb +++ b/lib/split/persistence/redis_adapter.rb @@ -81,6 +81,14 @@ def self.fetch_values_in_batch(split_ids, field) def self.get_redis_key(key_frag) "#{config[:namespace]}:#{key_frag}" end + + def self.garbage_collect(type, key) + Split.redis.with do |conn| + new_key = "gc:#{type}:#{conn.incr("gc:index")}" + conn.rename(key, new_key) + Split.configuration.on_garbage_collection.call(new_key) + end + end def get_redis_key(key_frag) self.class.get_redis_key(key_frag) From 471bdd9081550cb886cc86d2e6fe8e4742e552d5 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 18:12:15 -0700 Subject: [PATCH 18/22] it's cool if the key doesn't exist --- lib/split/persistence/redis_adapter.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/split/persistence/redis_adapter.rb b/lib/split/persistence/redis_adapter.rb index 6f456284..d3ce4769 100644 --- a/lib/split/persistence/redis_adapter.rb +++ b/lib/split/persistence/redis_adapter.rb @@ -84,9 +84,13 @@ def self.get_redis_key(key_frag) def self.garbage_collect(type, key) Split.redis.with do |conn| - new_key = "gc:#{type}:#{conn.incr("gc:index")}" - conn.rename(key, new_key) - Split.configuration.on_garbage_collection.call(new_key) + begin + new_key = "gc:#{type}:#{conn.incr("gc:index")}" + conn.rename(key, new_key) + Split.configuration.on_garbage_collection.call(new_key) + rescue Redis::CommandError => e + raise e unless e.message.includes?("no such key") + end end end From dc1429cebb254980398cedafb14dec6ecde55e90 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Fri, 29 Sep 2017 18:20:12 -0700 Subject: [PATCH 19/22] fix typo --- lib/split/persistence/redis_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/split/persistence/redis_adapter.rb b/lib/split/persistence/redis_adapter.rb index d3ce4769..8c530f8d 100644 --- a/lib/split/persistence/redis_adapter.rb +++ b/lib/split/persistence/redis_adapter.rb @@ -89,7 +89,7 @@ def self.garbage_collect(type, key) conn.rename(key, new_key) Split.configuration.on_garbage_collection.call(new_key) rescue Redis::CommandError => e - raise e unless e.message.includes?("no such key") + raise e unless e.message.include?("no such key") end end end From 43aa4784d6fe98e499d5fee3d028c4bf29dc7100 Mon Sep 17 00:00:00 2001 From: Steven Ou Date: Thu, 12 Sep 2019 13:56:03 -0700 Subject: [PATCH 20/22] make split_id protected --- lib/split/helper.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/split/helper.rb b/lib/split/helper.rb index 9dab2e39..66780ee4 100644 --- a/lib/split/helper.rb +++ b/lib/split/helper.rb @@ -173,10 +173,6 @@ def ab_user @ab_user ||= Split::Persistence.adapter.new(self) end - def split_id - ab_user.key_frag - end - def exclude_visitor? instance_eval(&Split.configuration.ignore_filter) end @@ -196,6 +192,10 @@ def is_ignored_ip_address? protected + def split_id + ab_user.key_frag + end + def normalize_experiment(metric_descriptor) if Hash === metric_descriptor experiment_name = metric_descriptor.keys.first From d7a108d074c4bf0a45425d0377449e6f94f2ebbf Mon Sep 17 00:00:00 2001 From: Tyler Guillen Date: Thu, 18 Mar 2021 14:11:18 -0700 Subject: [PATCH 21/22] ruby 2.4 bigdecimal fixes --- lib/split/dashboard/helpers.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/split/dashboard/helpers.rb b/lib/split/dashboard/helpers.rb index 1153207d..dff5c50b 100644 --- a/lib/split/dashboard/helpers.rb +++ b/lib/split/dashboard/helpers.rb @@ -17,7 +17,9 @@ def number_to_currency(number) end def round(number, precision = 2) - BigDecimal.new(number.to_s).round(precision).to_f + BigDecimal(number.to_s).round(precision).to_f + rescue ArgumentError + number end def confidence_level(z_score) From 2e176e708c72043f014a348eb0c76779dea02b2d Mon Sep 17 00:00:00 2001 From: Tyler Guillen Date: Thu, 18 Mar 2021 14:14:44 -0700 Subject: [PATCH 22/22] cast to float --- lib/split/dashboard/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/split/dashboard/helpers.rb b/lib/split/dashboard/helpers.rb index dff5c50b..40b33a15 100644 --- a/lib/split/dashboard/helpers.rb +++ b/lib/split/dashboard/helpers.rb @@ -19,7 +19,7 @@ def number_to_currency(number) def round(number, precision = 2) BigDecimal(number.to_s).round(precision).to_f rescue ArgumentError - number + number.to_f end def confidence_level(z_score)