Commit e7f795a3 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'ab-count-strategies' into 'master'

Remove feature flag for tablesample counts

See merge request gitlab-org/gitlab-ce!31048
parents 25698fda 259493bb
---
title: Use tablesample approximate counting by default.
merge_request: 31048
author:
type: performance
...@@ -37,7 +37,6 @@ module Gitlab ...@@ -37,7 +37,6 @@ module Gitlab
# @return [Hash] of Model -> count mapping # @return [Hash] of Model -> count mapping
def self.approximate_counts(models, strategies: [TablesampleCountStrategy, ReltuplesCountStrategy, ExactCountStrategy]) def self.approximate_counts(models, strategies: [TablesampleCountStrategy, ReltuplesCountStrategy, ExactCountStrategy])
strategies.each_with_object({}) do |strategy, counts_by_model| 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?
...@@ -51,5 +50,4 @@ module Gitlab ...@@ -51,5 +50,4 @@ module Gitlab
end end
end end
end end
end
end end
...@@ -23,10 +23,6 @@ module Gitlab ...@@ -23,10 +23,6 @@ module Gitlab
rescue *CONNECTION_ERRORS rescue *CONNECTION_ERRORS
{} {}
end end
def self.enabled?
true
end
end end
end end
end end
......
...@@ -31,10 +31,6 @@ module Gitlab ...@@ -31,10 +31,6 @@ module Gitlab
{} {}
end end
def self.enabled?
Gitlab::Database.postgresql?
end
private private
# Models using single-type inheritance (STI) don't work with # Models using single-type inheritance (STI) don't work with
......
...@@ -28,10 +28,6 @@ module Gitlab ...@@ -28,10 +28,6 @@ module Gitlab
{} {}
end end
def self.enabled?
Gitlab::Database.postgresql? && Feature.enabled?(:tablesample_counts)
end
private private
def perform_count(model, estimate) def perform_count(model, estimate)
......
...@@ -23,18 +23,4 @@ describe Gitlab::Database::Count::ExactCountStrategy do ...@@ -23,18 +23,4 @@ describe Gitlab::Database::Count::ExactCountStrategy do
expect(subject).to eq({}) expect(subject).to eq({})
end 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 enabled for MySQL' do
allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
expect(described_class.enabled?).to be_truthy
end
end
end end
...@@ -48,18 +48,4 @@ describe Gitlab::Database::Count::ReltuplesCountStrategy do ...@@ -48,18 +48,4 @@ describe Gitlab::Database::Count::ReltuplesCountStrategy do
end end
end 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 end
...@@ -56,22 +56,4 @@ describe Gitlab::Database::Count::TablesampleCountStrategy do ...@@ -56,22 +56,4 @@ describe Gitlab::Database::Count::TablesampleCountStrategy do
end end
end 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 end
...@@ -9,24 +9,13 @@ describe Gitlab::Database::Count do ...@@ -9,24 +9,13 @@ describe Gitlab::Database::Count do
let(:models) { [Project, Identity] } let(:models) { [Project, Identity] }
context '.approximate_counts' do 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 context 'fallbacks' do
subject { described_class.approximate_counts(models, strategies: strategies) } subject { described_class.approximate_counts(models, strategies: strategies) }
let(:strategies) do let(:strategies) do
[ [
double('s1', enabled?: true, new: first_strategy), double('s1', new: first_strategy),
double('s2', enabled?: true, new: second_strategy) double('s2', new: second_strategy)
] ]
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment