Merge branch 'follow-up-geo-sync-disabled-wikis' into 'master'

Geo: sync disabled wikis. Stage 2

Closes #7623

See merge request gitlab-org/gitlab-ee!6906

app/models/project.rb

@@ -90,7 +90,7 @@ class Project < ActiveRecord::Base
   after_create :create_project_feature, unless: :project_feature
 
   after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) }
-  before_destroy :untrack_site_statistics
+  before_destroy -> { SiteStatistic.untrack(STATISTICS_ATTRIBUTE) }
 
   after_create :create_ci_cd_settings,
     unless: :ci_cd_settings,
@@ -2122,11 +2122,6 @@ class Project < ActiveRecord::Base
     Gitlab::PagesTransfer.new.rename_project(path_before, self.path, namespace.full_path)
   end
 
-  def untrack_site_statistics
-    SiteStatistic.untrack(STATISTICS_ATTRIBUTE)
-    self.project_feature.untrack_statistics_for_deletion!
-  end
-
   # rubocop: disable CodeReuse/ServiceClass
   def execute_rename_repository_hooks!(full_path_before)
     # When we import a project overwriting the original project, there

app/models/project_feature.rb

@@ -21,7 +21,6 @@ class ProjectFeature < ActiveRecord::Base
   ENABLED  = 20
 
   FEATURES = %i(issues merge_requests wiki snippets builds repository).freeze
-  STATISTICS_ATTRIBUTE = 'wikis_count'.freeze
 
   class << self
     def access_level_attribute(feature)
@@ -61,9 +60,6 @@ class ProjectFeature < ActiveRecord::Base
     end
   end
 
-  after_create ->(model) { SiteStatistic.track(STATISTICS_ATTRIBUTE) if model.wiki_enabled? }
-  after_update :update_site_statistics
-
   def feature_available?(feature, user)
     get_permission(user, access_level(feature))
   end
@@ -88,30 +84,8 @@ class ProjectFeature < ActiveRecord::Base
     issues_access_level > DISABLED
   end
 
-  # This is a workaround for the removal hooks not been triggered when removing a Project.
-  #
-  # ProjectFeature is removed using database cascade index rule.
-  # This method is called by Project model when deletion starts.
-  def untrack_statistics_for_deletion!
-    return unless wiki_enabled?
-
-    SiteStatistic.untrack(STATISTICS_ATTRIBUTE)
-  end
-
   private
 
-  def update_site_statistics
-    return unless wiki_access_level_changed?
-
-    if self.wiki_access_level_was == DISABLED
-      # possible new states are PRIVATE / ENABLED, both should be tracked
-      SiteStatistic.track(STATISTICS_ATTRIBUTE)
-    elsif self.wiki_access_level == DISABLED
-      # old state was either PRIVATE / ENABLED, only untrack if new state is DISABLED
-      SiteStatistic.untrack(STATISTICS_ATTRIBUTE)
-    end
-  end
-
   # Validates builds and merge requests access level
   # which cannot be higher than repository access level
   def repository_children_level

app/models/site_statistic.rb

@@ -4,7 +4,7 @@ class SiteStatistic < ActiveRecord::Base
   # prevents the creation of multiple rows
   default_value_for :id, 1
 
-  COUNTER_ATTRIBUTES = %w(repositories_count wikis_count).freeze
+  COUNTER_ATTRIBUTES = %w(repositories_count).freeze
   REQUIRED_SCHEMA_VERSION = 20180629153018
 
   # Tracks specific attribute

db/post_migrate/20180917172041_remove_wikis_count_from_site_statistics.rb

+# frozen_string_literal: true
+class RemoveWikisCountFromSiteStatistics < ActiveRecord::Migration
+  def change
+    remove_column :site_statistics, :wikis_count, :integer
+  end
+end

db/schema.rb

@@ -1145,7 +1145,6 @@ ActiveRecord::Schema.define(version: 20180920043317) do
   create_table "geo_node_statuses", force: :cascade do |t|
     t.integer "geo_node_id", null: false
     t.integer "db_replication_lag_seconds"
-    t.integer "repositories_count"
     t.integer "repositories_synced_count"
     t.integer "repositories_failed_count"
     t.integer "lfs_objects_count"
