From cfac3efb2f75d19b35f428fa4d248b4114cd0437 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 13 Jan 2010 23:07:28 +0000 Subject: [PATCH] When loading critical system indexes into the relcache, ensure we lock the underlying catalog not only the index itself. Otherwise, if the cache load process touches the catalog (which will happen for many though not all of these indexes), we are locking index before parent table, which can result in a deadlock against processes that are trying to lock them in the normal order. Per today's failure on buildfarm member gothic_moth; it's surprising the problem hadn't been identified before. Back-patch to 8.2. Earlier releases didn't have the issue because they didn't try to lock these indexes during load (instead assuming that they couldn't change schema at all during multiuser operation). --- src/backend/utils/cache/relcache.c | 71 ++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 8235d93d8b6..99a7482fb93 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.250.2.4 2010/01/12 18:12:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.250.2.5 2010/01/13 23:07:28 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -43,8 +43,10 @@ #include "catalog/pg_constraint.h" #include "catalog/pg_namespace.h" #include "catalog/pg_opclass.h" +#include "catalog/pg_operator.h" #include "catalog/pg_proc.h" #include "catalog/pg_rewrite.h" +#include "catalog/pg_trigger.h" #include "catalog/pg_type.h" #include "commands/trigger.h" #include "miscadmin.h" @@ -194,6 +196,7 @@ static void RelationParseRelOptions(Relation relation, HeapTuple tuple); static void RelationBuildTupleDesc(Relation relation); static Relation RelationBuildDesc(Oid targetRelId, bool insertIt); static void RelationInitPhysicalAddr(Relation relation); +static void load_critical_index(Oid indexoid, Oid heapoid); static TupleDesc GetPgClassDescriptor(void); static TupleDesc GetPgIndexDescriptor(void); static void AttrDefaultFetch(Relation relation); @@ -2451,28 +2454,22 @@ RelationCacheInitializePhase2(void) */ if (!criticalRelcachesBuilt) { - Relation ird; - -#define LOAD_CRIT_INDEX(indexoid) \ - do { \ - LockRelationOid(indexoid, AccessShareLock); \ - ird = RelationBuildDesc(indexoid, true); \ - if (ird == NULL) \ - elog(PANIC, "could not open critical system index %u", \ - indexoid); \ - ird->rd_isnailed = true; \ - ird->rd_refcnt = 1; \ - UnlockRelationOid(indexoid, AccessShareLock); \ - } while (0) - - LOAD_CRIT_INDEX(ClassOidIndexId); - LOAD_CRIT_INDEX(AttributeRelidNumIndexId); - LOAD_CRIT_INDEX(IndexRelidIndexId); - LOAD_CRIT_INDEX(AccessMethodStrategyIndexId); - LOAD_CRIT_INDEX(AccessMethodProcedureIndexId); - LOAD_CRIT_INDEX(OperatorOidIndexId); - LOAD_CRIT_INDEX(RewriteRelRulenameIndexId); - LOAD_CRIT_INDEX(TriggerRelidNameIndexId); + load_critical_index(ClassOidIndexId, + RelationRelationId); + load_critical_index(AttributeRelidNumIndexId, + AttributeRelationId); + load_critical_index(IndexRelidIndexId, + IndexRelationId); + load_critical_index(AccessMethodStrategyIndexId, + AccessMethodOperatorRelationId); + load_critical_index(AccessMethodProcedureIndexId, + AccessMethodProcedureRelationId); + load_critical_index(OperatorOidIndexId, + OperatorRelationId); + load_critical_index(RewriteRelRulenameIndexId, + RewriteRelationId); + load_critical_index(TriggerRelidNameIndexId, + TriggerRelationId); #define NUM_CRITICAL_INDEXES 8 /* fix if you change list above */ @@ -2606,6 +2603,34 @@ RelationCacheInitializePhase2(void) } } +/* + * Load one critical system index into the relcache + * + * indexoid is the OID of the target index, heapoid is the OID of the catalog + * it belongs to. + */ +static void +load_critical_index(Oid indexoid, Oid heapoid) +{ + Relation ird; + + /* + * We must lock the underlying catalog before locking the index to avoid + * deadlock, since RelationBuildDesc might well need to read the catalog, + * and if anyone else is exclusive-locking this catalog and index they'll + * be doing it in that order. + */ + LockRelationOid(heapoid, AccessShareLock); + LockRelationOid(indexoid, AccessShareLock); + ird = RelationBuildDesc(indexoid, true); + if (ird == NULL) + elog(PANIC, "could not open critical system index %u", indexoid); + ird->rd_isnailed = true; + ird->rd_refcnt = 1; + UnlockRelationOid(indexoid, AccessShareLock); + UnlockRelationOid(heapoid, AccessShareLock); +} + /* * GetPgClassDescriptor -- get a predefined tuple descriptor for pg_class * GetPgIndexDescriptor -- get a predefined tuple descriptor for pg_index