Merge branch 'ab/reindexing-guard-lingering' into 'master'

Exclude temporary indexes from reindexing

See merge request gitlab-org/gitlab!43992

lib/gitlab/database/postgres_index.rb

@@ -21,6 +21,8 @@ module Gitlab
         limit(how_many).order(Arel.sql('RANDOM()'))
       end
 
+      scope :not_match, ->(regex) { where("name !~ ?", regex)}
+
       def to_s
         name
       end

lib/gitlab/database/reindexing.rb

@@ -10,6 +10,13 @@ module Gitlab
           end
         end
       end
+
+      def self.candidate_indexes
+        Gitlab::Database::PostgresIndex
+          .regular
+          .not_match("^#{ConcurrentReindex::TEMPORARY_INDEX_PREFIX}")
+          .not_match("^#{ConcurrentReindex::REPLACED_INDEX_PREFIX}")
+      end
     end
   end
 end

lib/gitlab/database/reindexing/concurrent_reindex.rb

@@ -24,6 +24,7 @@ module Gitlab
           raise ReindexError, 'UNIQUE indexes are currently not supported' if index.unique?
           raise ReindexError, 'partitioned indexes are currently not supported' if index.partitioned?
           raise ReindexError, 'indexes serving an exclusion constraint are currently not supported' if index.exclusion?
+          raise ReindexError, 'index is a left-over temporary index from a previous reindexing run' if index.name.start_with?(TEMPORARY_INDEX_PREFIX, REPLACED_INDEX_PREFIX)
 
           logger.info "Starting reindex of #{index}"
 

lib/tasks/gitlab/db.rake

@@ -177,7 +177,7 @@ namespace :gitlab do
       indexes = if args[:index_name]
                   Gitlab::Database::PostgresIndex.by_identifier(args[:index_name])
                 else
-                  Gitlab::Database::PostgresIndex.regular.random_few(2)
+                  Gitlab::Database::Reindexing.candidate_indexes.random_few(2)
                 end
 
       Gitlab::Database::Reindexing.perform(indexes)

spec/lib/gitlab/database/postgres_index_spec.rb

@@ -30,7 +30,7 @@ RSpec.describe Gitlab::Database::PostgresIndex do
     end
   end
 
-  describe '#regular' do
+  describe '.regular' do
     it 'only non-unique indexes' do
       expect(described_class.regular).to all(have_attributes(unique: false))
     end
@@ -44,7 +44,17 @@ RSpec.describe Gitlab::Database::PostgresIndex do
     end
   end
 
-  describe '#random_few' do
+  describe '.not_match' do
+    it 'excludes indexes matching the given regex' do
+      expect(described_class.not_match('^bar_k').map(&:name)).to all(match(/^(?!bar_k).*/))
+    end
+
+    it 'matches indexes without this prefix regex' do
+      expect(described_class.not_match('^bar_k')).not_to be_empty
+    end
+  end
+
+  describe '.random_few' do
     it 'limits to two records by default' do
       expect(described_class.random_few(2).size).to eq(2)
     end

spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb

@@ -58,6 +58,28 @@ RSpec.describe Gitlab::Database::Reindexing::ConcurrentReindex, '#perform' do
     end
   end
 
+  context 'when the index is a lingering temporary index from a previous reindexing run' do
+    context 'with the temporary index prefix' do
+      let(:index_name) { 'tmp_reindex_something' }
+
+      it 'raises an error' do
+        expect do
+          subject.perform
+        end.to raise_error(described_class::ReindexError, /left-over temporary index/)
+      end
+    end
+
+    context 'with the replaced index prefix' do
+      let(:index_name) { 'old_reindex_something' }
+
+      it 'raises an error' do
+        expect do
+          subject.perform
+        end.to raise_error(described_class::ReindexError, /left-over temporary index/)
+      end
+    end
+  end
+
   context 'replacing the original index with a rebuilt copy' do
     let(:replacement_name) { 'tmp_reindex_42' }
     let(:replaced_name) { 'old_reindex_42' }

spec/lib/gitlab/database/reindexing_spec.rb

@@ -52,4 +52,15 @@ RSpec.describe Gitlab::Database::Reindexing do
       it_behaves_like 'reindexing'
     end
   end
+
+  describe '.candidate_indexes' do
+    subject { described_class.candidate_indexes }
+
+    it 'retrieves regular indexes that are no left-overs from previous runs' do
+      result = double
+      expect(Gitlab::Database::PostgresIndex).to receive_message_chain('regular.not_match.not_match').with(no_args).with('^tmp_reindex_').with('^old_reindex_').and_return(result)
+
+      expect(subject).to eq(result)
+    end
+  end
 end

spec/tasks/gitlab/db_rake_spec.rb

@@ -170,7 +170,7 @@ RSpec.describe 'gitlab:db namespace rake task' do
 
     context 'when no index_name is given' do
       it 'rebuilds a random number of large indexes' do
-        expect(Gitlab::Database::PostgresIndex).to receive_message_chain('regular.random_few').and_return(indexes)
+        expect(Gitlab::Database::Reindexing).to receive_message_chain('candidate_indexes.random_few').and_return(indexes)
         expect(Gitlab::Database::Reindexing).to receive(:perform).with(indexes)
 
         run_rake_task('gitlab:db:reindex')