Merge branch '13840-track-primary-package-file-checksum-counts' into 'master'

Resolve "Track primary package file checksum counts"

Closes #13840

See merge request gitlab-org/gitlab!27271

db/migrate/20200218113721_add_indexes_to_package_file.rb

+# frozen_string_literal: true
+
+class AddIndexesToPackageFile < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :packages_package_files, :verification_failure, where: "(verification_failure IS NOT NULL)", name: "packages_packages_verification_failure_partial"
+    add_concurrent_index :packages_package_files, :verification_checksum, where: "(verification_checksum IS NOT NULL)", name: "packages_packages_verification_checksum_partial"
+  end
+
+  def down
+    remove_concurrent_index :packages_package_files, :verification_failure
+    remove_concurrent_index :packages_package_files, :verification_checksum
+  end
+end

db/structure.sql

@@ -10544,6 +10544,10 @@ CREATE INDEX note_mentions_temp_index ON public.notes USING btree (id, noteable_
 
 CREATE UNIQUE INDEX one_canonical_wiki_page_slug_per_metadata ON public.wiki_page_slugs USING btree (wiki_page_meta_id) WHERE (canonical = true);
 
+CREATE INDEX packages_packages_verification_checksum_partial ON public.packages_package_files USING btree (verification_checksum) WHERE (verification_checksum IS NOT NULL);
+
+CREATE INDEX packages_packages_verification_failure_partial ON public.packages_package_files USING btree (verification_failure) WHERE (verification_failure IS NOT NULL);
+
 CREATE INDEX partial_index_ci_builds_on_scheduled_at_with_scheduled_jobs ON public.ci_builds USING btree (scheduled_at) WHERE ((scheduled_at IS NOT NULL) AND ((type)::text = 'Ci::Build'::text) AND ((status)::text = 'scheduled'::text));
 
 CREATE INDEX partial_index_deployments_for_legacy_successful_deployments ON public.deployments USING btree (id) WHERE ((finished_at IS NULL) AND (status = 2));
@@ -13065,6 +13069,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200217210353
 20200217223651
 20200217225719
+20200218113721
 20200219105209
 20200219133859
 20200219135440

doc/administration/monitoring/prometheus/gitlab_metrics.md

@@ -155,6 +155,9 @@ configuration option in `gitlab.yml`. These metrics are served from the
 | `geo_repositories_retrying_verification_count` | Gauge   | 11.2  | Number of repositories verification failures that Geo is actively trying to correct on secondary  | url |
 | `geo_wikis_retrying_verification_count`        | Gauge   | 11.2  | Number of wikis verification failures that Geo is actively trying to correct on secondary | url |
 | `global_search_bulk_cron_queue_size`           | Gauge   | 12.10 | Number of database records waiting to be synchronized to Elasticsearch | |
+| `package_files_count`                          | Gauge   | 13.0  | Number of package files on primary | url |
+| `package_files_checksummed_count`              | Gauge   | 13.0  | Number of package files checksummed on primary | url |
+| `package_files_checksum_failed_count`          | Gauge   | 13.0  | Number of package files failed to calculate the checksum on primary
 
 ## Database load balancing metrics **(PREMIUM ONLY)**
 

doc/api/geo_nodes.md

@@ -364,6 +364,9 @@ Example response:
     "last_successful_status_check_timestamp": 1510125024,
     "version": "10.3.0",
     "revision": "33d33a096a",
+    "package_files_count": 10,
+    "package_files_checksummed_count": 10,
+    "package_files_checksum_failed_count": 0
   },
   {
     "geo_node_id": 2,
@@ -431,7 +434,10 @@ Example response:
     "cursor_last_event_timestamp": 1509681166,
     "last_successful_status_check_timestamp": 1510125024,
     "version": "10.3.0",
-    "revision": "33d33a096a"
+    "revision": "33d33a096a",
+    "package_files_count": 10,
+    "package_files_checksummed_count": 10,
+    "package_files_checksum_failed_count": 0
   }
 ]
 ```

doc/development/geo/framework.md

@@ -84,10 +84,8 @@ module Geo
       model_record.file
     end
 
-    private
-
     # Specify the model this replicator belongs to
-    def model
+    def self.model
       ::Packages::PackageFile
     end
   end
@@ -182,8 +180,8 @@ For example, to add support for files referenced by a `Widget` model with a
    end
    ```
 
