diff --git a/README.md b/README.md index a531721f..71d53139 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,19 @@ Split.configure do |config| end ``` +The recalculation is triggered while rendering the dashboard, which can make it +slow when many experiments are due for recalculation at once. On busy +installations you can turn the on-dashboard calculation off and instead run it +from a background job (e.g. `Split::ExperimentCatalog.all.each(&:calc_winning_alternatives)`): + +```ruby +Split.configure do |config| + config.dashboard_calculate_winning_alternatives = false +end +``` + +The dashboard will then display the most recently calculated probabilities. + ## Extras ### Weighted alternatives diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index b43e5920..caebb21c 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -29,6 +29,7 @@ class Configuration attr_accessor :winning_alternative_recalculation_interval attr_accessor :redis attr_accessor :dashboard_pagination_default_per_page + attr_accessor :dashboard_calculate_winning_alternatives attr_accessor :cache attr_reader :experiments @@ -234,6 +235,7 @@ def initialize @winning_alternative_recalculation_interval = 60 * 60 * 24 # 1 day @redis = ENV.fetch(ENV.fetch("REDIS_PROVIDER", "REDIS_URL"), "redis://localhost:6379") @dashboard_pagination_default_per_page = 10 + @dashboard_calculate_winning_alternatives = true end private diff --git a/lib/split/dashboard.rb b/lib/split/dashboard.rb index 48c3f1e7..3c0698de 100755 --- a/lib/split/dashboard.rb +++ b/lib/split/dashboard.rb @@ -19,9 +19,9 @@ class Dashboard < Sinatra::Base helpers Split::DashboardPaginationHelpers get "/" do - # Display experiments without a winner at the top of the dashboard - @experiments = Split::ExperimentCatalog.all_active_first - @unintialized_experiments = Split.configuration.experiments.keys - @experiments.map(&:name) + @experiment_names = Split::ExperimentCatalog.all_active_first_names + @experiments = Split::Experiment.find_many(paginated(@experiment_names)) + @unintialized_experiments = Split.configuration.experiments.keys - @experiment_names @metrics = Split::Metric.all diff --git a/lib/split/dashboard/views/_experiment.erb b/lib/split/dashboard/views/_experiment.erb index d23ac2b5..d3faebd0 100644 --- a/lib/split/dashboard/views/_experiment.erb +++ b/lib/split/dashboard/views/_experiment.erb @@ -4,7 +4,7 @@ <% experiment_class = "experiment" %> <% end %> -<% experiment.calc_winning_alternatives %> +<% experiment.calc_winning_alternatives if Split.configuration.dashboard_calculate_winning_alternatives %> <% extra_columns = [] experiment.alternatives.each do |alternative| diff --git a/lib/split/dashboard/views/index.erb b/lib/split/dashboard/views/index.erb index b34f5d60..77c71342 100644 --- a/lib/split/dashboard/views/index.erb +++ b/lib/split/dashboard/views/index.erb @@ -1,4 +1,4 @@ -<% if @experiments.any? %> +<% if @experiment_names.any? %>
The list below contains all the registered experiments along with the number of test participants, completed and conversion rate currently in the system.
No experiments have started yet, you need to define them in your code and introduce them to your users.
diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 042a2a4a..17d88e08 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -24,6 +24,18 @@ def self.find(name) end end + def self.find_many(names) + names = Array(names) + return [] if names.empty? + + data = ExperimentStorage::RedisStorage.load_many(names) + names.each_index.filter_map do |i| + experiment = new(names[i]) + experiment.set_alternatives_and_options(data[i]) + experiment unless experiment.alternatives.empty? + end + end + def initialize(name, options = {}) options = DEFAULT_OPTIONS.merge(options) diff --git a/lib/split/experiment_catalog.rb b/lib/split/experiment_catalog.rb index 7a8674cc..ae6b38e8 100644 --- a/lib/split/experiment_catalog.rb +++ b/lib/split/experiment_catalog.rb @@ -4,13 +4,25 @@ module Split 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::Experiment.find_many(experiment_names) end # Return experiments without a winner (considered "active") first def self.all_active_first - all.partition { |e| not e.winner }.map { |es| es.sort_by(&:name) }.flatten + Split::Experiment.find_many(all_active_first_names) + end + + def self.all_active_first_names + names = experiment_names + return names if names.empty? + + winners = Split.redis.hgetall(:experiment_winner) + active, finished = names.partition { |name| !winners.key?(name) } + active.sort + finished.sort + end + + def self.experiment_names + Split.redis.smembers(:experiments) end def self.find(name) diff --git a/lib/split/experiment_storage.rb b/lib/split/experiment_storage.rb index 382ad765..589819f5 100644 --- a/lib/split/experiment_storage.rb +++ b/lib/split/experiment_storage.rb @@ -88,13 +88,29 @@ def exists? redis.exists?(@name) end + def self.load_many(names) + storages = names.map { |name| new(name) } + reads = nil + Split.redis.pipelined do |pipe| + reads = storages.map { |storage| storage.pipeline_read(pipe) } + end + storages.zip(reads).map { |storage, futures| storage.build_from_raw(*futures.map(&:value)) } + end + def load! - raw_config, raw_alternatives, raw_metadata, raw_goals = redis.pipelined do |pipe| - pipe.hgetall(experiment_config_key) - pipe.lrange(@name, 0, -1) - pipe.get(metadata_key) + self.class.load_many([@name]).first + end + + def pipeline_read(pipe) + [ + pipe.hgetall(experiment_config_key), + pipe.lrange(@name, 0, -1), + pipe.get(metadata_key), pipe.lrange(goals_key, 0, -1) - end + ] + end + + def build_from_raw(raw_config, raw_alternatives, raw_metadata, raw_goals) config = raw_config.transform_keys(&:to_sym) { diff --git a/spec/dashboard_spec.rb b/spec/dashboard_spec.rb index d985ef3d..3d95c8a5 100644 --- a/spec/dashboard_spec.rb +++ b/spec/dashboard_spec.rb @@ -151,6 +151,21 @@ def link(color) expect(last_response.body).to_not include("Reopen Experiment") end end + + describe "winning alternative calculation" do + before { experiment } + + it "recalculates winning alternatives by default" do + expect_any_instance_of(Split::Experiment).to receive(:calc_winning_alternatives) + get "/" + end + + it "skips the calculation when dashboard_calculate_winning_alternatives is false" do + Split.configuration.dashboard_calculate_winning_alternatives = false + expect_any_instance_of(Split::Experiment).to_not receive(:calc_winning_alternatives) + get "/" + end + end end describe "reopen experiment" do