@@ -1165,7 +1164,6 @@ ActiveRecord::Schema.define(version: 20180920043317) do
     t.integer "replication_slots_count"
     t.integer "replication_slots_used_count"
     t.integer "replication_slots_max_retained_wal_bytes", limit: 8
-    t.integer "wikis_count"
     t.integer "wikis_synced_count"
     t.integer "wikis_failed_count"
     t.integer "job_artifacts_count"
@@ -2570,7 +2568,6 @@ ActiveRecord::Schema.define(version: 20180920043317) do
 
   create_table "site_statistics", force: :cascade do |t|
     t.integer "repositories_count", default: 0, null: false
-    t.integer "wikis_count", default: 0, null: false
   end
 
   create_table "slack_integrations", force: :cascade do |t|

ee/app/models/geo_node_status.rb

@@ -22,13 +22,16 @@ class GeoNodeStatus < ActiveRecord::Base
 
   sha_attribute :storage_configuration_digest
 
+  # It's needed for backward compatibility as we expose them via public API
+  alias_attribute :wikis_count, :projects_count
+  alias_attribute :repositories_count, :projects_count
+
   # Be sure to keep this consistent with Prometheus naming conventions
   PROMETHEUS_METRICS = {
     db_replication_lag_seconds: 'Database replication lag (seconds)',
     repositories_count: 'Total number of repositories available on primary',
     repositories_synced_count: 'Number of repositories synced on secondary',
     repositories_failed_count: 'Number of repositories failed to sync on secondary',
-    wikis_count: 'Total number of wikis available on primary',
     wikis_synced_count: 'Number of wikis synced on secondary',
     wikis_failed_count: 'Number of wikis failed to sync on secondary',
     repositories_checksummed_count: 'Number of repositories checksummed on primary',
@@ -162,11 +165,6 @@ class GeoNodeStatus < ActiveRecord::Base
     self.storage_shards = StorageShard.all
     self.storage_configuration_digest = StorageShard.build_digest
 
-    # Backward compatibility. These are deprecated and not used normally. Only needed for smooth update
-    # when secondary node is outdated yet
-    self.repositories_count = projects_finder.count_projects
-    self.wikis_count = projects_finder.count_projects
-
     self.version = Gitlab::VERSION
     self.revision = Gitlab.revision
 
@@ -313,7 +311,7 @@ class GeoNodeStatus < ActiveRecord::Base
   end
 
   def wikis_synced_in_percentage
-    calc_percentage(wikis_count, wikis_synced_count)
+    calc_percentage(projects_count, wikis_synced_count)
   end
 
   def repositories_checksummed_in_percentage
@@ -321,7 +319,7 @@ class GeoNodeStatus < ActiveRecord::Base
   end
 
   def wikis_checksummed_in_percentage
-    calc_percentage(wikis_count, wikis_checksummed_count)
+    calc_percentage(projects_count, wikis_checksummed_count)
   end
 
   def repositories_verified_in_percentage
@@ -329,7 +327,7 @@ class GeoNodeStatus < ActiveRecord::Base
   end
 
   def wikis_verified_in_percentage
-    calc_percentage(wikis_count, wikis_verified_count)
+    calc_percentage(projects_count, wikis_verified_count)
   end
 
   def repositories_checked_in_percentage
@@ -363,15 +361,6 @@ class GeoNodeStatus < ActiveRecord::Base
     public_send(key) # rubocop:disable GitlabSecurity/PublicSend
   end
 
-  # This method is for backward compatibility only.
-  # During the app update the secondary node can be outdated, and it does not provide
-  # us with the projects_count which is already expected by updated primary node.
-  # So we just fallback to old repositories_count
-  def projects_count
-    projects_count_attr = read_attribute(:projects_count)
-    projects_count_attr.nil? ? repositories_count : projects_count_attr
-  end
-
   private
 
   def primary_storage_digest

ee/db/post_migrate/20180815043102_remove_wikis_count_and_repositories_count_from_geo_node_statuses.rb

+# 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 RemoveWikisCountAndRepositoriesCountFromGeoNodeStatuses < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    remove_column :geo_node_statuses, :wikis_count, :integer
+    remove_column :geo_node_statuses, :repositories_count, :integer
+  end
+
+  def down
+    add_column :geo_node_statuses, :wikis_count, :integer
+    add_column :geo_node_statuses, :repositories_count, :integer
+  end
+end

ee/lib/tasks/geo.rake

@@ -261,13 +261,13 @@ namespace :geo do
 
     print 'Wikis: '.rjust(COLUMN_WIDTH)
     show_failed_value(current_node_status.wikis_failed_count)
-    print "#{current_node_status.wikis_synced_count}/#{current_node_status.wikis_count} "
+    print "#{current_node_status.wikis_synced_count}/#{current_node_status.projects_count} "
     puts using_percentage(current_node_status.wikis_synced_in_percentage)
 
     if Gitlab::Geo.repository_verification_enabled?
       print 'Verified Wikis: '.rjust(COLUMN_WIDTH)
       show_failed_value(current_node_status.wikis_verification_failed_count)
-      print "#{current_node_status.wikis_verified_count}/#{current_node_status.wikis_count} "
+      print "#{current_node_status.wikis_verified_count}/#{current_node_status.projects_count} "
       puts using_percentage(current_node_status.wikis_verified_in_percentage)
     end
 

ee/spec/factories/geo_node_statuses.rb

@@ -17,11 +17,9 @@ FactoryBot.define do
       job_artifacts_failed_count 3
       job_artifacts_synced_count 577
       job_artifacts_synced_missing_on_primary_count 91
-      repositories_count 10
       projects_count 10
       repositories_synced_count 5
       repositories_failed_count 0
-      wikis_count 9
       wikis_synced_count 4
       wikis_failed_count 1
       repositories_checksummed_count 600

ee/spec/services/geo/metrics_update_service_spec.rb

@@ -15,11 +15,9 @@ describe Geo::MetricsUpdateService, :geo, :prometheus do
     {
       status_message: nil,
       db_replication_lag_seconds: 0,
-      repositories_count: 10,
       projects_count: 10,
       repositories_synced_count: 1,
       repositories_failed_count: 2,
-      wikis_count: 10,
       wikis_synced_count: 2,
       wikis_failed_count: 3,
       lfs_objects_count: 100,
@@ -56,7 +54,6 @@ describe Geo::MetricsUpdateService, :geo, :prometheus do
     {
       status_message: nil,
       repositories_count: 10,
-      wikis_count: 10,
       projects_count: 10,
       lfs_objects_count: 100,
       job_artifacts_count: 100,
@@ -149,7 +146,6 @@ describe Geo::MetricsUpdateService, :geo, :prometheus do
         expect(metric_value(:geo_repositories)).to eq(10)
         expect(metric_value(:geo_repositories_synced)).to eq(1)
         expect(metric_value(:geo_repositories_failed)).to eq(2)
-        expect(metric_value(:geo_wikis)).to eq(10)
         expect(metric_value(:geo_wikis_synced)).to eq(2)
         expect(metric_value(:geo_wikis_failed)).to eq(3)
         expect(metric_value(:geo_lfs_objects)).to eq(100)

lib/tasks/gitlab/site_statistics.rake

@@ -10,14 +10,6 @@ namespace :gitlab do
       SiteStatistic.update_all('repositories_count = (SELECT COUNT(*) FROM projects)')
     end
     puts 'OK!'.color(:green)
-
-    print '* Wikis... '
-    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('wikis_count = (SELECT COUNT(*) FROM project_features WHERE wiki_access_level != 0)')
-    end
-    puts 'OK!'.color(:green)
     puts
   end
 end

spec/factories/site_statistics.rb

@@ -2,6 +2,5 @@ FactoryBot.define do
   factory :site_statistics, class: 'SiteStatistic' do
     id 1
     repositories_count 999
-    wikis_count 555
   end
 end

spec/models/project_feature_spec.rb

@@ -128,46 +128,4 @@ describe ProjectFeature do
       end
     end
   end
-
-  context 'Site Statistics' do
-    set(:project_with_wiki) { create(:project, :wiki_enabled) }
-    set(:project_without_wiki) { create(:project, :wiki_disabled) }
-
-    context 'when creating a project' do
-      it 'tracks wiki availability when wikis are enabled by default' do
-        expect { create(:project) }.to change { SiteStatistic.fetch.wikis_count }.by(1)
-      end
-
-      it 'does not track wiki availability when wikis are disabled by default' do
-        expect { create(:project, :wiki_disabled) }.not_to change { SiteStatistic.fetch.wikis_count }
-      end
-    end
-
-    context 'when updating a project_feature' do
-      it 'untracks wiki availability when disabling wiki access' do
-        expect { project_with_wiki.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) }
-          .to change { SiteStatistic.fetch.wikis_count }.by(-1)
-      end
-
-      it 'tracks again wiki availability when re-enabling wiki access as public' do
-        expect { project_without_wiki.project_feature.update_attribute(:wiki_access_level, ProjectFeature::ENABLED) }
-          .to change { SiteStatistic.fetch.wikis_count }.by(1)
-      end
-
-      it 'tracks again wiki availability when re-enabling wiki access as private' do
-        expect { project_without_wiki.project_feature.update_attribute(:wiki_access_level, ProjectFeature::PRIVATE) }
-          .to change { SiteStatistic.fetch.wikis_count }.by(1)
-      end
-    end
-
-    context 'when removing a project' do
-      it 'untracks wiki availability when removing a project with previous wiki access' do
-        expect { project_with_wiki.destroy }.to change { SiteStatistic.fetch.wikis_count }.by(-1)
-      end
-
-      it 'does not untrack wiki availability when removing a project without wiki access' do
-        expect { project_without_wiki.destroy }.not_to change { SiteStatistic.fetch.wikis_count }
-      end
-    end
-  end
 end

spec/models/site_statistic_spec.rb

@@ -25,7 +25,6 @@ describe SiteStatistic do
 
       it 'increases the attribute counter' do
         expect { described_class.track('repositories_count') }.to change { statistics.reload.repositories_count }.by(1)
-        expect { described_class.track('wikis_count') }.to change { statistics.reload.wikis_count }.by(1)
       end
 
       it 'doesnt increase the attribute counter when an exception happens during transaction' do
@@ -56,7 +55,6 @@ describe SiteStatistic do
 
       it 'decreases the attribute counter' do
         expect { described_class.untrack('repositories_count') }.to change { statistics.reload.repositories_count }.by(-1)
-        expect { described_class.untrack('wikis_count') }.to change { statistics.reload.wikis_count }.by(-1)
       end
 
       it 'doesnt decrease the attribute counter when an exception happens during transaction' do

spec/services/groups/destroy_service_spec.rb

@@ -35,14 +35,6 @@ describe Groups::DestroyService do
       it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) }
     end
 
