Merge branch '5172-configurable-verification-concurrency' into 'master'

Resolve "Geo: Use a separate column to rate limit verification checksum workers"

Closes #5172

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

.flayignore

@@ -24,6 +24,8 @@ lib/gitlab/git/tag.rb
 
 ee/db/**/*
 ee/app/serializers/ee/merge_request_widget_entity.rb
+ee/lib/api/epics.rb
+ee/lib/api/geo_nodes.rb
 ee/lib/ee/gitlab/ldap/sync/admin_users.rb
 ee/app/workers/geo/file_download_dispatch_worker/job_artifact_job_finder.rb
 ee/app/workers/geo/file_download_dispatch_worker/lfs_object_job_finder.rb

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180608201435) do
+ActiveRecord::Schema.define(version: 20180612175636) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1159,6 +1159,7 @@ ActiveRecord::Schema.define(version: 20180608201435) do
     t.string "url", null: false
     t.string "selective_sync_type"
     t.text "selective_sync_shards"
+    t.integer "verification_max_capacity", default: 100, null: false
   end
 
   add_index "geo_nodes", ["access_key"], name: "index_geo_nodes_on_access_key", using: :btree

doc/api/geo_nodes.md

@@ -25,6 +25,7 @@ Example response:
     "current": true,
     "files_max_capacity": 10,
     "repos_max_capacity": 25,
+    "verification_max_capacity": 100,
     "clone_protocol": "http"
   },
   {
@@ -35,6 +36,7 @@ Example response:
     "current": false,
     "files_max_capacity": 10,
     "repos_max_capacity": 25,
+    "verification_max_capacity": 100,
     "clone_protocol": "http"
   }
 ]
@@ -61,6 +63,7 @@ Example response:
   "current": true,
   "files_max_capacity": 10,
   "repos_max_capacity": 25,
+  "verification_max_capacity": 100,
   "clone_protocol": "http"
 }
 ```
@@ -75,13 +78,14 @@ _This can only be run against a primary Geo node._
 PUT /geo_nodes/:id
 ```
 
-| Attribute            | Type    | Required | Description                                                               |
-|----------------------|---------|----------|---------------------------------------------------------------------------|
-| `id`                 | integer | yes      | The ID of the Geo node.                                                   |
-| `enabled`            | boolean | no       | Flag indicating if the Geo node is enabled.                               |
-| `url`                | string  | no       | The URL to connect to the Geo node.                                       |
-| `files_max_capacity` | integer | no       | Control the maximum concurrency of LFS/attachment backfill for this node. |
-| `repos_max_capacity` | integer | no       | Control the maximum concurrency of repository backfill for this node.     |
+| Attribute            | Type    | Required  | Description                                                               |
+|----------------------|---------|-----------|---------------------------------------------------------------------------|
+| `id`                 | integer | yes       | The ID of the Geo node.                                                   |
+| `enabled`            | boolean | no        | Flag indicating if the Geo node is enabled.                               |
+| `url`                | string  | no        | The URL to connect to the Geo node.                                       |
+| `files_max_capacity` | integer | no        | Control the maximum concurrency of LFS/attachment backfill for this secondary node. |
+| `repos_max_capacity` | integer | no        | Control the maximum concurrency of repository backfill for this secondary node.     |
+| `verification_max_capacity` | integer | no | Control the maximum concurrency of verification for this node. |
 
 Example response:
 
@@ -94,6 +98,7 @@ Example response:
   "current": true,
   "files_max_capacity": 10,
   "repos_max_capacity": 25,
+  "verification_max_capacity": 100,
   "clone_protocol": "http"
 }
 ```
@@ -131,6 +136,7 @@ Example response:
   "current": true,
   "files_max_capacity": 10,
   "repos_max_capacity": 25,
+  "verification_max_capacity": 100,
   "clone_protocol": "http"
 }
 ```

ee/app/controllers/admin/geo_nodes_controller.rb

@@ -47,7 +47,8 @@ class Admin::GeoNodesController < Admin::ApplicationController
       :selective_sync_shards,
       :namespace_ids,
       :repos_max_capacity,
-      :files_max_capacity
+      :files_max_capacity,
+      :verification_max_capacity
     )
   end
 

ee/app/models/geo_node.rb

