Merge branch '33681-api' into 'master'

Implement a percentage based rollout for ElasticSearch

See merge request gitlab-org/gitlab!22240

changelogs/unreleased/33681-api.yml

+---
+title: Add API for rollout Elasticsearch per plan level
+merge_request: 22240
+author:
+type: added

config/sidekiq_queues.yml

@@ -117,6 +117,7 @@
   - [elastic_full_index, 1]
   - [elastic_commit_indexer, 1]
   - [elastic_namespace_indexer, 1]
+  - [elastic_namespace_rollout, 1]
   - [export_csv, 1]
   - [incident_management, 2]
   - [jira_connect, 1]

db/migrate/20191225071320_add_index_to_elasticsearch_indexed_namespaces.rb

+# frozen_string_literal: true
+
+class AddIndexToElasticsearchIndexedNamespaces < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:elasticsearch_indexed_namespaces, :created_at)
+  end
+
+  def down
+    remove_concurrent_index(:elasticsearch_indexed_namespaces, :created_at)
+  end
+end

db/schema.rb

@@ -1460,6 +1460,7 @@ ActiveRecord::Schema.define(version: 2020_01_06_085831) do
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
     t.integer "namespace_id"
+    t.index ["created_at"], name: "index_elasticsearch_indexed_namespaces_on_created_at"
     t.index ["namespace_id"], name: "index_elasticsearch_indexed_namespaces_on_namespace_id", unique: true
   end
 

ee/app/models/elasticsearch_indexed_namespace.rb

@@ -10,10 +10,58 @@ class ElasticsearchIndexedNamespace < ApplicationRecord
 
   validates :namespace_id, presence: true, uniqueness: true
 
+  scope :namespace_in, -> (namespaces) { where(namespace_id: namespaces) }
+
+  BATCH_OPERATION_SIZE = 1000
+
   def self.target_attr_name
     :namespace_id
   end
 
+  def self.index_first_n_namespaces_of_plan(plan, number_of_namespaces)
+    indexed_namespaces = self.select(:namespace_id)
+    now = Time.now
+
+    ids = GitlabSubscription
+      .with_hosted_plan(plan)
+      .where.not(namespace_id: indexed_namespaces)
+      .order(namespace_id: :asc)
+      .limit(number_of_namespaces)
+      .pluck(:namespace_id)
+
+    ids.in_groups_of(BATCH_OPERATION_SIZE, false) do |batch_ids|
+      insert_rows = batch_ids.map do |id|
+        # Ensure ordering with incremental created_at,
+        # so rollback can start from the bigger namespace_id
+        now += 1.0e-05.seconds
+        { created_at: now, updated_at: now, namespace_id: id }
+      end
+
+      Gitlab::Database.bulk_insert(table_name, insert_rows)
+
+      jobs = batch_ids.map { |id| [id, :index] }
+
+      ElasticNamespaceIndexerWorker.bulk_perform_async(jobs)
+    end
+  end
+
+  def self.unindex_last_n_namespaces_of_plan(plan, number_of_namespaces)
+    namespaces_under_plan = GitlabSubscription.with_hosted_plan(plan).select(:namespace_id)
+
+    ids = where(namespace: namespaces_under_plan)
+      .order(created_at: :desc)
+      .limit(number_of_namespaces)
+      .pluck(:namespace_id)
+
+    ids.in_groups_of(BATCH_OPERATION_SIZE, false) do |batch_ids|
+      where(namespace_id: batch_ids).delete_all
+
+      jobs = batch_ids.map { |id| [id, :delete] }
+
+      ElasticNamespaceIndexerWorker.bulk_perform_async(jobs)
+    end
+  end
+
   private
 
   def index

ee/app/models/gitlab_subscription.rb

@@ -13,8 +13,12 @@ class GitlabSubscription < ApplicationRecord
 
   delegate :name, :title, to: :hosted_plan, prefix: :plan, allow_nil: true
 
+  scope :with_hosted_plan, -> (plan_name) do
+    joins(:hosted_plan).where(trial: false, 'plans.name' => plan_name)
+  end
+
   scope :with_a_paid_hosted_plan, -> do
-    joins(:hosted_plan).where(trial: false, 'plans.name' => Plan::PAID_HOSTED_PLANS)
+    with_hosted_plan(Plan::PAID_HOSTED_PLANS)
   end
 
   def seats_in_use

ee/app/workers/all_queues.yml

