From d2af5f8a3e89f02dd54949fa42e2dcd31ddcd5c7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 30 Sep 2004 23:21:26 +0000 Subject: [PATCH] Adjust index locking rules as per my proposal of earlier today. You now are supposed to take some kind of lock on an index whenever you are going to access the index contents, rather than relying only on a lock on the parent table. --- src/backend/access/heap/tuptoaster.c | 7 +++-- src/backend/access/index/indexam.c | 15 +++++++---- src/backend/commands/analyze.c | 8 +++--- src/backend/commands/vacuum.c | 37 +++++++++++++++++++------- src/backend/commands/vacuumlazy.c | 6 ++--- src/backend/executor/execUtils.c | 39 +++++++++++++++------------- src/include/commands/vacuum.h | 8 +++--- 7 files changed, 74 insertions(+), 46 deletions(-) diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c index fe38999141..0aa590a99e 100644 --- a/src/backend/access/heap/tuptoaster.c +++ b/src/backend/access/heap/tuptoaster.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.45 2004/08/29 05:06:40 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.46 2004/09/30 23:21:06 tgl Exp $ * * * INTERFACE ROUTINES @@ -1007,11 +1007,13 @@ toast_save_datum(Relation rel, Datum value) data_todo = VARATT_SIZE(value) - VARHDRSZ; /* - * Open the toast relation + * Open the toast relation. We must explicitly lock the toast index + * because we aren't using an index scan here. */ toastrel = heap_open(rel->rd_rel->reltoastrelid, RowExclusiveLock); toasttupDesc = toastrel->rd_att; toastidx = index_open(toastrel->rd_rel->reltoastidxid); + LockRelation(toastidx, RowExclusiveLock); /* * Split up the item into chunks @@ -1065,6 +1067,7 @@ toast_save_datum(Relation rel, Datum value) /* * Done - close toast relation and return the reference */ + UnlockRelation(toastidx, RowExclusiveLock); index_close(toastidx); heap_close(toastrel, RowExclusiveLock); diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index 949deb27a2..7c698fb48a 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.74 2004/08/29 04:12:20 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.75 2004/09/30 23:21:14 tgl Exp $ * * INTERFACE ROUTINES * index_open - open an index relation by relation OID @@ -112,10 +112,15 @@ /* ---------------- * index_open - open an index relation by relation OID * - * Note: we acquire no lock on the index. An AccessShareLock is - * acquired by index_beginscan (and released by index_endscan). - * Generally, the caller should already hold some type of lock on - * the parent relation to ensure that the index doesn't disappear. + * Note: we acquire no lock on the index. A lock is not needed when + * simply examining the index reldesc; the index's schema information + * is considered to be protected by the lock that the caller had better + * be holding on the parent relation. Some type of lock should be + * obtained on the index before physically accessing it, however. + * This is handled automatically for most uses by index_beginscan + * and index_endscan for scan cases, or by ExecOpenIndices and + * ExecCloseIndices for update cases. Other callers will need to + * obtain their own locks. * * This is a convenience routine adapted for indexscan use. * Some callers may prefer to use relation_open directly. diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index ce7db27211..486e86d677 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.76 2004/08/29 05:06:41 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.77 2004/09/30 23:21:19 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -243,7 +243,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt) * if there was an explicit column list in the ANALYZE command, * however. */ - vac_open_indexes(onerel, &nindexes, &Irel); + vac_open_indexes(onerel, AccessShareLock, &nindexes, &Irel); hasindex = (nindexes > 0); indexdata = NULL; analyzableindex = false; @@ -310,7 +310,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt) */ if (attr_cnt <= 0 && !analyzableindex) { - vac_close_indexes(nindexes, Irel); + vac_close_indexes(nindexes, Irel, AccessShareLock); relation_close(onerel, AccessShareLock); return; } @@ -427,7 +427,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt) } /* Done with indexes */ - vac_close_indexes(nindexes, Irel); + vac_close_indexes(nindexes, Irel, NoLock); /* * Close source relation now, but keep lock so that no one deletes it diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index ec39789f09..6e6b15f724 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.291 2004/09/13 20:06:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.292 2004/09/30 23:21:19 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1062,7 +1062,7 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt) scan_heap(vacrelstats, onerel, &vacuum_pages, &fraged_pages); /* Now open all indexes of the relation */ - vac_open_indexes(onerel, &nindexes, &Irel); + vac_open_indexes(onerel, AccessExclusiveLock, &nindexes, &Irel); if (nindexes > 0) vacrelstats->hasindex = true; @@ -1088,11 +1088,11 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt) /* Try to shrink heap */ repair_frag(vacrelstats, onerel, &vacuum_pages, &fraged_pages, nindexes, Irel); - vac_close_indexes(nindexes, Irel); + vac_close_indexes(nindexes, Irel, NoLock); } else { - vac_close_indexes(nindexes, Irel); + vac_close_indexes(nindexes, Irel, NoLock); if (vacuum_pages.num_pages > 0) { /* Clean pages from vacuum_pages list */ @@ -3210,8 +3210,14 @@ vac_cmp_vtlinks(const void *left, const void *right) } +/* + * Open all the indexes of the given relation, obtaining the specified kind + * of lock on each. Return an array of Relation pointers for the indexes + * into *Irel, and the number of indexes into *nindexes. + */ void -vac_open_indexes(Relation relation, int *nindexes, Relation **Irel) +vac_open_indexes(Relation relation, LOCKMODE lockmode, + int *nindexes, Relation **Irel) { List *indexoidlist; ListCell *indexoidscan; @@ -3230,23 +3236,34 @@ vac_open_indexes(Relation relation, int *nindexes, Relation **Irel) foreach(indexoidscan, indexoidlist) { Oid indexoid = lfirst_oid(indexoidscan); + Relation ind; - (*Irel)[i] = index_open(indexoid); - i++; + ind = index_open(indexoid); + (*Irel)[i++] = ind; + LockRelation(ind, lockmode); } list_free(indexoidlist); } - +/* + * Release the resources acquired by vac_open_indexes. Optionally release + * the locks (say NoLock to keep 'em). + */ void -vac_close_indexes(int nindexes, Relation *Irel) +vac_close_indexes(int nindexes, Relation *Irel, LOCKMODE lockmode) { if (Irel == NULL) return; while (nindexes--) - index_close(Irel[nindexes]); + { + Relation ind = Irel[nindexes]; + + if (lockmode != NoLock) + UnlockRelation(ind, lockmode); + index_close(ind); + } pfree(Irel); } diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index f19001d679..ebdb27f336 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -31,7 +31,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.45 2004/08/29 05:06:41 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.46 2004/09/30 23:21:19 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -145,14 +145,14 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt) vacrelstats->threshold = GetAvgFSMRequestSize(&onerel->rd_node); /* Open all indexes of the relation */ - vac_open_indexes(onerel, &nindexes, &Irel); + vac_open_indexes(onerel, ShareUpdateExclusiveLock, &nindexes, &Irel); hasindex = (nindexes > 0); /* Do the vacuuming */ lazy_scan_heap(onerel, vacrelstats, Irel, nindexes); /* Done with indexes */ - vac_close_indexes(nindexes, Irel); + vac_close_indexes(nindexes, Irel, NoLock); /* * Optionally truncate the relation. diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index a1736766b2..638a4f73df 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.115 2004/09/11 18:28:34 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.116 2004/09/30 23:21:23 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -682,28 +682,29 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo) IndexInfo *ii; /* - * Open (and lock, if necessary) the index relation + * Open and lock the index relation + * + * If the index AM supports concurrent updates, obtain RowExclusiveLock + * to signify that we are updating the index. This locks out only + * operations that need exclusive access, such as relocating the index + * to a new tablespace. * * If the index AM is not safe for concurrent updates, obtain an * exclusive lock on the index to lock out other updaters as well - * as readers (index_beginscan places AccessShareLock). We will - * release this lock in ExecCloseIndices. - * - * If the index AM supports concurrent updates, we obtain no lock - * here at all, which is a tad weird, but safe since any critical - * operation on the index (like deleting it) will acquire - * exclusive lock on the parent table. Perhaps someday we should - * acquire RowExclusiveLock on the index here? + * as readers (index_beginscan places AccessShareLock). * * If there are multiple not-concurrent-safe indexes, all backends - * must lock the indexes in the same order or we will get - * deadlocks here during concurrent updates. This is guaranteed - * by RelationGetIndexList(), which promises to return the index - * list in OID order. + * must lock the indexes in the same order or we will get deadlocks + * here. This is guaranteed by RelationGetIndexList(), which promises + * to return the index list in OID order. + * + * The locks will be released in ExecCloseIndices. */ indexDesc = index_open(indexOid); - if (!indexDesc->rd_am->amconcurrent) + if (indexDesc->rd_am->amconcurrent) + LockRelation(indexDesc, RowExclusiveLock); + else LockRelation(indexDesc, AccessExclusiveLock); /* extract index key information from the index's pg_index info */ @@ -736,10 +737,12 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo) for (i = 0; i < numIndices; i++) { if (indexDescs[i] == NULL) - continue; + continue; /* shouldn't happen? */ - /* Drop lock, if one was acquired by ExecOpenIndices */ - if (!indexDescs[i]->rd_am->amconcurrent) + /* Drop lock acquired by ExecOpenIndices */ + if (indexDescs[i]->rd_am->amconcurrent) + UnlockRelation(indexDescs[i], RowExclusiveLock); + else UnlockRelation(indexDescs[i], AccessExclusiveLock); index_close(indexDescs[i]); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index cc22a1f9ce..ba9d06d37a 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/commands/vacuum.h,v 1.57 2004/08/29 05:06:56 momjian Exp $ + * $PostgreSQL: pgsql/src/include/commands/vacuum.h,v 1.58 2004/09/30 23:21:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -126,9 +126,9 @@ extern DLLIMPORT int default_statistics_target; /* DLLIMPORT for PostGIS */ /* in commands/vacuum.c */ extern void vacuum(VacuumStmt *vacstmt); -extern void vac_open_indexes(Relation relation, int *nindexes, - Relation **Irel); -extern void vac_close_indexes(int nindexes, Relation *Irel); +extern void vac_open_indexes(Relation relation, LOCKMODE lockmode, + int *nindexes, Relation **Irel); +extern void vac_close_indexes(int nindexes, Relation *Irel, LOCKMODE lockmode); extern void vac_update_relstats(Oid relid, BlockNumber num_pages, double num_tuples,