From 7aa772f03e03c361683cf05c8cd66a9bfc8956c7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 18 Aug 2006 16:09:13 +0000 Subject: Now that we've rearranged relation open to get a lock before touching the rel, it's easy to get rid of the narrow race-condition window that used to exist in VACUUM and CLUSTER. Did some minor code-beautification work in the same area, too. --- src/backend/commands/cluster.c | 49 ++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 19 deletions(-) (limited to 'src/backend/commands/cluster.c') diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index f0cbaefd60e..bd2301dd62e 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.152 2006/07/31 20:09:00 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.153 2006/08/18 16:09:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -239,6 +239,18 @@ cluster_rel(RelToCluster *rvtc, bool recheck) /* Check for user-requested abort. */ CHECK_FOR_INTERRUPTS(); + /* + * We grab exclusive access to the target rel and index for the duration + * of the transaction. (This is redundant for the single-transaction + * case, since cluster() already did it.) The index lock is taken inside + * check_index_is_clusterable. + */ + OldHeap = try_relation_open(rvtc->tableOid, AccessExclusiveLock); + + /* If the table has gone away, we can skip processing it */ + if (!OldHeap) + return; + /* * Since we may open a new transaction for each relation, we have to check * that the relation still is what we think it is. @@ -252,20 +264,23 @@ cluster_rel(RelToCluster *rvtc, bool recheck) HeapTuple tuple; Form_pg_index indexForm; + /* Check that the user still owns the relation */ + if (!pg_class_ownercheck(rvtc->tableOid, GetUserId())) + { + relation_close(OldHeap, AccessExclusiveLock); + return; + } + /* - * Check if the relation and index still exist before opening them + * Check that the index still exists */ if (!SearchSysCacheExists(RELOID, - ObjectIdGetDatum(rvtc->tableOid), - 0, 0, 0) || - !SearchSysCacheExists(RELOID, ObjectIdGetDatum(rvtc->indexOid), 0, 0, 0)) + { + relation_close(OldHeap, AccessExclusiveLock); return; - - /* Check that the user still owns the relation */ - if (!pg_class_ownercheck(rvtc->tableOid, GetUserId())) - return; + } /* * Check that the index is still the one with indisclustered set. @@ -273,25 +288,21 @@ cluster_rel(RelToCluster *rvtc, bool recheck) tuple = SearchSysCache(INDEXRELID, ObjectIdGetDatum(rvtc->indexOid), 0, 0, 0); - if (!HeapTupleIsValid(tuple)) - return; /* could have gone away... */ + if (!HeapTupleIsValid(tuple)) /* probably can't happen */ + { + relation_close(OldHeap, AccessExclusiveLock); + return; + } indexForm = (Form_pg_index) GETSTRUCT(tuple); if (!indexForm->indisclustered) { ReleaseSysCache(tuple); + relation_close(OldHeap, AccessExclusiveLock); return; } ReleaseSysCache(tuple); } - /* - * We grab exclusive access to the target rel and index for the duration - * of the transaction. (This is redundant for the single- transaction - * case, since cluster() already did it.) The index lock is taken inside - * check_index_is_clusterable. - */ - OldHeap = heap_open(rvtc->tableOid, AccessExclusiveLock); - /* Check index is valid to cluster on */ check_index_is_clusterable(OldHeap, rvtc->indexOid, recheck); -- cgit v1.2.3