From b83b9701deaed7d1d49d7393d75f75d28fe50778 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 13 Jan 2023 11:04:26 +0800 Subject: [PATCH] FIX: Add migration to reindex invalid indexes (#19858) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Discourse, there are many migration files where we CREATE INDEX CONCURRENTLY which requires us to set disable_ddl_transaction!. Setting disable_ddl_transaction! in a migration file runs the SQL statements outside of a transaction. The implication of this is that there is no ROLLBACK should any of the SQL statements fail. We have seen lock timeouts occuring when running CREATE INDEX CONCURRENTLY. When that happens, the index would still have been created but marked as invalid by Postgres. Per the postgres documentation: > If a problem arises while scanning the table, such as a deadlock or a uniqueness violation in a unique index, the CREATE INDEX command will fail but leave behind an “invalid” index. This index will be ignored for querying purposes because it might be incomplete; however it will still consume update overhead. > The recommended recovery method in such cases is to drop the index and try again to perform CREATE INDEX CONCURRENTLY . (Another possibility is to rebuild the index with REINDEX INDEX CONCURRENTLY ). When such scenarios happen, we are supposed to either drop and create the index again or run a REINDEX operation. However, I noticed today that we have not been doing so in Discourse. Instead, we’ve been incorrectly working around the problem by checking for the index existence before creating the index in order to make the migration idempotent. What this potentially mean is that we might have invalid indexes which are lying around in the database which PG will ignore for querying purposes. This commits adds a migration which queries for all the invalid indexes in the `public` namespace and reindexes them. --- .../20230113002617_reindex_invalid_indexes.rb | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 db/post_migrate/20230113002617_reindex_invalid_indexes.rb diff --git a/db/post_migrate/20230113002617_reindex_invalid_indexes.rb b/db/post_migrate/20230113002617_reindex_invalid_indexes.rb new file mode 100644 index 00000000000..290e9e0f537 --- /dev/null +++ b/db/post_migrate/20230113002617_reindex_invalid_indexes.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class ReindexInvalidIndexes < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def up + invalid_index_names = DB.query_single(<<~SQL) + SELECT + pg_class.relname + FROM pg_class, pg_index, pg_namespace + WHERE pg_index.indisvalid = false + AND pg_index.indexrelid = pg_class.oid + AND pg_namespace.nspname = 'public' + AND relnamespace = pg_namespace.oid; + SQL + + invalid_index_names.each { |index_name| execute "REINDEX INDEX CONCURRENTLY #{index_name}" } + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end