Cleanup leftover indexes from reindexing

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/332475

lib/gitlab/database/reindexing.rb

@@ -8,12 +8,37 @@ module Gitlab
 
       SUPPORTED_TYPES = %w(btree gist).freeze
 
+      # When dropping an index, we acquire a SHARE UPDATE EXCLUSIVE lock,
+      # which only conflicts with DDL and vacuum. We therefore execute this with a rather
+      # high lock timeout and a long pause in between retries. This is an alternative to
+      # setting a high statement timeout, which would lead to a long running query with effects
+      # on e.g. vacuum.
+      REMOVE_INDEX_RETRY_CONFIG = [[1.minute, 9.minutes]] * 30
+
       # candidate_indexes: Array of Gitlab::Database::PostgresIndex
       def self.perform(candidate_indexes, how_many: DEFAULT_INDEXES_PER_INVOCATION)
         IndexSelection.new(candidate_indexes).take(how_many).each do |index|
           Coordinator.new(index).perform
         end
       end
+
+      def self.cleanup_leftovers!
+        PostgresIndex.reindexing_leftovers.each do |index|
+          Gitlab::AppLogger.info("Removing index #{index.identifier} which is a leftover, temporary index from previous reindexing activity")
+
+          retries = Gitlab::Database::WithLockRetriesOutsideTransaction.new(
+            timing_configuration: REMOVE_INDEX_RETRY_CONFIG,
+            klass: self.class,
+            logger: Gitlab::AppLogger
+          )
+
+          retries.run(raise_on_exhaustion: false) do
+            ApplicationRecord.connection.tap do |conn|
+              conn.execute("DROP INDEX CONCURRENTLY IF EXISTS #{conn.quote_table_name(index.schema)}.#{conn.quote_table_name(index.name)}")
+            end
+          end
+        end
+      end
     end
   end
 end

lib/gitlab/database/reindexing/reindex_concurrently.rb

@@ -11,13 +11,6 @@ module Gitlab
         STATEMENT_TIMEOUT = 9.hours
         PG_MAX_INDEX_NAME_LENGTH = 63
 
-        # When dropping an index, we acquire a SHARE UPDATE EXCLUSIVE lock,
-        # which only conflicts with DDL and vacuum. We therefore execute this with a rather
-        # high lock timeout and a long pause in between retries. This is an alternative to
-        # setting a high statement timeout, which would lead to a long running query with effects
-        # on e.g. vacuum.
-        REMOVE_INDEX_RETRY_CONFIG = [[1.minute, 9.minutes]] * 30
-
         attr_reader :index, :logger
 
         def initialize(index, logger: Gitlab::AppLogger)

lib/tasks/gitlab/db.rake

@@ -173,6 +173,9 @@ namespace :gitlab do
 
       ActiveRecord::Base.logger = Logger.new($stdout) if Gitlab::Utils.to_boolean(ENV['LOG_QUERIES_TO_CONSOLE'], default: false)
 
+      # Cleanup leftover temporary indexes from previous, possibly aborted runs (if any)
+      Gitlab::Database::Reindexing.cleanup_leftovers!
+
       Gitlab::Database::Reindexing.perform(indexes)
     rescue StandardError => e
       Gitlab::AppLogger.error(e)

spec/lib/gitlab/database/reindexing_spec.rb

@@ -25,4 +25,32 @@ RSpec.describe Gitlab::Database::Reindexing do
       subject
     end
   end
+
+  describe '.cleanup_leftovers!' do
+    subject { described_class.cleanup_leftovers! }
+
+    before do
+      ApplicationRecord.connection.execute(<<~SQL)
+        CREATE INDEX foobar_ccnew ON users (id);
+        CREATE INDEX foobar_ccnew1 ON users (id);
+      SQL
+    end
+
+    it 'drops both leftover indexes' do
+      expect_query("SET lock_timeout TO '60000ms'")
+      expect_query("DROP INDEX CONCURRENTLY IF EXISTS \"public\".\"foobar_ccnew\"")
+      expect_query("RESET idle_in_transaction_session_timeout; RESET lock_timeout")
+      expect_query("SET lock_timeout TO '60000ms'")
+      expect_query("DROP INDEX CONCURRENTLY IF EXISTS \"public\".\"foobar_ccnew1\"")
+      expect_query("RESET idle_in_transaction_session_timeout; RESET lock_timeout")
+
+      subject
+    end
+
+    def expect_query(sql)
+      expect(ApplicationRecord.connection).to receive(:execute).ordered.with(sql).and_wrap_original do |method, sql|
+        method.call(sql.sub(/CONCURRENTLY/, ''))
+      end
+    end
+  end
 end

spec/tasks/gitlab/db_rake_spec.rb

@@ -201,6 +201,12 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do
     let(:reindex) { double('reindex') }
     let(:indexes) { double('indexes') }
 
+    it 'cleans up any leftover indexes' do
+      expect(Gitlab::Database::Reindexing).to receive(:cleanup_leftovers!)
+
+      run_rake_task('gitlab:db:reindex')
+    end
+
     context 'when no index_name is given' do
       it 'uses all candidate indexes' do
         expect(Gitlab::Database::PostgresIndex).to receive(:reindexing_support).and_return(indexes)