Geo: sync disabled wikis. Stage 2

We started syncing all the wiki regardless of the fact it's disabled or
not. We couldn't do that in one stage because of needing of smoth update
and deprecating things. This is the second stage that finally removes
unused columns in the geo_node_status table.

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

@@ -1141,7 +1141,6 @@ ActiveRecord::Schema.define(version: 20180917214204) 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"
@@ -1161,7 +1160,6 @@ ActiveRecord::Schema.define(version: 20180917214204) 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"
@@ -2566,7 +2564,6 @@ ActiveRecord::Schema.define(version: 20180917214204) 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