-1. Add a partial index on `verification_failure` to ensure re-verification can
-   be performed efficiently:
+1. Add a partial index on `verification_failure` and `verification_checksum` to ensure
+   re-verification can be performed efficiently:
 
    ```ruby
    # frozen_string_literal: true
@@ -197,10 +195,12 @@ For example, to add support for files referenced by a `Widget` model with a
 
      def up
        add_concurrent_index :widgets, :verification_failure, where: "(verification_failure IS NOT NULL)", name: "widgets_verification_failure_partial"
+       add_concurrent_index :widgets, :verification_checksum, where: "(verification_checksum IS NOT NULL)", name: "widgets_verification_checksum_partial"
      end
 
      def down
        remove_concurrent_index :widgets, :verification_failure
+       remove_concurrent_index :widgets, :verification_checksum
      end
    end
    ```
@@ -351,3 +351,30 @@ For example, to add support for files referenced by a `Widget` model with a
    ```
 
 Widget files should now be replicated and verified by Geo!
+
+### Verification statistics with Blob Replicator Strategy
+
+GitLab Geo stores statistic data in the `geo_node_statuses` table.
+
+1. Add fields `widget_count`, `widget_checksummed_count`, and `widget_checksum_failed_count`
+   to `GeoNodeStatus#RESOURCE_STATUS_FIELDS` array in `ee/app/models/geo_node_status.rb`.
+1. Add the same fields to `GeoNodeStatus#PROMETHEUS_METRICS` hash in
+   `ee/app/models/geo_node_status.rb`.
+1. Add the same fields to `Sidekiq metrics` table in
+   `doc/administration/monitoring/prometheus/gitlab_metrics.md`.
+1. Add the same fields to `GET /geo_nodes/status` example response in `doc/api/geo_nodes.md`.
+1. Modify `GeoNodeStatus#load_verification_data` to make sure the fields mantioned above
+   are set:
+
+   ```ruby
+     self.widget_count = Geo::WidgetReplicator.model.count
+     self.widget_checksummed_count = Geo::WidgetReplicator.checksummed.count
+     self.widget_checksum_failed_count = Geo::WidgetReplicator.checksum_failed.count
+   ```
+
+1. Make sure `Widget` model has `checksummed` and `checksum_failed` scopes.
+1. Update `ee/spec/fixtures/api/schemas/public_api/v4/geo_node_status.json` with new fields.
+1. Update `GeoNodeStatus#PROMETHEUS_METRICS` hash in `ee/app/models/geo_node_status.rb` with new fields.
+1. Update `Sidekiq metrics` table in `doc/administration/monitoring/prometheus/gitlab_metrics.md` with new fields.
+1. Update `GET /geo_nodes/status` example response in `doc/api/geo_nodes.md` with new fields.
+1. Update `ee/spec/models/geo_node_status_spec.rb` and `ee/spec/factories/geo_node_statuses.rb` with new fields.

ee/app/models/geo_node_status.rb

@@ -71,6 +71,9 @@ class GeoNodeStatus < ApplicationRecord
     design_repositories_count
     design_repositories_synced_count
     design_repositories_failed_count
+    package_files_count
+    package_files_checksummed_count
+    package_files_checksum_failed_count
   ).freeze
 
   # Be sure to keep this consistent with Prometheus naming conventions
@@ -142,7 +145,10 @@ class GeoNodeStatus < ApplicationRecord
     design_repositories_count: 'Total number of syncable design repositories available on primary',
     design_repositories_synced_count: 'Number of syncable design repositories synced on secondary',
     design_repositories_failed_count: 'Number of syncable design repositories failed to sync on secondary',
-    design_repositories_registry_count: 'Number of design repositories in the registry'
+    design_repositories_registry_count: 'Number of design repositories in the registry',
+    package_files_count: 'Number of package files on primary',
+    package_files_checksummed_count: 'Number of package files checksummed on primary',
+    package_files_checksum_failed_count: 'Number of package files failed to checksum on primary'
   }.freeze
 
   EXPIRATION_IN_MINUTES = 5
@@ -157,6 +163,9 @@ class GeoNodeStatus < ApplicationRecord
 
       define_method("#{attr_name}=") do |val|
         status[attr_name] = val.nil? ? nil : val.to_i
