Merge branch '296882-standalone-shard-replica-settings' into 'master'

Allow setting the shard/replica separately for ES indexes (step 1)

See merge request gitlab-org/gitlab!56344

changelogs/unreleased/296882-standalone-shard-replica-settings.yml

+---
+title: Allow setting the shard/replica separately for standalone indexes
+merge_request: 56344
+author:
+type: changed

db/migrate/20210317100520_create_elastic_index_settings.rb

+# frozen_string_literal: true
+
+class CreateElasticIndexSettings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    create_table_with_constraints :elastic_index_settings do |t|
+      t.timestamps_with_timezone null: false
+      t.integer :number_of_replicas, null: false, default: 1, limit: 2
+      t.integer :number_of_shards, null: false, default: 5, limit: 2
+      t.text :alias_name, null: false
+
+      t.text_limit :alias_name, 255
+      t.index :alias_name, unique: true
+    end
+  end
+
+  def down
+    drop_table :elastic_index_settings
+  end
+end

db/migrate/20210324131727_migrate_elastic_index_settings.rb

+# frozen_string_literal: true
+
+class MigrateElasticIndexSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  ALIAS_NAME = [Rails.application.class.module_parent_name.downcase, Rails.env].join('-')
+
+  class ElasticIndexSetting < ActiveRecord::Base
+  end
+
+  class ApplicationSetting < ActiveRecord::Base
+  end
+
+  def up
+    setting = ApplicationSetting.first
+    number_of_replicas = setting&.elasticsearch_replicas || 1
+    number_of_shards = setting&.elasticsearch_shards || 5
+
+    return if ElasticIndexSetting.exists?(alias_name: ALIAS_NAME)
+
+    ElasticIndexSetting.create!(
+      alias_name: ALIAS_NAME,
+      number_of_replicas: number_of_replicas,
+      number_of_shards: number_of_shards
+    )
+  end
+
+  def down
+    ElasticIndexSetting.where(alias_name: ALIAS_NAME).delete_all
+  end
+end

db/schema_migrations/20210317100520

+54c701451c305ffdead2a9019cf07adae835c5873025caa1f32169f5ae83bf5d
\ No newline at end of file

db/schema_migrations/20210324131727

+e0fab4d950a5be032f823160b1805c44262f9e3d233dc76cd108483a5b92896b
\ No newline at end of file

db/structure.sql

@@ -12270,6 +12270,25 @@ CREATE SEQUENCE draft_notes_id_seq
 
 ALTER SEQUENCE draft_notes_id_seq OWNED BY draft_notes.id;
 