@@ -70,6 +70,7 @@
 - elastic_commit_indexer
 - elastic_indexer
 - elastic_full_index
+- elastic_namespace_rollout
 - export_csv
 - ldap_group_sync
 - new_epic

ee/app/workers/elastic_namespace_rollout_worker.rb

+# frozen_string_literal: true
+
+class ElasticNamespaceRolloutWorker
+  include ApplicationWorker
+
+  feature_category :search
+  sidekiq_options retry: 2
+
+  ROLLOUT = 'rollout'
+  ROLLBACK = 'rollback'
+
+  # @param plan [String] which plan the rollout is scoped to
+  # @param percentage [Integer]
+  # @param mode [ROLLOUT, ROLLBACK] determine whether to rollout or rollback
+  def perform(plan, percentage, mode)
+    total_with_plan = GitlabSubscription.with_hosted_plan(plan).count
+
+    expected_count = total_with_plan * (percentage / 100.0)
+
+    current_count = ElasticsearchIndexedNamespace
+      .namespace_in(GitlabSubscription.with_hosted_plan(plan).select(:namespace_id))
+      .count
+
+    case mode
+    when ROLLOUT
+      rollout(plan, expected_count, current_count)
+    when ROLLBACK
+      rollback(plan, expected_count, current_count)
+    end
+  end
+
+  private
+
+  def rollout(plan, expected_count, current_count)
+    required_count_changes = [expected_count - current_count, 0].max
+
+    logger.info(message: 'rollout_elasticsearch_indexed_namespaces', changes: required_count_changes, expected_count: expected_count, current_count: current_count, plan: plan)
+
+    if required_count_changes > 0
+      ElasticsearchIndexedNamespace.index_first_n_namespaces_of_plan(plan, required_count_changes)
+    end
+  end
+
+  def rollback(plan, expected_count, current_count)
+    required_count_changes = [current_count - expected_count, 0].max
+
+    logger.info(message: 'rollback_elasticsearch_indexed_namespaces', changes: required_count_changes, expected_count: expected_count, current_count: current_count, plan: plan)
+
+    if required_count_changes > 0
+      ElasticsearchIndexedNamespace.unindex_last_n_namespaces_of_plan(plan, required_count_changes)
+    end
+  end
+
+  def logger
+    @logger ||= ::Gitlab::Elasticsearch::Logger.build
+  end
+end

ee/lib/api/elasticsearch_indexed_namespaces.rb

+# frozen_string_literal: true
+
+module API
+  class ElasticsearchIndexedNamespaces < Grape::API
+    before { authenticated_as_admin! }
+
+    resource :elasticsearch_indexed_namespaces do
+      desc 'Rollout namespaces to be indexed up to n%' do
+        detail <<~END
+          This feature was introduced in GitLab 12.7.
+
+          This will only ever increase the number of indexed namespaces. Providing a value lower than the current rolled out percentage will have no effect.
+
+          This percentage is never persisted but is used to calculate the number of new namespaces to rollout.
+
+          If the same percentage is applied again at a later time, due to possible new namespaces being created during the period, some of them will also be indexed. Therefore you may expect that setting this to 10%, then waiting a month and setting to 10% again will trigger new namespaces to be added (i.e. 10% of the number of newly created namespaces in the last month within the given plan).
+        END
+      end
+      params do
+        requires :percentage, type: Integer, values: 0..100
+        requires :plan, type: String, values: Plan::ALL_HOSTED_PLANS
+      end
+      put 'rollout' do
+        ElasticNamespaceRolloutWorker.perform_async(params[:plan], params[:percentage], ElasticNamespaceRolloutWorker::ROLLOUT)
+      end
+
+      desc 'Rollback namespaces to be indexed down to n%' do
+        detail <<~END
+          This feature was introduced in GitLab 12.7.
+
+          This will only ever decrease the number of indexed namespaces. Providing a value higher than the current rolled out percentage will have no effect.
+
+          This percentage is never persisted but is used to calculate the number of namespaces to rollback.
+        END
+      end
+      params do
+        requires :percentage, type: Integer, values: 0..100
+        requires :plan, type: String, values: Plan::ALL_HOSTED_PLANS
+      end
+      put 'rollback' do
+        ElasticNamespaceRolloutWorker.perform_async(params[:plan], params[:percentage], ElasticNamespaceRolloutWorker::ROLLBACK)
+      end
+    end
+  end
+end

ee/lib/ee/api/api.rb