@@ -29,6 +29,10 @@ class GeoNode < ActiveRecord::Base
     allow_nil: true
   }
 
+  validates :repos_max_capacity, numericality: { greater_than_or_equal_to: 0 }
+  validates :files_max_capacity, numericality: { greater_than_or_equal_to: 0 }
+  validates :verification_max_capacity, numericality: { greater_than_or_equal_to: 0 }
+
   validate :check_not_adding_primary_as_secondary, if: :secondary?
 
   after_save :expire_cache!

ee/app/views/admin/geo_nodes/_form.html.haml

 = form_errors(geo_node)
 .form-group.row
-  = form.label :url, 'URL', class: 'col-form-label'
+  .col-sm-2
+    = form.label :url, 'URL', class: 'col-form-label'
   .col-sm-10
     = form.text_field :url, class: 'form-control'
 
@@ -13,20 +14,23 @@
 
 .js-hide-if-geo-primary{ class: ('hidden' unless geo_node.secondary?) }
   .form-group.row
-    = form.label :selective_sync_type, s_('Selective synchronization'), class: 'col-form-label'
+    .col-sm-2
+      = form.label :selective_sync_type, s_('Selective synchronization'), class: 'col-form-label'
     .col-sm-10
       = form.select :selective_sync_type, selective_sync_type_options_for_select(geo_node),
           {}, { class: "form-control js-geo-node-selective-sync-type" }
 
   .form-group.row.js-sync-by-namespace{ class: ('hidden' unless geo_node.selective_sync_by_namespaces?) }
-    = form.label :namespace_ids, s_('Geo|Groups to synchronize'), class: 'col-form-label'
+    .col-sm-2
+      = form.label :namespace_ids, s_('Geo|Groups to synchronize'), class: 'col-form-label'
     .col-sm-10
       = hidden_field_tag "#{form.object_name}[namespace_ids]", geo_node.namespace_ids.join(","), class: 'js-geo-node-namespaces', data: { selected: node_namespaces_options(geo_node.namespaces).to_json }
       .form-text.text-muted
         #{ s_("Choose which groups you wish to synchronize to this secondary node.") }
 
   .form-group.row.js-sync-by-shard{ class: ('hidden' unless geo_node.selective_sync_by_shards?) }
-    = form.label :selective_sync_shards, s_('Geo|Shards to synchronize'), class: 'col-form-label'
+    .col-sm-2
+      = form.label :selective_sync_shards, s_('Geo|Shards to synchronize'), class: 'col-form-label'
     .col-sm-10
       = form.select :selective_sync_shards, repository_storages_options_for_select(geo_node.selective_sync_shards),
         { include_hidden: false }, multiple: true, class: 'form-control'
@@ -34,15 +38,25 @@
         #{ s_("Choose which shards you wish to synchronize to this secondary node.") }
 
 .form-group.row.js-hide-if-geo-primary{ class: ('hidden' unless geo_node.secondary?) }
-  = form.label :repos_max_capacity, s_('Geo|Repository sync capacity'), class: 'col-form-label'
+  .col-sm-2
+    = form.label :repos_max_capacity, s_('Geo|Repository sync capacity'), class: 'col-form-label'
   .col-sm-10
     = form.number_field :repos_max_capacity, class: 'form-control', min: 0
     .form-text.text-muted
       #{ s_('Control the maximum concurrency of repository backfill for this secondary node') }
 
 .form-group.row.js-hide-if-geo-primary{ class: ('hidden' unless geo_node.secondary?) }
-  = form.label :files_max_capacity, s_('Geo|File sync capacity'), class: 'col-form-label'
+  .col-sm-2
+    = form.label :files_max_capacity, s_('Geo|File sync capacity'), class: 'col-form-label'
   .col-sm-10
     = form.number_field :files_max_capacity, class: 'form-control', min: 0
     .form-text.text-muted
       #{ s_('Control the maximum concurrency of LFS/attachment backfill for this secondary node') }
+
+.form-group.row
+  .col-sm-2
+    = form.label :verification_max_capacity, s_('Geo|Verification capacity'), class: 'col-form-label'
+  .col-sm-10
+    = form.number_field :verification_max_capacity, class: 'form-control', min: 0
+    .form-text.text-muted
+      #{ s_('Control the maximum concurrency of verification operations for this Geo node') }