+CREATE TABLE elastic_index_settings (
+    id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    number_of_replicas smallint DEFAULT 1 NOT NULL,
+    number_of_shards smallint DEFAULT 5 NOT NULL,
+    alias_name text NOT NULL,
+    CONSTRAINT check_c30005c325 CHECK ((char_length(alias_name) <= 255))
+);
+
+CREATE SEQUENCE elastic_index_settings_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE elastic_index_settings_id_seq OWNED BY elastic_index_settings.id;
+
 CREATE TABLE elastic_reindexing_subtasks (
     id bigint NOT NULL,
     elastic_reindexing_task_id bigint NOT NULL,
@@ -19276,6 +19295,8 @@ ALTER TABLE ONLY dora_daily_metrics ALTER COLUMN id SET DEFAULT nextval('dora_da
 
 ALTER TABLE ONLY draft_notes ALTER COLUMN id SET DEFAULT nextval('draft_notes_id_seq'::regclass);
 
+ALTER TABLE ONLY elastic_index_settings ALTER COLUMN id SET DEFAULT nextval('elastic_index_settings_id_seq'::regclass);
+
 ALTER TABLE ONLY elastic_reindexing_subtasks ALTER COLUMN id SET DEFAULT nextval('elastic_reindexing_subtasks_id_seq'::regclass);
 
 ALTER TABLE ONLY elastic_reindexing_tasks ALTER COLUMN id SET DEFAULT nextval('elastic_reindexing_tasks_id_seq'::regclass);
@@ -20511,6 +20532,9 @@ ALTER TABLE ONLY dora_daily_metrics
 ALTER TABLE ONLY draft_notes
     ADD CONSTRAINT draft_notes_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY elastic_index_settings
+    ADD CONSTRAINT elastic_index_settings_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY elastic_reindexing_subtasks
     ADD CONSTRAINT elastic_reindexing_subtasks_pkey PRIMARY KEY (id);
 
@@ -22470,6 +22494,8 @@ CREATE INDEX index_draft_notes_on_discussion_id ON draft_notes USING btree (disc
 
 CREATE INDEX index_draft_notes_on_merge_request_id ON draft_notes USING btree (merge_request_id);
 
+CREATE UNIQUE INDEX index_elastic_index_settings_on_alias_name ON elastic_index_settings USING btree (alias_name);
+
 CREATE INDEX index_elastic_reindexing_subtasks_on_elastic_reindexing_task_id ON elastic_reindexing_subtasks USING btree (elastic_reindexing_task_id);
 
 CREATE UNIQUE INDEX index_elastic_reindexing_tasks_on_in_progress ON elastic_reindexing_tasks USING btree (in_progress) WHERE in_progress;

ee/app/models/ee/application_setting.rb

@@ -36,18 +36,10 @@ module EE
                 presence: true,
                 numericality: { only_integer: true, greater_than_or_equal_to: 0 }
 
-      validates :elasticsearch_shards,
-                presence: true,
-                numericality: { only_integer: true, greater_than: 0 }
-
       validates :deletion_adjourned_period,
                 presence: true,
                 numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 90 }
 
-      validates :elasticsearch_replicas,
-                presence: true,
-                numericality: { only_integer: true, greater_than_or_equal_to: 0 }
-
       validates :elasticsearch_max_bulk_size_mb,
                 presence: true,
                 numericality: { only_integer: true, greater_than: 0 }
@@ -143,8 +135,6 @@ module EE
           elasticsearch_indexed_file_size_limit_kb: 1024, # 1 MiB (units in KiB)
           elasticsearch_max_bulk_concurrency: 10,
           elasticsearch_max_bulk_size_bytes: 10.megabytes,
-          elasticsearch_replicas: 1,
-          elasticsearch_shards: 5,
           elasticsearch_url: ENV['ELASTIC_URL'] || 'http://localhost:9200',
           elasticsearch_client_request_timeout: 0,
           elasticsearch_analyzers_smartcn_enabled: false,
@@ -185,6 +175,14 @@ module EE
       ElasticsearchIndexedProject.target_ids
     end
 
+    def elasticsearch_shards
+      Elastic::IndexSetting.number_of_shards
+    end
+
+    def elasticsearch_replicas
+      Elastic::IndexSetting.number_of_replicas
+    end
+
     def elasticsearch_indexes_project?(project)
       return false unless elasticsearch_indexing?
       return true unless elasticsearch_limit_indexing?

ee/app/models/elastic/index_setting.rb

+# frozen_string_literal: true
+
+module Elastic
+  class IndexSetting < ApplicationRecord
+    self.table_name = 'elastic_index_settings'
+
+    validates :alias_name, uniqueness: true, length: { maximum: 255 }
+    validates :number_of_replicas, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+    validates :number_of_shards, presence: true, numericality: { only_integer: true, greater_than: 0 }
+
+    class << self
+      def [](alias_name)
+        safe_find_or_create_by(alias_name: alias_name)
+      end
+
+      def default
+        self[Elastic::Latest::Config.index_name]
+      end
+
+      def number_of_replicas
+        default.number_of_replicas
+      end
+
+      def number_of_shards
+        default.number_of_shards
+      end
+    end
+  end
+end

ee/app/services/ee/application_settings/update_service.rb

@@ -16,6 +16,9 @@ module EE
           params[:maintenance_mode_message] = nil
         end
 
+        elasticsearch_shards = params.delete(:elasticsearch_shards)
+        elasticsearch_replicas = params.delete(:elasticsearch_replicas)
+
         elasticsearch_namespace_ids = params.delete(:elasticsearch_namespace_ids)
         elasticsearch_project_ids = params.delete(:elasticsearch_project_ids)
 
@@ -23,6 +26,7 @@ module EE
           find_or_create_elasticsearch_index
           update_elasticsearch_containers(ElasticsearchIndexedNamespace, elasticsearch_namespace_ids)
           update_elasticsearch_containers(ElasticsearchIndexedProject, elasticsearch_project_ids)
+          update_elasticsearch_index_settings(number_of_replicas: elasticsearch_replicas, number_of_shards: elasticsearch_shards)
         end
 
         result
@@ -44,6 +48,15 @@ module EE
         new_container_ids.each { |id| klass.create!(klass.target_attr_name => id) }
       end
 
+      def update_elasticsearch_index_settings(number_of_replicas:, number_of_shards:)
+        return if number_of_replicas.nil? && number_of_shards.nil?
+
+        Elastic::IndexSetting.default.update!(
+          number_of_replicas: number_of_replicas.to_i,
+          number_of_shards: number_of_shards.to_i
+        )
+      end
+
       private
 
       def should_auto_approve_blocked_users?

ee/app/services/elastic/cluster_reindexing_service.rb

@@ -35,10 +35,10 @@ module Elastic
       [elastic_helper.target_name] + elastic_helper.standalone_indices_proxies.map(&:index_name)
     end
 
-    def default_index_options(index_name)
+    def default_index_options(alias_name:, index_name:)
       {
         refresh_interval: elastic_helper.get_settings(index_name: index_name).dig('refresh_interval'), # Use existing setting or nil for default
-        number_of_replicas: Gitlab::CurrentSettings.elasticsearch_replicas,
+        number_of_replicas: Elastic::IndexSetting[alias_name].number_of_replicas,
         translog: { durability: 'request' }
       }
     end
@@ -142,7 +142,10 @@ module Elastic
 
     def apply_default_index_options
       current_task.subtasks.each do |subtask|
-        elastic_helper.update_settings(index_name: subtask.index_name_to, settings: default_index_options(subtask.index_name_from))
+        elastic_helper.update_settings(
+          index_name: subtask.index_name_to,
+          settings: default_index_options(alias_name: subtask.alias_name, index_name: subtask.index_name_from)
+        )
       end
     end
 

ee/app/views/admin/application_settings/_elasticsearch_form.html.haml

@@ -74,14 +74,14 @@
 
             .form-group
               = f.label :elasticsearch_shards, _('Number of Elasticsearch shards'), class: 'label-bold'
-              = f.number_field :elasticsearch_shards, value: @application_setting.elasticsearch_shards, class: 'form-control gl-form-input'
+              = f.number_field :elasticsearch_shards, value: Elastic::IndexSetting.number_of_shards, class: 'form-control gl-form-input'
               .form-text.gl-text-gray-600.gl-mt-0
                 = _('How many shards to split the Elasticsearch index over.')
                 = recreate_index_text
 
             .form-group
               = f.label :elasticsearch_replicas, _('Number of Elasticsearch replicas'), class: 'label-bold'
-              = f.number_field :elasticsearch_replicas, value: @application_setting.elasticsearch_replicas, class: 'form-control gl-form-input'
+              = f.number_field :elasticsearch_replicas, value: Elastic::IndexSetting.number_of_replicas, class: 'form-control gl-form-input'
               .form-text.gl-text-gray-600.gl-mt-0
                 = _('How many replicas each Elasticsearch shard has.')
                 = recreate_index_text

ee/lib/elastic/latest/config.rb

@@ -23,8 +23,8 @@ module Elastic
 
       settings \
         index: {
-          number_of_shards: Elastic::AsJSON.new { Gitlab::CurrentSettings.elasticsearch_shards },
-          number_of_replicas: Elastic::AsJSON.new { Gitlab::CurrentSettings.elasticsearch_replicas },
+          number_of_shards: Elastic::AsJSON.new { Elastic::IndexSetting.default.number_of_shards },
+          number_of_replicas: Elastic::AsJSON.new { Elastic::IndexSetting.default.number_of_replicas },
           highlight: {
             # `highlight.max_analyzed_offset` is technically not measured in
             # bytes, but rather in characters. Since this is an uppper bound on

ee/spec/factories/elastic/index_setting.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :elastic_index_setting, class: 'Elastic::IndexSetting' do
+    sequence(:alias_name) { |n| "alias_name_#{n}" }
+  end
+end

ee/spec/models/application_setting_spec.rb

@@ -29,18 +29,6 @@ RSpec.describe ApplicationSetting do
     it { is_expected.not_to allow_value(subject.mirror_max_capacity + 1).for(:mirror_capacity_threshold) }
     it { is_expected.to allow_value(nil).for(:custom_project_templates_group_id) }
 
-    it { is_expected.to allow_value(10).for(:elasticsearch_shards) }
-    it { is_expected.not_to allow_value(nil).for(:elasticsearch_shards) }
-    it { is_expected.not_to allow_value(0).for(:elasticsearch_shards) }
-    it { is_expected.not_to allow_value(1.1).for(:elasticsearch_shards) }
-    it { is_expected.not_to allow_value(-1).for(:elasticsearch_shards) }
-
-    it { is_expected.to allow_value(10).for(:elasticsearch_replicas) }
-    it { is_expected.to allow_value(0).for(:elasticsearch_replicas) }
-    it { is_expected.not_to allow_value(nil).for(:elasticsearch_replicas) }
-    it { is_expected.not_to allow_value(1.1).for(:elasticsearch_replicas) }
-    it { is_expected.not_to allow_value(-1).for(:elasticsearch_replicas) }
-
     it { is_expected.to allow_value(10).for(:elasticsearch_indexed_file_size_limit_kb) }
     it { is_expected.not_to allow_value(0).for(:elasticsearch_indexed_file_size_limit_kb) }
     it { is_expected.not_to allow_value(nil).for(:elasticsearch_indexed_file_size_limit_kb) }

ee/spec/models/elastic/index_setting_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Elastic::IndexSetting do
+  subject(:setting) { described_class.default }
+
+  describe 'validations' do
+    it { is_expected.to allow_value(10).for(:number_of_shards) }
+    it { is_expected.not_to allow_value(nil).for(:number_of_shards) }
+    it { is_expected.not_to allow_value(0).for(:number_of_shards) }
+    it { is_expected.not_to allow_value(1.1).for(:number_of_shards) }
+    it { is_expected.not_to allow_value(-1).for(:number_of_shards) }
+
+    it { is_expected.to allow_value(10).for(:number_of_replicas) }
+    it { is_expected.to allow_value(0).for(:number_of_replicas) }
+    it { is_expected.not_to allow_value(nil).for(:number_of_replicas) }
+    it { is_expected.not_to allow_value(1.1).for(:number_of_replicas) }
+    it { is_expected.not_to allow_value(-1).for(:number_of_replicas) }
+
+    it { is_expected.to allow_value('a').for(:alias_name) }
+    it { is_expected.not_to allow_value('a' * 256).for(:alias_name) }
+  end
+
+  describe '.[]' do
+    it 'returns existing record' do
+      record = create(:elastic_index_setting)
+
+      expect(described_class[record.alias_name]).to eq record
+    end
+
+    it 'creates a new record' do
+      expect { described_class['new_alias'] }.to change { described_class.count }.by(1)
+    end
+  end
+
+  describe '.default' do
+    it 'returns index_setting record for the default index' do
+      index_name = 'default_index_name'
+      allow(Elastic::Latest::Config).to receive(:index_name).and_return(index_name)
+
+      expect(described_class.default.alias_name).to eq(index_name)
+    end
+  end
+
+  describe '.number_of_replicas' do
+    it 'returns default number of replicas' do
+      allow(described_class).to receive(:default).and_return(double(described_class, number_of_replicas: 2))
+
+      expect(described_class.number_of_replicas).to eq(2)
+    end
+  end
+
+  describe '.number_of_shards' do
+    it 'returns default number of shards' do
+      allow(described_class).to receive(:default).and_return(double(described_class, number_of_shards: 8))
+
+      expect(described_class.number_of_shards).to eq(8)
+    end
+  end
+end

ee/spec/services/elastic/cluster_reindexing_service_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe Elastic::ClusterReindexingService, :elastic do
     let(:expected_default_settings) do
       {
         refresh_interval: refresh_interval,
-        number_of_replicas: Gitlab::CurrentSettings.elasticsearch_replicas,
+        number_of_replicas: Elastic::IndexSetting[subtask.alias_name].number_of_replicas,
         translog: { durability: 'request' }
       }
     end

spec/migrations/migrate_elastic_index_settings_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require Rails.root.join('db', 'migrate', '20210324131727_migrate_elastic_index_settings.rb')
+
+RSpec.describe MigrateElasticIndexSettings do
+  let(:elastic_index_settings) { table(:elastic_index_settings) }
+  let(:application_settings) { table(:application_settings) }
+
+  context 'with application_settings present' do
+    before do
+      application_settings.create!(elasticsearch_replicas: 2, elasticsearch_shards: 15)
+    end
+
+    it 'migrates settings' do
+      migrate!
+
+      settings = elastic_index_settings.all
+
+      expect(settings.size).to eq 1
+
+      setting = settings.first
+
+      expect(setting.number_of_replicas).to eq(2)
+      expect(setting.number_of_shards).to eq(15)
+    end
+  end
+
+  context 'without application_settings present' do
+    it 'migrates settings' do
+      migrate!
+
+      settings = elastic_index_settings.all
+
+      expect(settings.size).to eq 1
+
+      setting = elastic_index_settings.first
+
+      expect(setting.number_of_replicas).to eq(1)
+      expect(setting.number_of_shards).to eq(5)
+    end
+  end
+end