Consider truncated index names

PG limits index names to 63 chars and has to add a suffix for the
temporary index while reindexing.

lib/gitlab/database/reindexing/reindex_concurrently.rb

@@ -9,6 +9,7 @@ module Gitlab
 
         TEMPORARY_INDEX_PATTERN = '\_ccnew[0-9]*'
         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
@@ -38,13 +39,15 @@ module Gitlab
           # While this has been backpatched, we continue to disable expression indexes until further review.
           raise ReindexError, 'expression indexes are currently not supported' if index.expression?
 
-          with_logging do
-            set_statement_timeout do
-              execute("REINDEX INDEX CONCURRENTLY #{quote_table_name(index.schema)}.#{quote_table_name(index.name)}")
+          begin
+            with_logging do
+              set_statement_timeout do
+                execute("REINDEX INDEX CONCURRENTLY #{quote_table_name(index.schema)}.#{quote_table_name(index.name)}")
+              end
             end
+          ensure
+            cleanup_dangling_indexes
           end
-        ensure
-          cleanup_dangling_indexes
         end
 
         private
@@ -79,8 +82,21 @@ module Gitlab
         end
 
         def cleanup_dangling_indexes
-          Gitlab::Database::PostgresIndex.match("#{Regexp.escape(index.name)}#{TEMPORARY_INDEX_PATTERN}").each do |lingering_index|
-            remove_index(lingering_index)
+          Gitlab::Database::PostgresIndex.match("#{TEMPORARY_INDEX_PATTERN}$").each do |lingering_index|
+            # Example lingering index name: some_index_ccnew1
+
+            # Example prefix: 'some_index'
+            prefix = lingering_index.name.gsub(/#{TEMPORARY_INDEX_PATTERN}/, '')
+
+            # Example suffix: '_ccnew1'
+            suffix = lingering_index.name.match(/#{TEMPORARY_INDEX_PATTERN}/)[0]
+
+            # Only remove if the lingering index name could have been chosen
+            # as a result of a REINDEX operation (considering that PostgreSQL
+            # truncates index names to 63 chars and adds a suffix).
+            if index.name[0...PG_MAX_INDEX_NAME_LENGTH - suffix.length] == prefix
+              remove_index(lingering_index)
+            end
           end
         end
 

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

@@ -8,7 +8,7 @@ RSpec.describe Gitlab::Database::Reindexing::ReindexConcurrently, '#perform' do
   let(:table_name) { '_test_reindex_table' }
   let(:column_name) { '_test_column' }
   let(:index_name) { '_test_reindex_index' }
-  let(:index) { Gitlab::Database::PostgresIndex.by_identifier("public.#{index_name}") }
+  let(:index) { Gitlab::Database::PostgresIndex.by_identifier("public.#{iname(index_name)}") }
   let(:logger) { double('logger', debug: nil, info: nil, error: nil ) }
   let(:connection) { ActiveRecord::Base.connection }
 
@@ -73,33 +73,55 @@ RSpec.describe Gitlab::Database::Reindexing::ReindexConcurrently, '#perform' do
   context 'with dangling indexes matching TEMPORARY_INDEX_PATTERN, i.e. /some\_index\_ccnew(\d)*/' do
     before do
       # dangling indexes
-      connection.execute("CREATE INDEX #{index_name}_ccnew ON #{table_name} (#{column_name})")
-      connection.execute("CREATE INDEX #{index_name}_ccnew2 ON #{table_name} (#{column_name})")
+      connection.execute("CREATE INDEX #{iname(index_name, '_ccnew')} ON #{table_name} (#{column_name})")
+      connection.execute("CREATE INDEX #{iname(index_name, '_ccnew2')} ON #{table_name} (#{column_name})")
 
       # Unrelated index - don't drop
       connection.execute("CREATE INDEX some_other_index_ccnew ON #{table_name} (#{column_name})")
     end
 
-    it 'drops the dangling indexes while controlling lock_timeout' do
-      expect_to_execute_in_order(
-        # Regular index rebuild
-        "SET statement_timeout TO '32400s'",
-        "REINDEX INDEX CONCURRENTLY \"public\".\"#{index.name}\"",
-        "RESET statement_timeout",
-        # Drop _ccnew index
-        "SET lock_timeout TO '60000ms'",
-        "DROP INDEX CONCURRENTLY IF EXISTS \"public\".\"#{index.name}_ccnew\"",
-        "RESET idle_in_transaction_session_timeout; RESET lock_timeout",
-        # Drop _ccnew2 index
-        "SET lock_timeout TO '60000ms'",
-        "DROP INDEX CONCURRENTLY IF EXISTS \"public\".\"#{index.name}_ccnew2\"",
-        "RESET idle_in_transaction_session_timeout; RESET lock_timeout"
-      )
-
-      subject
+    shared_examples_for 'dropping the dangling index' do
+      it 'drops the dangling indexes while controlling lock_timeout' do
+        expect_to_execute_in_order(
+          # Regular index rebuild
+          "SET statement_timeout TO '32400s'",
+          "REINDEX INDEX CONCURRENTLY \"public\".\"#{index_name}\"",
+          "RESET statement_timeout",
+          # Drop _ccnew index
+          "SET lock_timeout TO '60000ms'",
+          "DROP INDEX CONCURRENTLY IF EXISTS \"public\".\"#{iname(index_name, '_ccnew')}\"",
+          "RESET idle_in_transaction_session_timeout; RESET lock_timeout",
+          # Drop _ccnew2 index
+          "SET lock_timeout TO '60000ms'",
+          "DROP INDEX CONCURRENTLY IF EXISTS \"public\".\"#{iname(index_name, '_ccnew2')}\"",
+          "RESET idle_in_transaction_session_timeout; RESET lock_timeout"
+        )
+
+        subject
+      end
+    end
+
+    context 'with normal index names' do
+      it_behaves_like 'dropping the dangling index'
+    end
+
+    context 'with index name at 63 character limit' do
+      let(:index_name) { 'a' * 63 }
+
+      before do
+        # Another unrelated index - don't drop
+        extra_index = index_name[0...55]
+        connection.execute("CREATE INDEX #{extra_index}_ccnew ON #{table_name} (#{column_name})")
+      end
+
+      it_behaves_like 'dropping the dangling index'
     end
   end
 
+  def iname(name, suffix = '')
+    "#{name[0...63 - suffix.size]}#{suffix}"
+  end
+
   def expect_to_execute_in_order(*queries)
     # Indexes cannot be created CONCURRENTLY in a transaction. Since the tests are wrapped in transactions,
     # verify the original call but pass through the non-concurrent form.