From eeb68c1cddf3cdbfbd543a7aa24e6cbbe0e31b16 Mon Sep 17 00:00:00 2001
From: Thomas Munro <tmunro@postgresql.org>
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 65b74cb6d87..24e830a39c1 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -6068,48 +6068,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 58877e7f1cf..c6b8f4fb47f 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -638,7 +638,6 @@ get_partition_col_typmod(PartitionKey key, int col)
/* routines in utils/cache/relcache.c */
extern void RelationIncrementReferenceCount(Relation rel);
extern void RelationDecrementReferenceCount(Relation rel);
-extern bool RelationHasUnloggedIndex(Relation rel);
extern List *RelationGetRepsetList(Relation rel);
#endif /* REL_H */
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 8585194e785..7b428fe9c01 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))