+
+        return unless self.class.column_names.include?(attr_name)
+
         write_attribute(attr_name, val)
       end
     end
@@ -327,6 +336,7 @@ class GeoNodeStatus < ApplicationRecord
   attr_in_percentage :replication_slots_used,        :replication_slots_used_count,        :replication_slots_count
   attr_in_percentage :container_repositories_synced, :container_repositories_synced_count, :container_repositories_count
   attr_in_percentage :design_repositories_synced,    :design_repositories_synced_count,    :design_repositories_count
+  attr_in_percentage :package_files_checksummed,     :package_files_checksummed_count,     :package_files_count
 
   def storage_shards_match?
     return true if geo_node.primary?
@@ -459,6 +469,9 @@ class GeoNodeStatus < ApplicationRecord
       self.repositories_checksum_failed_count = repository_verification_finder.count_verification_failed_repositories
       self.wikis_checksummed_count = repository_verification_finder.count_verified_wikis
       self.wikis_checksum_failed_count = repository_verification_finder.count_verification_failed_wikis
+      self.package_files_count = Geo::PackageFileReplicator.primary_total_count
+      self.package_files_checksummed_count = Geo::PackageFileReplicator.checksummed_count
+      self.package_files_checksum_failed_count = Geo::PackageFileReplicator.checksum_failed_count
     elsif Gitlab::Geo.secondary?
       self.repositories_verified_count = registries_for_verified_projects(:repository).count
       self.repositories_verification_failed_count = registries_for_verification_failed_projects(:repository).count

ee/app/replicators/geo/package_file_replicator.rb

@@ -8,9 +8,7 @@ module Geo
       model_record.file
     end
 
-    private
-
-    def model
+    def self.model
       ::Packages::PackageFile
     end
   end

ee/changelogs/unreleased/13840-track-primary-package-file-checksum-counts.yml

+---
+title: Track primary package file checksum counts
+merge_request: 27271
+author:
+type: added

ee/lib/gitlab/geo/replicable_model.rb

@@ -9,6 +9,9 @@ module Gitlab
       included do
         # If this hook turns out not to apply to all Models, perhaps we should extract a `ReplicableBlobModel`
         after_create_commit -> { replicator.handle_after_create_commit if replicator.respond_to?(:handle_after_create_commit) }
+
+        scope :checksummed, -> { where('verification_checksum IS NOT NULL') }
+        scope :checksum_failed, -> { where('verification_failure IS NOT NULL') }
       end
 
       class_methods do

ee/lib/gitlab/geo/replicator.rb

@@ -51,8 +51,26 @@ module Gitlab
         const_get(replicator_class_name, false)
       end
 
+      def self.checksummed
+        model.checksummed
+      end
+
+      def self.checksummed_count
+        model.checksummed.count
+      end
+
+      def self.checksum_failed_count
+        model.checksum_failed.count
+      end
+
+      def self.primary_total_count
+        model.count
+      end
+
       attr_reader :model_record_id
 
+      delegate :model, to: :class
+
       def initialize(model_record: nil, model_record_id: nil)
         @model_record = model_record
         @model_record_id = model_record_id

ee/spec/factories/geo_node_statuses.rb

@@ -34,6 +34,9 @@ FactoryBot.define do
       repositories_checksum_failed_count { 120 }
       wikis_checksummed_count { 585 }
       wikis_checksum_failed_count { 55 }
+      package_files_count { 585 }
+      package_files_checksummed_count { 585 }
+      package_files_checksum_failed_count { 585 }
       repositories_verified_count { 501 }
       repositories_verification_failed_count { 100 }
       repositories_checksum_mismatch_count { 15 }

ee/spec/factories/packages.rb

@@ -210,9 +210,17 @@ FactoryBot.define do
       size { 1149.bytes }
     end
 
-    trait :object_storage do
+    trait(:object_storage) do
       file_store { Packages::PackageFileUploader::Store::REMOTE }
     end
+
+    trait(:checksummed) do
+      verification_checksum { 'abc' }
+    end
+
+    trait(:checksum_failure) do
+      verification_failure { 'Could not calculate the checksum' }
+    end
   end
 
   factory :maven_metadatum, class: 'Packages::MavenMetadatum' do

