Merge branch '211630-move-geo_node_status-data-from-individual-columns-to-jsonb-hash' into 'master'

Move geo_node_status data from individual columns to JSONB hash

Closes #211630

See merge request gitlab-org/gitlab!28586

db/migrate/20200401095430_add_jsonb_to_geo_node_status_table.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddJsonbToGeoNodeStatusTable < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    change_table :geo_node_statuses do |t|
+      t.jsonb :status, null: false, default: {}
+    end
+  end
+end

db/structure.sql

@@ -2650,7 +2650,8 @@ CREATE TABLE public.geo_node_statuses (
     design_repositories_count integer,
     design_repositories_synced_count integer,
     design_repositories_failed_count integer,
-    design_repositories_registry_count integer
+    design_repositories_registry_count integer,
+    status jsonb DEFAULT '{}'::jsonb NOT NULL
 );
 
 CREATE SEQUENCE public.geo_node_statuses_id_seq
@@ -13076,6 +13077,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200331132103
 20200331195952
 20200331220930
+20200401095430
 20200401211005
 20200402123926
 20200402124802

ee/app/models/geo_node_status.rb

@@ -34,6 +34,45 @@ class GeoNodeStatus < ApplicationRecord
   alias_attribute :last_event_timestamp, :last_event_date_timestamp
   alias_attribute :cursor_last_event_timestamp, :cursor_last_event_date_timestamp
 
+  RESOURCE_STATUS_FIELDS = %w(
+    repositories_synced_count
+    repositories_failed_count
+    lfs_objects_count
+    lfs_objects_synced_count
+    lfs_objects_failed_count
+    attachments_count
+    attachments_synced_count
+    attachments_failed_count
+    wikis_synced_count
+    wikis_failed_count
+    job_artifacts_count
+    job_artifacts_synced_count
+    job_artifacts_failed_count
+    repositories_verified_count
+    repositories_verification_failed_count
+    wikis_verified_count
+    wikis_verification_failed_count
+    lfs_objects_synced_missing_on_primary_count
+    job_artifacts_synced_missing_on_primary_count
+    attachments_synced_missing_on_primary_count
+    repositories_checksummed_count
+    repositories_checksum_failed_count
+    repositories_checksum_mismatch_count
+    wikis_checksummed_count
+    wikis_checksum_failed_count
+    wikis_checksum_mismatch_count
+    repositories_retrying_verification_count
+    wikis_retrying_verification_count
+    projects_count
+    container_repositories_count
+    container_repositories_synced_count
+    container_repositories_failed_count
+    container_repositories_registry_count
+    design_repositories_count
+    design_repositories_synced_count
+    design_repositories_failed_count
+  ).freeze
+
   # Be sure to keep this consistent with Prometheus naming conventions
   PROMETHEUS_METRICS = {
     db_replication_lag_seconds: 'Database replication lag (seconds)',
@@ -110,6 +149,27 @@ class GeoNodeStatus < ApplicationRecord
   HEALTHY_STATUS = 'Healthy'.freeze
   UNHEALTHY_STATUS = 'Unhealthy'.freeze
 
+  def self.alternative_status_store_accessor(attr_names)
+    attr_names.each do |attr_name|
+      define_method(attr_name) do
+        status[attr_name] || read_attribute(attr_name)
+      end
+
+      define_method("#{attr_name}=") do |val|
+        status[attr_name] = val.nil? ? nil : val.to_i
+        write_attribute(attr_name, val)
+      end
+    end
+  end
+
+  # We migrated from attributes stored in individual columns to
+  # a single JSONB hash. To create accessors method for every fields in hash we could use
+  # Rails embeded store_accessor but in this case we won't have smooth update procedure.
+  # The method "alternative_status_store_accessor" does actually the same that store_accessor would
+  # but it uses the old fashioned fields in case the new ones are not set yet. We also casting the type into integer when
+  # we set the value to preserve the old behaviour.
+  alternative_status_store_accessor RESOURCE_STATUS_FIELDS
+
   def self.current_node_status
     current_node = Gitlab::Geo.current_node
     return unless current_node
@@ -240,9 +300,9 @@ class GeoNodeStatus < ApplicationRecord
 
   def self.attr_in_percentage(attr_name, count, total)
     define_method("#{attr_name}_in_percentage") do
-      return 0 if read_attribute(total).to_i.zero?
+      return 0 if self[total].to_i.zero?
 
-      (read_attribute(count).to_f / read_attribute(total).to_f) * 100.0
+      (self[count].to_f / self[total].to_f) * 100.0
     end
   end
 

ee/spec/models/geo_node_status_spec.rb

@@ -1268,5 +1268,31 @@ describe GeoNodeStatus, :geo, :geo_fdw do
         subject
       end
     end
+
+    context 'backward compatibility when counters stored in separate columns' do
+      describe '#projects_count' do
+        it 'counts the number of projects' do
+          subject.write_attribute(:projects_count, 10)
+          subject.status = {}
+
+          expect(subject.projects_count).to eq 10
+        end
+
+        it 'sets data in both ways, deprecated and the new one' do
+          subject.projects_count = 10
+
+          expect(subject.projects_count).to eq 10
+          expect(subject.read_attribute(:projects_count)).to eq 10
+        end
+
+        it 'uses column counters when calculates percents using attr_in_percentage' do
+          subject.write_attribute(:design_repositories_count, 10)
+          subject.write_attribute(:design_repositories_synced_count, 5)
+          subject.status = {}
+
+          expect(subject.design_repositories_synced_in_percentage).to be_within(0.0001).of(50)
+        end
+      end
+    end
   end
 end