From 990d265e5d576b3b4133232f302d6207987f1511 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Fri, 21 Feb 2020 20:15:04 +0100 Subject: [PATCH] Don't reindex invalid indexes on TOAST tables. Such indexes can only be duplicated leftovers of failed REINDEX CONCURRENTLY commands. As we only allow to drop invalid indexes on TOAST tables, reindexing those would lead to useless duplicated indexes that can't be dropped anymore. Reported-by: Sergei Kornilov, Justin Pryzby Author: Julien Rouhaud Reviewed-by: Discussion: https://postgr.es/m/36712441546604286%40sas1-890ba5c2334a.qloud-c.yandex.net Discussion: https://postgr.es/m/20200216190835.GA21832@telsasoft.com Backpatch-through: 12 --- src/backend/catalog/index.c | 29 +++++++++++ .../expected/reindex-concurrently.out | 49 ++++++++++++++++++- .../isolation/specs/reindex-concurrently.spec | 23 +++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 8880586c37..201a3c39df 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -46,6 +46,7 @@ #include "catalog/pg_depend.h" #include "catalog/pg_description.h" #include "catalog/pg_inherits.h" +#include "catalog/pg_namespace_d.h" #include "catalog/pg_opclass.h" #include "catalog/pg_operator.h" #include "catalog/pg_tablespace.h" @@ -3717,6 +3718,34 @@ reindex_relation(Oid relid, int flags, int options) { Oid indexOid = lfirst_oid(indexId); + /* + * We skip any invalid index on a TOAST table. Those can only be + * a duplicate leftover of a failed REINDEX CONCURRENTLY, and if we + * rebuild it it won't be possible to drop it anymore. + */ + if (rel->rd_rel->relnamespace == PG_TOAST_NAMESPACE) + { + HeapTuple tup; + bool skipit; + + tup = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for index %u", indexOid); + + skipit = ((Form_pg_index) GETSTRUCT(tup))->indisvalid == false; + + ReleaseSysCache(tup); + + if (skipit) + { + ereport(NOTICE, + (errmsg("skipping invalid index \"%s.%s\"", + get_namespace_name(get_rel_namespace(indexOid)), + get_rel_name(indexOid)))); + continue; + } + } + reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS), persistence, options); diff --git a/src/test/isolation/expected/reindex-concurrently.out b/src/test/isolation/expected/reindex-concurrently.out index 9e04169b2f..fa9039c125 100644 --- a/src/test/isolation/expected/reindex-concurrently.out +++ b/src/test/isolation/expected/reindex-concurrently.out @@ -1,4 +1,4 @@ -Parsed test spec with 3 sessions +Parsed test spec with 5 sessions starting permutation: reindex sel1 upd2 ins2 del2 end1 end2 step reindex: REINDEX TABLE CONCURRENTLY reind_con_tab; @@ -76,3 +76,50 @@ step end1: COMMIT; step reindex: REINDEX TABLE CONCURRENTLY reind_con_tab; step end2: COMMIT; step reindex: <... completed> + +starting permutation: lock timeout reindex unlock check_invalid normal_reindex check_invalid nowarn reindex check_invalid +step lock: BEGIN; SELECT data FROM reind_con_tab WHERE data = 'aa' FOR UPDATE; +data + +aa +step timeout: SET statement_timeout to 5000; +step reindex: REINDEX TABLE CONCURRENTLY reind_con_tab; +step unlock: SELECT pg_sleep(6); COMMIT; +pg_sleep + + +step reindex: <... completed> +error in steps unlock reindex: ERROR: canceling statement due to statement timeout +step check_invalid: SELECT i.indisvalid + FROM pg_class c + JOIN pg_class t ON t.oid = c.reltoastrelid + JOIN pg_index i ON i.indrelid = t.oid + WHERE c.relname = 'reind_con_tab' + ORDER BY i.indisvalid::text COLLATE "C"; +indisvalid + +f +t +step normal_reindex: REINDEX TABLE reind_con_tab; +step check_invalid: SELECT i.indisvalid + FROM pg_class c + JOIN pg_class t ON t.oid = c.reltoastrelid + JOIN pg_index i ON i.indrelid = t.oid + WHERE c.relname = 'reind_con_tab' + ORDER BY i.indisvalid::text COLLATE "C"; +indisvalid + +f +t +step nowarn: SET client_min_messages = 'ERROR'; +step reindex: REINDEX TABLE CONCURRENTLY reind_con_tab; +step check_invalid: SELECT i.indisvalid + FROM pg_class c + JOIN pg_class t ON t.oid = c.reltoastrelid + JOIN pg_index i ON i.indrelid = t.oid + WHERE c.relname = 'reind_con_tab' + ORDER BY i.indisvalid::text COLLATE "C"; +indisvalid + +f +t diff --git a/src/test/isolation/specs/reindex-concurrently.spec b/src/test/isolation/specs/reindex-concurrently.spec index eb59fe0cba..0599e8e213 100644 --- a/src/test/isolation/specs/reindex-concurrently.spec +++ b/src/test/isolation/specs/reindex-concurrently.spec @@ -31,6 +31,22 @@ step "end2" { COMMIT; } session "s3" step "reindex" { REINDEX TABLE CONCURRENTLY reind_con_tab; } +step "nowarn" { SET client_min_messages = 'ERROR'; } +step "timeout" { SET statement_timeout to 5000; } + +session "s4" +step "lock" { BEGIN; SELECT data FROM reind_con_tab WHERE data = 'aa' FOR UPDATE; } +step "unlock" { SELECT pg_sleep(6); COMMIT; } + +session "s5" +setup { SET client_min_messages = 'WARNING'; } +step "normal_reindex" { REINDEX TABLE reind_con_tab; } +step "check_invalid" {SELECT i.indisvalid + FROM pg_class c + JOIN pg_class t ON t.oid = c.reltoastrelid + JOIN pg_index i ON i.indrelid = t.oid + WHERE c.relname = 'reind_con_tab' + ORDER BY i.indisvalid::text COLLATE "C"; } permutation "reindex" "sel1" "upd2" "ins2" "del2" "end1" "end2" permutation "sel1" "reindex" "upd2" "ins2" "del2" "end1" "end2" @@ -38,3 +54,10 @@ permutation "sel1" "upd2" "reindex" "ins2" "del2" "end1" "end2" permutation "sel1" "upd2" "ins2" "reindex" "del2" "end1" "end2" permutation "sel1" "upd2" "ins2" "del2" "reindex" "end1" "end2" permutation "sel1" "upd2" "ins2" "del2" "end1" "reindex" "end2" +# A failed REINDEX CONCURRENTLY will leave an invalid index on reind_con_tab +# TOAST table. Any following successful REINDEX should leave this index as +# invalid, otherwise we would end up with a useless and duplicated index that +# can't be dropped. +permutation "lock" "timeout" "reindex" "unlock" "check_invalid" + "normal_reindex" "check_invalid" + "nowarn" "reindex" "check_invalid" -- 2.20.1