@@ -18,6 +18,7 @@ module EE
         mount ::API::EpicIssues
         mount ::API::EpicLinks
         mount ::API::Epics
+        mount ::API::ElasticsearchIndexedNamespaces
         mount ::API::FeatureFlags
         mount ::API::FeatureFlagScopes
         mount ::API::ContainerRegistryEvent

ee/spec/models/elasticsearch_indexed_namespace_spec.rb

@@ -7,6 +7,16 @@ describe ElasticsearchIndexedNamespace do
     stub_ee_application_setting(elasticsearch_indexing: true)
   end
 
+  describe 'scope' do
+    describe '.namespace_in' do
+      let(:records) { create_list(:elasticsearch_indexed_namespace, 3) }
+
+      it 'returns records of the ids' do
+        expect(described_class.namespace_in(records.last(2).map(&:id))).to eq(records.last(2))
+      end
+    end
+  end
+
   it_behaves_like 'an elasticsearch indexed container' do
     let(:container) { :elasticsearch_indexed_namespace }
     let(:attribute) { :namespace_id }
@@ -17,4 +27,79 @@ describe ElasticsearchIndexedNamespace do
       expect(ElasticNamespaceIndexerWorker).to receive(:perform_async).with(subject.namespace_id, :delete)
     end
   end
+
+  context 'with plans' do
+    Plan::PAID_HOSTED_PLANS.each do |plan|
+      plan_factory = "#{plan}_plan"
+      let_it_be(plan_factory) { create(plan_factory) }
+    end
+
+    let_it_be(:namespaces) { create_list(:namespace, 3) }
+    let_it_be(:subscription1) { create(:gitlab_subscription, namespace: namespaces[2]) }
+    let_it_be(:subscription2) { create(:gitlab_subscription, namespace: namespaces[0]) }
+    let_it_be(:subscription3) { create(:gitlab_subscription, :silver, namespace: namespaces[1]) }
+
+    before do
+      stub_ee_application_setting(elasticsearch_indexing: false)
+    end
+
+    def get_indexed_namespaces
+      described_class.order(:created_at).pluck(:namespace_id)
+    end
+
+    def expect_queue_to_contain(*args)
+      expect(ElasticNamespaceIndexerWorker.jobs).to include(
+        hash_including("args" => args)
+      )
+    end
+
+    describe '.index_first_n_namespaces_of_plan' do
+      it 'creates records, scoped by plan and ordered by namespace id' do
+        ids = namespaces.map(&:id)
+
+        described_class.index_first_n_namespaces_of_plan('gold', 1)
+
+        expect(get_indexed_namespaces).to eq([ids[0]])
+        expect_queue_to_contain(ids[0], "index")
+
+        described_class.index_first_n_namespaces_of_plan('gold', 2)
+
+        expect(get_indexed_namespaces).to eq([ids[0], ids[2]])
+        expect_queue_to_contain(ids[2], "index")
+
+        described_class.index_first_n_namespaces_of_plan('silver', 1)
+
+        expect(get_indexed_namespaces).to eq([ids[0], ids[2], ids[1]])
+        expect_queue_to_contain(ids[1], "index")
+      end
+    end
+
+    describe '.unindex_last_n_namespaces_of_plan' do
+      before do
+        described_class.index_first_n_namespaces_of_plan('gold', 2)
+        described_class.index_first_n_namespaces_of_plan('silver', 1)
+      end
+
+      it 'creates records, scoped by plan and ordered by namespace id' do
+        ids = namespaces.map(&:id)
+
+        expect(get_indexed_namespaces).to contain_exactly(ids[0], ids[2], ids[1])
+
+        described_class.unindex_last_n_namespaces_of_plan('gold', 1)
+
+        expect(get_indexed_namespaces).to contain_exactly(ids[0], ids[1])
+        expect_queue_to_contain(ids[2], "delete")
+
+        described_class.unindex_last_n_namespaces_of_plan('silver', 1)
+
+        expect(get_indexed_namespaces).to contain_exactly(ids[0])
+        expect_queue_to_contain(ids[1], "delete")
+
+        described_class.unindex_last_n_namespaces_of_plan('gold', 1)
+
+        expect(get_indexed_namespaces).to be_empty
+        expect_queue_to_contain(ids[0], "delete")
+      end
+    end
+  end
 end

ee/spec/models/gitlab_subscription_spec.rb