ee/spec/fixtures/api/schemas/public_api/v4/geo_node_status.json

@@ -44,6 +44,9 @@
     "wikis_checksummed_count",
     "wikis_checksum_failed_count",
     "wikis_checksummed_in_percentage",
+    "package_files_count",
+    "package_files_checksum_failed_count",
+    "package_files_checksummed_in_percentage",
     "repositories_verified_count",
     "repositories_verification_failed_count",
     "repositories_verified_in_percentage",
@@ -123,6 +126,10 @@
     "wikis_checksummed_count": { "type": ["integer", "null"] },
     "wikis_checksum_failed_count": { "type": ["integer", "null"] },
     "wikis_checksummed_in_percentage": { "type": "string" },
+    "package_files_count": { "type": ["integer", "null"] },
+    "package_files_checksummed_count": { "type": ["integer", "null"] },
+    "package_files_checksum_failed_count": { "type": ["integer", "null"] },
+    "package_files_checksummed_in_percentage": { "type": "string" },
     "repositories_verified_count": { "type": ["integer", "null"] },
     "repositories_verification_failed_count": { "type": ["integer", "null"] },
     "repositories_verified_in_percentage": { "type": "string" },

ee/spec/lib/gitlab/geo/replicator_spec.rb

@@ -8,6 +8,18 @@ describe Gitlab::Geo::Replicator do
   let_it_be(:primary_node) { create(:geo_node, :primary) }
   let_it_be(:secondary_node) { create(:geo_node) }
 
+  before(:all) do
+    ActiveRecord::Schema.define do
+      create_table :dummy_models
+    end
+  end
+
+  after(:all) do
+    ActiveRecord::Schema.define do
+      drop_table :dummy_models, force: true
+    end
+  end
+
   context 'with defined events' do
     before do
       stub_const('DummyReplicator', Class.new(Gitlab::Geo::Replicator))
@@ -46,7 +58,7 @@ describe Gitlab::Geo::Replicator do
 
     context 'model DSL' do
       before do
-        stub_const('DummyModel', Class.new)
+        stub_const('DummyModel', Class.new(ApplicationRecord))
 
         DummyModel.class_eval do
           include ActiveModel::Model

ee/spec/models/geo_node_status_spec.rb

@@ -1224,6 +1224,53 @@ describe GeoNodeStatus, :geo, :geo_fdw do
     end
   end
 
+  describe '#package_files_checksummed_count' do
+    before do
+      stub_current_geo_node(primary)
+    end
+
+    it 'returns the right number of checksummed package files' do
+      create(:package_file, :jar, :checksummed)
+      create(:package_file, :jar, :checksummed)
+      create(:package_file, :jar, :checksum_failure)
+
+      expect(subject.package_files_checksummed_count).to eq(2)
+    end
+  end
+
+  describe '#package_files_checksum_failed_count' do
+    before do
+      stub_current_geo_node(primary)
+    end
+
+    it 'returns the right number of failed package files' do
+      create(:package_file, :jar, :checksummed)
+      create(:package_file, :jar, :checksum_failure)
+      create(:package_file, :jar, :checksum_failure)
+
+      expect(subject.package_files_checksum_failed_count).to eq(2)
+    end
+  end
+
+  describe '#package_files_checksummed_in_percentage' do
+    before do
+      stub_current_geo_node(primary)
+    end
+
+    it 'returns 0 when no package files available' do
+      expect(subject.package_files_checksummed_in_percentage).to eq(0)
+    end
+
+    it 'returns the right percentage' do
+      create(:package_file, :jar, :checksummed)
+      create(:package_file, :jar, :checksummed)
+      create(:package_file, :jar, :checksummed)
+      create(:package_file, :jar, :checksum_failure)
+
+      expect(subject.package_files_checksummed_in_percentage).to be_within(0.0001).of(75)
+    end
+  end
+
   describe '#load_data_from_current_node' do
     context 'on the primary' do
       before do

spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb

@@ -89,7 +89,7 @@ RSpec.shared_examples 'a blob replicator' do
   end
 
   describe '#model' do
-    let(:invoke_model) { replicator.send(:model) }
+    let(:invoke_model) { replicator.class.model }
 
     it 'is implemented' do
       expect do