ee/app/workers/geo/repository_verification/primary/shard_worker.rb

@@ -4,8 +4,6 @@ module Geo
       class ShardWorker < Geo::Scheduler::Primary::SchedulerWorker
         sidekiq_options retry: false
 
-        MAX_CAPACITY = 100
-
         attr_accessor :shard_name
 
         def perform(shard_name)
@@ -28,7 +26,7 @@ module Geo
         end
 
         def max_capacity
-          MAX_CAPACITY
+          current_node.verification_max_capacity
         end
 
         def schedule_job(project_id)

ee/app/workers/geo/repository_verification/secondary/shard_worker.rb

@@ -4,8 +4,6 @@ module Geo
       class ShardWorker < Geo::Scheduler::Secondary::SchedulerWorker
         include CronjobQueue
 
-        MAX_CAPACITY = 1000
-
         attr_accessor :shard_name
 
         def perform(shard_name)
@@ -23,7 +21,7 @@ module Geo
         end
 
         def max_capacity
-          MAX_CAPACITY
+          current_node.verification_max_capacity
         end
 
         def load_pending_resources

ee/changelogs/unreleased/5172-configurable-verification-concurrency.yml

+---
+title: Allow repository verification concurrency to be controlled on primary and secondary
+merge_request: 6102
+author:
+type: added

ee/db/migrate/20180612175636_add_geo_nodes_verification_max_capacity.rb

+class AddGeoNodesVerificationMaxCapacity < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :geo_nodes, :verification_max_capacity, :integer, default: 100, null: false
+  end
+end

ee/lib/api/geo_nodes.rb

@@ -134,6 +134,7 @@ module API
           optional :url, type: String, desc: 'The URL to connect to the Geo node'
           optional :files_max_capacity, type: Integer, desc: 'Control the maximum concurrency of LFS/attachment backfill for this secondary node'
           optional :repos_max_capacity, type: Integer, desc: 'Control the maximum concurrency of repository backfill for this secondary node'
+          optional :verification_max_capacity, type: Integer, desc: 'Control the maximum concurrency of repository verification for this node'
         end
         put do
           not_found!('GeoNode') unless geo_node

ee/lib/ee/api/entities.rb

@@ -218,6 +218,7 @@ module EE
         expose :current?, as: :current
         expose :files_max_capacity
         expose :repos_max_capacity
+        expose :verification_max_capacity
 
         # Retained for backwards compatibility. Remove in API v5
         expose :clone_protocol do |_record, _options|

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

@@ -8,6 +8,7 @@
     "current",
     "files_max_capacity",
     "repos_max_capacity",
+    "verification_max_capacity",
     "clone_protocol",
     "_links"
   ],
@@ -19,6 +20,7 @@
     "current": { "type": "boolean" },
     "files_max_capacity": { "type": "integer" },
     "repos_max_capacity": { "type": "integer" },
+    "verification_max_capacity": { "type": "integer" },
     "clone_protocol": { "type": ["string"] },
     "web_edit_url": { "type": "string" },
     "_links": {

ee/spec/models/geo_node_spec.rb

@@ -23,6 +23,9 @@ describe GeoNode, type: :model do
 
   context 'validations' do
     it { is_expected.to validate_inclusion_of(:selective_sync_type).in_array([nil, *GeoNode::SELECTIVE_SYNC_TYPES]) }
+    it { is_expected.to validate_numericality_of(:repos_max_capacity).is_greater_than_or_equal_to(0) }
+    it { is_expected.to validate_numericality_of(:files_max_capacity).is_greater_than_or_equal_to(0) }
+    it { is_expected.to validate_numericality_of(:verification_max_capacity).is_greater_than_or_equal_to(0) }
   end
 
   context 'default values' do

ee/spec/requests/api/geo_nodes_spec.rb

@@ -177,7 +177,8 @@ describe API::GeoNodes, :geo, :prometheus, api: true do
         enabled: false,
         url: 'https://updated.example.com/',
         files_max_capacity: 33,
-        repos_max_capacity: 44
+        repos_max_capacity: 44,
+        verification_max_capacity: 55
       }.stringify_keys
 
       put api("/geo_nodes/#{secondary.id}", admin), params