@@ -3,6 +3,10 @@
 require 'spec_helper'
 
 describe GitlabSubscription do
+  %i[free_plan bronze_plan silver_plan gold_plan early_adopter_plan].each do |plan|
+    let_it_be(plan) { create(plan) }
+  end
+
   describe 'default values' do
     it do
       Timecop.freeze(Date.today + 30) do
@@ -26,6 +30,23 @@ describe GitlabSubscription do
     it { is_expected.to belong_to(:hosted_plan) }
   end
 
+  describe 'scopes' do
+    describe '.with_hosted_plan' do
+      let!(:gold_subscription) { create(:gitlab_subscription, hosted_plan: gold_plan) }
+      let!(:silver_subscription) { create(:gitlab_subscription, hosted_plan: silver_plan) }
+      let!(:early_adopter_subscription) { create(:gitlab_subscription, hosted_plan: early_adopter_plan) }
+
+      let!(:trial_subscription) { create(:gitlab_subscription, hosted_plan: gold_plan, trial: true) }
+
+      it 'scopes to the plan' do
+        expect(described_class.with_hosted_plan('gold')).to contain_exactly(gold_subscription)
+        expect(described_class.with_hosted_plan('silver')).to contain_exactly(silver_subscription)
+        expect(described_class.with_hosted_plan('early_adopter')).to contain_exactly(early_adopter_subscription)
+        expect(described_class.with_hosted_plan('bronze')).to be_empty
+      end
+    end
+  end
+
   describe '#seats_in_use' do
     let!(:user_1)         { create(:user) }
     let!(:user_2)         { create(:user) }
@@ -38,12 +59,6 @@ describe GitlabSubscription do
     let!(:subgroup_2)          { create(:group, parent: group) }
     let!(:gitlab_subscription) { create(:gitlab_subscription, namespace: group) }
 
-    before do
-      %i[free_plan bronze_plan silver_plan gold_plan].each do |plan|
-        create(plan)
-      end
-    end
-
     it 'returns count of members' do
       group.add_developer(user_1)
 
@@ -116,8 +131,6 @@ describe GitlabSubscription do
   end
 
   describe '#seats_owed' do
-    let!(:bronze_plan)         { create(:bronze_plan) }
-    let!(:early_adopter_plan)  { create(:early_adopter_plan) }
     let!(:gitlab_subscription) { create(:gitlab_subscription, subscription_attrs) }
 
     before do
@@ -262,8 +275,7 @@ describe GitlabSubscription do
     context 'after_destroy_commit' do
       it 'logs previous state to gitlab subscription history' do
         group = create(:group)
-        plan = create(:bronze_plan)
-        subject.update! max_seats_used: 37, seats: 11, namespace: group, hosted_plan: plan
+        subject.update! max_seats_used: 37, seats: 11, namespace: group, hosted_plan: bronze_plan
         db_created_at = described_class.last.created_at
 
         subject.destroy!
@@ -275,7 +287,7 @@ describe GitlabSubscription do
           'max_seats_used' => 37,
           'seats' => 11,
           'namespace_id' => group.id,
-          'hosted_plan_id' => plan.id,
+          'hosted_plan_id' => bronze_plan.id,
           'gitlab_subscription_created_at' => db_created_at
         )
       end

ee/spec/requests/api/elasticsearch_indexed_namespaces_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::ElasticsearchIndexedNamespaces do
+  let_it_be(:admin) { create(:admin) }
+  let_it_be(:non_admin) { create(:user) }
+
+  shared_examples 'rollout related' do
+    context 'when parameters are incorrect' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:percentage, :plan) do
+        -1    | 'gold'
+        101   | 'gold'
+        nil   | 'gold'
+        1     | nil
+        1     | 'foobar'
+      end
+
+      with_them do
+        it 'errs' do
+          put api(path, admin), params: { plan: plan, percentage: percentage }
+          expect(response).to have_gitlab_http_status(400)
+        end
+      end
+    end
+
+    it 'prohibits non-admin' do
+      put api(path, non_admin), params: { plan: 'gold', percentage: 50 }
+      expect(response).to have_gitlab_http_status(403)
+    end
+  end
+
+  describe 'PUT /elasticsearch_indexed_namespaces/rollout' do
+    let(:path) { "/elasticsearch_indexed_namespaces/rollout" }
+
+    include_context 'rollout related'
+
+    it 'invokes ElasticNamespaceRolloutWorker rollout' do
+      expect(ElasticNamespaceRolloutWorker).to receive(:perform_async).with('gold', 50, ElasticNamespaceRolloutWorker::ROLLOUT)
+
+      put api(path, admin), params: { plan: 'gold', percentage: 50 }
+
+      expect(response).to have_gitlab_http_status(200)
+    end
+  end
+
+  describe 'PUT /elasticsearch_indexed_namespaces/rollback' do
+    let(:path) { "/elasticsearch_indexed_namespaces/rollback" }
+
+    include_context 'rollout related'
+
+    it 'invokes ElasticNamespaceRolloutWorker rollback' do
+      expect(ElasticNamespaceRolloutWorker).to receive(:perform_async).with('gold', 50, ElasticNamespaceRolloutWorker::ROLLBACK)
+
+      put api(path, admin), params: { plan: 'gold', percentage: 50 }
+
+      expect(response).to have_gitlab_http_status(200)
+    end
+  end
+end

