Enable tablesample count strategy by default

https://gitlab.com/gitlab-org/gitlab-ce/issues/58792

9 files changed, 12 insertions, 78 deletions

diff --git a/changelogs/unreleased/ab-count-strategies.yml b/changelogs/unreleased/ab-count-strategies.yml
new file mode 100644
index 00000000000..bd95ff45d6f
--- /dev/null
+++ b/changelogs/unreleased/ab-count-strategies.yml
@@ -0,0 +1,5 @@
+---
+title: Use tablesample approximate counting by default.
+merge_request: 31048
+author:
+type: performance
diff --git a/lib/gitlab/database/count.rb b/lib/gitlab/database/count.rb
index f3d37ccd72a..eac61254bdf 100644
--- a/lib/gitlab/database/count.rb
+++ b/lib/gitlab/database/count.rb
@@ -37,16 +37,14 @@ module Gitlab
       # @return [Hash] of Model -> count mapping
       def self.approximate_counts(models, strategies: [TablesampleCountStrategy, ReltuplesCountStrategy, ExactCountStrategy])
         strategies.each_with_object({}) do |strategy, counts_by_model|
-          if strategy.enabled?
-            models_with_missing_counts = models - counts_by_model.keys
+          models_with_missing_counts = models - counts_by_model.keys
 
-            break counts_by_model if models_with_missing_counts.empty?
+          break counts_by_model if models_with_missing_counts.empty?
 
-            counts = strategy.new(models_with_missing_counts).count
+          counts = strategy.new(models_with_missing_counts).count
 
-            counts.each do |model, count|
-              counts_by_model[model] = count
-            end
+          counts.each do |model, count|
+            counts_by_model[model] = count
           end
         end
       end
diff --git a/lib/gitlab/database/count/exact_count_strategy.rb b/lib/gitlab/database/count/exact_count_strategy.rb
index fa6951eda22..0b8fe640bf8 100644
--- a/lib/gitlab/database/count/exact_count_strategy.rb
+++ b/lib/gitlab/database/count/exact_count_strategy.rb
@@ -23,10 +23,6 @@ module Gitlab
         rescue *CONNECTION_ERRORS
           {}
         end
-
-        def self.enabled?
-          true
-        end
       end
     end
   end
diff --git a/lib/gitlab/database/count/reltuples_count_strategy.rb b/lib/gitlab/database/count/reltuples_count_strategy.rb
index 695f6fa766e..6cd90c01ab2 100644
--- a/lib/gitlab/database/count/reltuples_count_strategy.rb
+++ b/lib/gitlab/database/count/reltuples_count_strategy.rb
@@ -31,10 +31,6 @@ module Gitlab
           {}
         end
 
-        def self.enabled?
-          Gitlab::Database.postgresql?
-        end
-
         private
 
         # Models using single-type inheritance (STI) don't work with
diff --git a/lib/gitlab/database/count/tablesample_count_strategy.rb b/lib/gitlab/database/count/tablesample_count_strategy.rb
index 7777f31f702..e9387a91a14 100644
--- a/lib/gitlab/database/count/tablesample_count_strategy.rb
+++ b/lib/gitlab/database/count/tablesample_count_strategy.rb
@@ -28,10 +28,6 @@ module Gitlab
           {}
         end
 
-        def self.enabled?
-          Gitlab::Database.postgresql? && Feature.enabled?(:tablesample_counts)
-        end
-
         private
 
         def perform_count(model, estimate)
diff --git a/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb
index 3991c737a26..0c1be4b4610 100644
--- a/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb
+++ b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb
@@ -23,18 +23,4 @@ describe Gitlab::Database::Count::ExactCountStrategy do
       expect(subject).to eq({})
     end
   end
-
-  describe '.enabled?' do
-    it 'is enabled for PostgreSQL' do
-      allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
-
-      expect(described_class.enabled?).to be_truthy
-    end
-
-    it 'is enabled for MySQL' do
-      allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
-
-      expect(described_class.enabled?).to be_truthy
-    end
-  end
 end
diff --git a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb
index bd3c66d0548..a528707c9dc 100644
--- a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb
+++ b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb
@@ -48,18 +48,4 @@ describe Gitlab::Database::Count::ReltuplesCountStrategy do
       end
     end
   end
-
-  describe '.enabled?' do
-    it 'is enabled for PostgreSQL' do
-      allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
-
-      expect(described_class.enabled?).to be_truthy
-    end
-
-    it 'is disabled for MySQL' do
-      allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
-
-      expect(described_class.enabled?).to be_falsey
-    end
-  end
 end
diff --git a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb
index 40d810b195b..a57f033b5ed 100644
--- a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb
+++ b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb
@@ -56,22 +56,4 @@ describe Gitlab::Database::Count::TablesampleCountStrategy do
       end
     end
   end
-
-  describe '.enabled?' do
-    before do
-      stub_feature_flags(tablesample_counts: true)
-    end
-
-    it 'is enabled for PostgreSQL' do
-      allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
-
-      expect(described_class.enabled?).to be_truthy
-    end
-
-    it 'is disabled for MySQL' do
-      allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
-
-      expect(described_class.enabled?).to be_falsey
-    end
-  end
 end
diff --git a/spec/lib/gitlab/database/count_spec.rb b/spec/lib/gitlab/database/count_spec.rb
index 1d096b8fa7c..71d6633f62f 100644
--- a/spec/lib/gitlab/database/count_spec.rb
+++ b/spec/lib/gitlab/database/count_spec.rb
@@ -9,24 +9,13 @@ describe Gitlab::Database::Count do
   let(:models) { [Project, Identity] }
 
   context '.approximate_counts' do
-    context 'selecting strategies' do
-      let(:strategies) { [double('s1', enabled?: true), double('s2', enabled?: false)] }
-
-      it 'uses only enabled strategies' do
-        expect(strategies[0]).to receive(:new).and_return(double('strategy1', count: {}))
-        expect(strategies[1]).not_to receive(:new)
-
-        described_class.approximate_counts(models, strategies: strategies)
-      end
-    end
-
     context 'fallbacks' do
       subject { described_class.approximate_counts(models, strategies: strategies) }
 
       let(:strategies) do
         [
-          double('s1', enabled?: true, new: first_strategy),
-          double('s2', enabled?: true, new: second_strategy)
+          double('s1', new: first_strategy),
+          double('s2', new: second_strategy)
         ]
       end