database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-05 07:21:24 +03:00
Code Activity

Now that we've rearranged relation open to get a lock before touching

Browse Source 
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.
This commit is contained in:
Tom Lane
2006-08-18 16:09:13 +00:00
parent e91600d1c2
commit 7aa772f03e
8 changed files with 153 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/backend/commands/analyze.c
View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.96 2006/07/14 14:52:18 momjian Exp $
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.97 2006/08/18 16:09:08 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -129,20 +129,16 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
CHECK_FOR_INTERRUPTS();
/*
* Race condition -- if the pg_class tuple has gone away since the last
* time we saw it, we don't need to process it.
* Open the relation, getting only a read lock on it. If the rel has
* been dropped since we last saw it, we don't need to process it.
*/
if (!SearchSysCacheExists(RELOID,
ObjectIdGetDatum(relid),
0, 0, 0))
onerel = try_relation_open(relid, AccessShareLock);
if (!onerel)
return;
/*
* Open the class, getting only a read lock on it, and check permissions.
* Permissions check should match vacuum's check!
* Check permissions --- this should match vacuum's check!
*/
onerel = relation_open(relid, AccessShareLock);
if (!(pg_class_ownercheck(RelationGetRelid(onerel), GetUserId()) ||
(pg_database_ownercheck(MyDatabaseId, GetUserId()) && !onerel->rd_rel->relisshared)))
{

55
src/backend/commands/cluster.c
View File

@ -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 if the relation and index still exist before opening them
*/
if (!SearchSysCacheExists(RELOID,
ObjectIdGetDatum(rvtc->tableOid),
0, 0, 0) ||
!SearchSysCacheExists(RELOID,
ObjectIdGetDatum(rvtc->indexOid),
0, 0, 0))
return;
/* Check that the user still owns the relation */
if (!pg_class_ownercheck(rvtc->tableOid, GetUserId()))
{
relation_close(OldHeap, AccessExclusiveLock);
return;
}
/*
* Check that the index still exists
*/
if (!SearchSysCacheExists(RELOID,
ObjectIdGetDatum(rvtc->indexOid),
0, 0, 0))
{
relation_close(OldHeap, AccessExclusiveLock);
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);

7
src/backend/commands/lockcmds.c
View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.14 2006/03/05 15:58:24 momjian Exp $
* $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.15 2006/08/18 16:09:08 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -59,7 +59,10 @@ LockTableCommand(LockStmt *lockstmt)
aclcheck_error(aclresult, ACL_KIND_CLASS,
get_rel_name(reloid));
rel = conditional_relation_open(reloid, lockstmt->mode, lockstmt->nowait);
if (lockstmt->nowait)
rel = relation_open_nowait(reloid, lockstmt->mode);
else
rel = relation_open(reloid, lockstmt->mode);
/* Currently, we only allow plain tables to be locked */
if (rel->rd_rel->relkind != RELKIND_RELATION)

51
src/backend/commands/vacuum.c
View File

@ -13,7 +13,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.337 2006/07/31 20:09:00 tgl Exp $
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.338 2006/08/18 16:09:08 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -1041,31 +1041,12 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
MyProc->inVacuum = true;
}
/*
* Tell the cache replacement strategy that vacuum is causing all
* following IO
*/
StrategyHintVacuum(true);
/*
* Check for user-requested abort. Note we want this to be inside a
* transaction, so xact.c doesn't issue useless WARNING.
*/
CHECK_FOR_INTERRUPTS();
/*
* Race condition -- if the pg_class tuple has gone away since the last
* time we saw it, we don't need to vacuum it.
*/
if (!SearchSysCacheExists(RELOID,
ObjectIdGetDatum(relid),
0, 0, 0))
{
StrategyHintVacuum(false);
CommitTransactionCommand();
return;
}
/*
* Determine the type of lock we want --- hard exclusive lock for a FULL
* vacuum, but just ShareUpdateExclusiveLock for concurrent vacuum. Either
@ -1074,7 +1055,21 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
lmode = vacstmt->full ? AccessExclusiveLock : ShareUpdateExclusiveLock;
/*
* Open the class, get an appropriate lock on it, and check permissions.
* Open the relation and get the appropriate lock on it.
*
* There's a race condition here: the rel may have gone away since
* the last time we saw it. If so, we don't need to vacuum it.
*/
onerel = try_relation_open(relid, lmode);
if (!onerel)
{
CommitTransactionCommand();
return;
}
/*
* Check permissions.
*
* We allow the user to vacuum a table if he is superuser, the table
* owner, or the database owner (but in the latter case, only if it's not
@ -1083,8 +1078,6 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
* Note we choose to treat permissions failure as a WARNING and keep
* trying to vacuum the rest of the DB --- is this appropriate?
*/
onerel = relation_open(relid, lmode);
if (!(pg_class_ownercheck(RelationGetRelid(onerel), GetUserId()) ||
(pg_database_ownercheck(MyDatabaseId, GetUserId()) && !onerel->rd_rel->relisshared)))
{
@ -1092,7 +1085,6 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
(errmsg("skipping \"%s\" --- only table or database owner can vacuum it",
RelationGetRelationName(onerel))));
relation_close(onerel, lmode);
StrategyHintVacuum(false);
CommitTransactionCommand();
return;
}
@ -1107,7 +1099,6 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
(errmsg("skipping \"%s\" --- cannot vacuum indexes, views, or special system tables",
RelationGetRelationName(onerel))));
relation_close(onerel, lmode);
StrategyHintVacuum(false);
CommitTransactionCommand();
return;
}
@ -1122,7 +1113,6 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
if (isOtherTempNamespace(RelationGetNamespace(onerel)))
{
relation_close(onerel, lmode);
StrategyHintVacuum(false);
CommitTransactionCommand();
return; /* assume no long-lived data in temp tables */
}
@ -1145,6 +1135,12 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
*/
toast_relid = onerel->rd_rel->reltoastrelid;
/*
* Tell the cache replacement strategy that vacuum is causing all
* following IO
*/
StrategyHintVacuum(true);
/*
* Do the actual work --- either FULL or "lazy" vacuum
*/
@ -1153,13 +1149,14 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
else
lazy_vacuum_rel(onerel, vacstmt);
StrategyHintVacuum(false);
/* all done with this class, but hold lock until commit */
relation_close(onerel, NoLock);
/*
* Complete the transaction and free all temporary memory used.
*/
StrategyHintVacuum(false);
CommitTransactionCommand();
/*