-    context 'site statistics' do
-      it 'doesnt trigger project deletion hooks twice' do
-        expect_any_instance_of(Project).to receive(:untrack_site_statistics).once
-
-        destroy_group(group, user, async)
-      end
-    end
-
     context 'mattermost team' do
       let!(:chat_team) { create(:chat_team, namespace: group) }
 

spec/tasks/gitlab/site_statistics_rake_spec.rb

@@ -6,7 +6,7 @@ describe 'rake gitlab:refresh_site_statistics' do
     Rake.application.rake_require 'tasks/gitlab/site_statistics'
 
     create(:project)
-    SiteStatistic.fetch.update(repositories_count: 0, wikis_count: 0)
+    SiteStatistic.fetch.update(repositories_count: 0)
   end
 
   let(:task) { 'gitlab:refresh_site_statistics' }
@@ -15,10 +15,9 @@ describe 'rake gitlab:refresh_site_statistics' do
     run_rake_task(task)
 
     expect(SiteStatistic.fetch.repositories_count).to eq(1)
-    expect(SiteStatistic.fetch.wikis_count).to eq(1)
   end
 
   it 'displays message listing counters' do
-    expect { run_rake_task(task) }.to output(/Updating Site Statistics counters:.* Repositories\.\.\. OK!.* Wikis\.\.\. OK!/m).to_stdout
+    expect { run_rake_task(task) }.to output(/Updating Site Statistics counters:.* Repositories\.\.\. OK!/m).to_stdout
   end
 end