Enable tablesample count strategy by default
https://gitlab.com/gitlab-org/gitlab-ce/issues/58792
This commit is contained in:
parent
25698fda23
commit
259493bb4c
9 changed files with 12 additions and 78 deletions
5
changelogs/unreleased/ab-count-strategies.yml
Normal file
5
changelogs/unreleased/ab-count-strategies.yml
Normal file
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Use tablesample approximate counting by default.
|
||||
merge_request: 31048
|
||||
author:
|
||||
type: performance
|
|
@ -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
|
||||
|
|
|
@ -23,10 +23,6 @@ module Gitlab
|
|||
rescue *CONNECTION_ERRORS
|
||||
{}
|
||||
end
|
||||
|
||||
def self.enabled?
|
||||
true
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -28,10 +28,6 @@ module Gitlab
|
|||
{}
|
||||
end
|
||||
|
||||
def self.enabled?
|
||||
Gitlab::Database.postgresql? && Feature.enabled?(:tablesample_counts)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def perform_count(model, estimate)
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
Loading…
Reference in a new issue