Merge branch 'ab/reindex-functional-analyze' into 'master'

Rebuild table statistics after reindexing an expression index

See merge request gitlab-org/gitlab!47422

changelogs/unreleased/ab-reindex-functional-analyze.yml

+---
+title: Add relation name to indexes view
+merge_request: 47422
+author:
+type: other

db/migrate/20201111145317_add_relation_to_indexes_view.rb

+# frozen_string_literal: true
+
+class AddRelationToIndexesView < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    execute(<<~SQL)
+      DROP VIEW postgres_indexes;
+
+      CREATE VIEW postgres_indexes AS
+      SELECT (pg_namespace.nspname::text || '.'::text) || pg_class.relname::text AS identifier,
+        pg_index.indexrelid,
+        pg_namespace.nspname AS schema,
+        pg_class.relname AS name,
+        pg_indexes.tablename,
+        pg_index.indisunique AS "unique",
+        pg_index.indisvalid AS valid_index,
+        pg_class.relispartition AS partitioned,
+        pg_index.indisexclusion AS exclusion,
+        pg_index.indexprs IS NOT NULL as expression,
+        pg_index.indpred IS NOT NULL as partial,
+        pg_indexes.indexdef AS definition,
+        pg_relation_size(pg_class.oid::regclass) AS ondisk_size_bytes
+      FROM pg_index
+        JOIN pg_class ON pg_class.oid = pg_index.indexrelid
+        JOIN pg_namespace ON pg_class.relnamespace = pg_namespace.oid
+        JOIN pg_indexes ON pg_class.relname = pg_indexes.indexname
+      WHERE pg_namespace.nspname <> 'pg_catalog'::name
+        AND (pg_namespace.nspname = ANY (ARRAY["current_schema"(), 'gitlab_partitions_dynamic'::name, 'gitlab_partitions_static'::name]));
+    SQL
+  end
+
+  def down
+    execute(<<~SQL)
+      DROP VIEW postgres_indexes;
+
+      CREATE VIEW postgres_indexes AS
+      SELECT (pg_namespace.nspname::text || '.'::text) || pg_class.relname::text AS identifier,
+        pg_index.indexrelid,
+        pg_namespace.nspname AS schema,
+        pg_class.relname AS name,
+        pg_index.indisunique AS "unique",
+        pg_index.indisvalid AS valid_index,
+        pg_class.relispartition AS partitioned,
+        pg_index.indisexclusion AS exclusion,
+        pg_index.indexprs IS NOT NULL as expression,
+        pg_index.indpred IS NOT NULL as partial,
+        pg_indexes.indexdef AS definition,
+        pg_relation_size(pg_class.oid::regclass) AS ondisk_size_bytes
+      FROM pg_index
+        JOIN pg_class ON pg_class.oid = pg_index.indexrelid
+        JOIN pg_namespace ON pg_class.relnamespace = pg_namespace.oid
+        JOIN pg_indexes ON pg_class.relname = pg_indexes.indexname
+      WHERE pg_namespace.nspname <> 'pg_catalog'::name
+        AND (pg_namespace.nspname = ANY (ARRAY["current_schema"(), 'gitlab_partitions_dynamic'::name, 'gitlab_partitions_static'::name]));
+    SQL
+  end
+end

db/schema_migrations/20201111145317

+51d26848722466503e43b0f41e2fa43ee6073a16b358311a0aff6d77fbb12b1d
\ No newline at end of file

db/structure.sql

@@ -14897,6 +14897,7 @@ CREATE VIEW postgres_indexes AS
     pg_index.indexrelid,
     pg_namespace.nspname AS schema,
     pg_class.relname AS name,
+    pg_indexes.tablename,
     pg_index.indisunique AS "unique",
     pg_index.indisvalid AS valid_index,
     pg_class.relispartition AS partitioned,

lib/gitlab/database/reindexing/concurrent_reindex.rb

@@ -59,6 +59,13 @@ module Gitlab
             raise ReindexError, "failed to reindex #{index}: #{message}"
           end
 
+          # Some expression indexes (aka functional indexes)
+          # require additional statistics. The existing statistics
+          # are tightly bound to the original index. We have to
+          # rebuild statistics for the new index before dropping
+          # the original one.
+          rebuild_statistics if index.expression?
+
           yield replacement_index
         ensure
           begin
@@ -96,6 +103,14 @@ module Gitlab
           end
         end
 
+        def rebuild_statistics
+          logger.info("rebuilding table statistics for #{index.schema}.#{index.tablename}")
+
+          connection.execute(<<~SQL)
+            ANALYZE #{quote_table_name(index.schema)}.#{quote_table_name(index.tablename)}
+          SQL
+        end
+
         def replacement_index_name
           @replacement_index_name ||= "#{TEMPORARY_INDEX_PREFIX}#{index.indexrelid}"
         end

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

@@ -8,7 +8,7 @@ RSpec.describe Gitlab::Database::Reindexing::ConcurrentReindex, '#perform' do
   let(:table_name) { '_test_reindex_table' }
   let(:column_name) { '_test_column' }
   let(:index_name) { '_test_reindex_index' }
-  let(:index) { instance_double(Gitlab::Database::PostgresIndex, indexrelid: 42, name: index_name, schema: 'public', partitioned?: false, unique?: false, exclusion?: false, definition: 'CREATE INDEX _test_reindex_index ON public._test_reindex_table USING btree (_test_column)') }
+  let(:index) { instance_double(Gitlab::Database::PostgresIndex, indexrelid: 42, name: index_name, schema: 'public', tablename: table_name, partitioned?: false, unique?: false, exclusion?: false, expression?: false, definition: 'CREATE INDEX _test_reindex_index ON public._test_reindex_table USING btree (_test_column)') }
   let(:logger) { double('logger', debug: nil, info: nil, error: nil ) }
   let(:connection) { ActiveRecord::Base.connection }
 
@@ -130,6 +130,36 @@ RSpec.describe Gitlab::Database::Reindexing::ConcurrentReindex, '#perform' do
         check_index_exists
       end
 
+      context 'for expression indexes' do
+        before do
+          allow(index).to receive(:expression?).and_return(true)
+        end
+
+        it 'rebuilds table statistics before dropping the original index' do
+          expect(connection).to receive(:execute).with('SET statement_timeout TO \'21600s\'').twice
+
+          expect_to_execute_concurrently_in_order(create_index)
+
+          expect_to_execute_concurrently_in_order(<<~SQL)
+            ANALYZE "#{index.schema}"."#{index.tablename}"
+          SQL
+
+          expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance|
+            expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield
+          end
+
+          expect_index_rename(index.name, replaced_name)
+          expect_index_rename(replacement_name, index.name)
+          expect_index_rename(replaced_name, replacement_name)
+
+          expect_to_execute_concurrently_in_order(drop_index)
+
+          subject.perform
+
+          check_index_exists
+        end
+      end
+
       context 'when a dangling index is left from a previous run' do
         before do
           connection.execute("CREATE INDEX #{replacement_name} ON #{table_name} (#{column_name})")