From 720b59b55b84c8a346098cdbf3d34c7e554b0bf5 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 28 Aug 2019 13:37:03 +1200 Subject: [PATCH] Avoid catalog lookups in RelationAllowsEarlyPruning(). RelationAllowsEarlyPruning() performed a catalog scan, but is used in two contexts where that was a bad idea: 1. In heap_page_prune_opt(), which runs very frequently in some large scans. This caused major performance problems in a field report that was easy to reproduce. 2. In TestForOldSnapshot(), which runs while we hold a buffer content lock. It's not clear if this was guaranteed to be free of buffer deadlock risk. The check was introduced in commit 2cc41acd8 and defended against a real problem: 9.6's hash indexes have no page LSN and so we can't allow early pruning (ie the snapshot-too-old feature). We can remove the check from all later releases though: hash indexes are now logged, and there is no way to create UNLOGGED indexes on regular logged tables. If a future release allows such a combination, it might need to put a similar check in place, but it'll need some more thought. Back-patch to 10. Author: Thomas Munro Reviewed-by: Tom Lane, who spotted the second problem Discussion: https://postgr.es/m/CA%2BhUKGKT8oTkp5jw_U4p0S-7UG9zsvtw_M47Y285bER6a2gD%2Bg%40mail.gmail.com Discussion: https://postgr.es/m/CAA4eK1%2BWy%2BN4eE5zPm765h68LrkWc3Biu_8rzzi%2BOYX4j%2BiHRw%40mail.gmail.com --- src/backend/utils/cache/relcache.c | 42 ------------------------------ src/include/utils/rel.h | 1 - src/include/utils/snapmgr.h | 1 - 3 files changed, 44 deletions(-) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 248860758c..585dcee5db 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5913,48 +5913,6 @@ RelationIdIsInInitFile(Oid relationId) return RelationSupportsSysCache(relationId); } -/* - * Tells whether any index for the relation is unlogged. - * - * Note: There doesn't seem to be any way to have an unlogged index attached - * to a permanent table, but it seems best to keep this general so that it - * returns sensible results even when they seem obvious (like for an unlogged - * table) and to handle possible future unlogged indexes on permanent tables. - */ -bool -RelationHasUnloggedIndex(Relation rel) -{ - List *indexoidlist; - ListCell *indexoidscan; - bool result = false; - - indexoidlist = RelationGetIndexList(rel); - - foreach(indexoidscan, indexoidlist) - { - Oid indexoid = lfirst_oid(indexoidscan); - HeapTuple tp; - Form_pg_class reltup; - - tp = SearchSysCache1(RELOID, ObjectIdGetDatum(indexoid)); - if (!HeapTupleIsValid(tp)) - elog(ERROR, "cache lookup failed for relation %u", indexoid); - reltup = (Form_pg_class) GETSTRUCT(tp); - - if (reltup->relpersistence == RELPERSISTENCE_UNLOGGED) - result = true; - - ReleaseSysCache(tp); - - if (result == true) - break; - } - - list_free(indexoidlist); - - return result; -} - /* * Invalidate (remove) the init file during commit of a transaction that * changed one or more of the relation cache entries that are kept in the diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index c5d36680a2..91b3b1b902 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -605,6 +605,5 @@ typedef struct ViewOptions /* routines in utils/cache/relcache.c */ extern void RelationIncrementReferenceCount(Relation rel); extern void RelationDecrementReferenceCount(Relation rel); -extern bool RelationHasUnloggedIndex(Relation rel); #endif /* REL_H */ diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 8c070d7f41..67b07df48c 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -40,7 +40,6 @@ RelationNeedsWAL(rel) \ && !IsCatalogRelation(rel) \ && !RelationIsAccessibleInLogicalDecoding(rel) \ - && !RelationHasUnloggedIndex(rel) \ ) #define EarlyPruningEnabled(rel) (old_snapshot_threshold >= 0 && RelationAllowsEarlyPruning(rel))