Read about our upcoming Code of Conduct on this issue

Commit 220b9544 authored by Gabriel Mazetto's avatar Gabriel Mazetto
Browse files

Remove Site Statistic

This approach caused many different problems as we tightened
the query execution timeout.
parent 66d9a411dfd1
......@@ -88,9 +88,6 @@ class Project < ActiveRecord::Base
after_create :create_project_feature, unless: :project_feature
after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) }
before_destroy -> { SiteStatistic.untrack(STATISTICS_ATTRIBUTE) }
after_create :create_ci_cd_settings,
unless: :ci_cd_settings,
if: proc { ProjectCiCdSetting.available? }
......
# frozen_string_literal: true
class SiteStatistic < ActiveRecord::Base
# prevents the creation of multiple rows
default_value_for :id, 1
COUNTER_ATTRIBUTES = %w(repositories_count).freeze
REQUIRED_SCHEMA_VERSION = 20180629153018
# Tracks specific attribute
#
# @param [String] raw_attribute must be one of the values listed in COUNTER_ATTRIBUTES
def self.track(raw_attribute)
with_statistics_available(raw_attribute) do |attribute|
SiteStatistic.update_all(["#{attribute} = #{attribute}+1"])
end
end
# Untracks specific attribute
#
# @param [String] raw_attribute must be one of the values listed in COUNTER_ATTRIBUTES
def self.untrack(raw_attribute)
with_statistics_available(raw_attribute) do |attribute|
SiteStatistic.update_all(["#{attribute} = #{attribute}-1 WHERE #{attribute} > 0"])
end
end
# Wrapper for track/untrack operations with basic validations and enforced requirements
#
# @param [String] raw_attribute must be one of the values listed in COUNTER_ATTRIBUTES
# @yield [String] attribute quoted to be used inside SQL / Arel query
def self.with_statistics_available(raw_attribute)
unless raw_attribute.in?(COUNTER_ATTRIBUTES)
raise ArgumentError, "Invalid attribute: '#{raw_attribute}' to '#{caller_locations(1, 1)[0].label}' method. " \
"Valid attributes are: #{COUNTER_ATTRIBUTES.join(', ')}"
end
return unless available?
self.fetch # make sure record exists
attribute = self.connection.quote_column_name(raw_attribute)
# will be running on its own transaction context
yield(attribute)
end
# Returns a site statistic record with tracked information
#
# @return [SiteStatistic] record with tracked information
def self.fetch
transaction(requires_new: true) do
SiteStatistic.first_or_create!
end
rescue ActiveRecord::RecordNotUnique
retry
end
# Return whether required schema change is available
#
# This is needed in order to degrade gracefully when testing schema migrations
#
# @return [Boolean] whether schema is available
def self.available?
@available_flag ||= ActiveRecord::Migrator.current_version >= REQUIRED_SCHEMA_VERSION
end
# Resets cached column information
#
# This is called during schema migration specs, in order to reset internal cache state
def self.reset_column_information
@available_flag = nil
super
end
end
---
title: Removed Site Statistics optimization as it was causing problems
merge_request: 23314
author:
type: removed
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class DropSiteStatistics < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
drop_table :site_statistics
end
def down
create_table :site_statistics do |t|
t.integer :repositories_count, default: 0, null: false
end
execute('INSERT INTO site_statistics (id) VALUES(1)')
end
end
......@@ -1885,10 +1885,6 @@
t.index ["name"], name: "index_shards_on_name", unique: true, using: :btree
end
create_table "site_statistics", force: :cascade do |t|
t.integer "repositories_count", default: 0, null: false
end
create_table "snippets", force: :cascade do |t|
t.string "title"
t.text "content"
......
namespace :gitlab do
desc "GitLab | Refresh Site Statistics counters"
task refresh_site_statistics: :environment do
puts 'Updating Site Statistics counters: '
print '* Repositories... '
SiteStatistic.transaction do
# see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967
ActiveRecord::Base.connection.execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql?
SiteStatistic.update_all('repositories_count = (SELECT COUNT(*) FROM projects)')
end
puts 'OK!'.color(:green)
puts
end
end
FactoryBot.define do
factory :site_statistics, class: 'SiteStatistic' do
id 1
repositories_count 999
end
end
......@@ -109,22 +109,6 @@
end
end
context 'Site Statistics' do
context 'when creating a new project' do
it 'tracks project in SiteStatistic' do
expect { create(:project) }.to change { SiteStatistic.fetch.repositories_count }.by(1)
end
end
context 'when deleting a project' do
it 'untracks project in SiteStatistic' do
project = create(:project)
expect { project.destroy }.to change { SiteStatistic.fetch.repositories_count }.by(-1)
end
end
end
context 'updating cd_cd_settings' do
it 'does not raise an error' do
project = create(:project)
......
require 'spec_helper'
describe SiteStatistic do
describe '.fetch' do
context 'existing record' do
it 'returns existing SiteStatistic model' do
statistics = create(:site_statistics)
expect(described_class.fetch).to be_a(described_class)
expect(described_class.fetch).to eq(statistics)
end
end
context 'non existing record' do
it 'creates a new SiteStatistic model' do
expect(described_class.first).to be_nil
expect(described_class.fetch).to be_a(described_class)
end
end
end
describe '.track' do
context 'with allowed attributes' do
let(:statistics) { create(:site_statistics) }
it 'increases the attribute counter' do
expect { described_class.track('repositories_count') }.to change { statistics.reload.repositories_count }.by(1)
end
it 'doesnt increase the attribute counter when an exception happens during transaction' do
expect do
begin
described_class.transaction do
described_class.track('repositories_count')
raise StandardError
end
rescue StandardError
# no-op
end
end.not_to change { statistics.reload.repositories_count }
end
end
context 'with not allowed attributes' do
it 'returns error' do
expect { described_class.track('something_else') }.to raise_error(ArgumentError).with_message(/Invalid attribute: \'something_else\' to \'track\' method/)
end
end
end
describe '.untrack' do
context 'with allowed attributes' do
let(:statistics) { create(:site_statistics) }
it 'decreases the attribute counter' do
expect { described_class.untrack('repositories_count') }.to change { statistics.reload.repositories_count }.by(-1)
end
it 'doesnt decrease the attribute counter when an exception happens during transaction' do
expect do
begin
described_class.transaction do
described_class.track('repositories_count')
raise StandardError
end
rescue StandardError
# no-op
end
end.not_to change { described_class.fetch.repositories_count }
end
end
context 'with not allowed attributes' do
it 'returns error' do
expect { described_class.untrack('something_else') }.to raise_error(ArgumentError).with_message(/Invalid attribute: \'something_else\' to \'untrack\' method/)
end
end
end
end
# frozen_string_literal: true
require 'rake_helper'
describe 'rake gitlab:refresh_site_statistics' do
before do
Rake.application.rake_require 'tasks/gitlab/site_statistics'
create(:project)
SiteStatistic.fetch.update(repositories_count: 0)
end
let(:task) { 'gitlab:refresh_site_statistics' }
it 'recalculates existing counters' do
run_rake_task(task)
expect(SiteStatistic.fetch.repositories_count).to eq(1)
end
it 'displays message listing counters' do
expect { run_rake_task(task) }.to output(/Updating Site Statistics counters:.* Repositories\.\.\. OK!/m).to_stdout
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