ee/spec/workers/elastic_namespace_rollout_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ElasticNamespaceRolloutWorker do
+  ROLLOUT = described_class::ROLLOUT
+  ROLLBACK = described_class::ROLLBACK
+
+  Plan::PAID_HOSTED_PLANS.each do |plan|
+    plan_factory = "#{plan}_plan"
+    let_it_be(plan_factory) { create(plan_factory) }
+  end
+
+  before_all do
+    Plan::PAID_HOSTED_PLANS.each do |plan|
+      4.times do
+        create(:gitlab_subscription, hosted_plan: public_send("#{plan}_plan"))
+      end
+    end
+  end
+
+  def expect_percentage_to_result_in_records(percentage, record_count, mode)
+    subject.perform('gold', percentage, mode)
+
+    namespace_ids = GitlabSubscription
+      .with_hosted_plan('gold')
+      .order(id: :asc)
+      .pluck(:namespace_id)
+
+    expect(
+      ElasticsearchIndexedNamespace.pluck(:namespace_id)
+    ).to contain_exactly(*namespace_ids.first(record_count))
+  end
+
+  def get_namespace_ids(plan, count)
+    GitlabSubscription
+      .with_hosted_plan(plan)
+      .order(id: :asc)
+      .pluck(:namespace_id)
+      .first(count)
+  end
+
+  it 'rolls out and back' do
+    # Rollout
+    expect_percentage_to_result_in_records(0, 0, ROLLOUT)
+    expect_percentage_to_result_in_records(50, 2, ROLLOUT)
+    expect_percentage_to_result_in_records(25, 2, ROLLOUT) # no op
+    expect_percentage_to_result_in_records(100, 4, ROLLOUT)
+
+    # Rollback
+    expect_percentage_to_result_in_records(50, 2, ROLLBACK)
+    expect_percentage_to_result_in_records(75, 2, ROLLBACK) # no op
+    expect_percentage_to_result_in_records(0, 0, ROLLBACK)
+  end
+
+  it 'distinguishes different plans' do
+    # Rollout
+    subject.perform('gold', 50, ROLLOUT)
+    subject.perform('silver', 25, ROLLOUT)
+
+    expect(
+      ElasticsearchIndexedNamespace.pluck(:namespace_id)
+    ).to contain_exactly(
+      *get_namespace_ids(:gold, 2),
+      *get_namespace_ids(:silver, 1)
+    )
+
+    # Rollback
+    subject.perform('gold', 25, ROLLBACK)
+    subject.perform('silver', 0, ROLLBACK)
+
+    expect(
+      ElasticsearchIndexedNamespace.pluck(:namespace_id)
+    ).to contain_exactly(
+      *get_namespace_ids(:gold, 1)
+    )
+  end
+
+  it 'logs' do
+    logger = subject.send(:logger)
+
+    expect(logger).to receive(:info).with(
+      hash_including(
+        message: "rollout_elasticsearch_indexed_namespaces",
+        changes: 3,
+        expected_count: 3,
+        current_count: 0,
+        plan: 'gold'
+      )
+    ).and_call_original
+
+    subject.perform('gold', 75, ROLLOUT)
+
+    expect(logger).to receive(:info).with(
+      hash_including(
+        message: "rollback_elasticsearch_indexed_namespaces",
+        changes: 2,
+        expected_count: 1,
+        current_count: 3,
+        plan: 'gold'
+      )
+    ).and_call_original
+
+    subject.perform('gold', 25, ROLLBACK)
+  end
+end