Skip to content

Commit bd0a260

Browse files
tglsfdctglsfdc
committed
Make CREATE/DROP/RENAME DATABASE wait a little bit to see if other backends
will exit before failing because of conflicting DB usage. Per discussion, this seems a good idea to help mask the fact that backend exit takes nonzero time. Remove a couple of thereby-obsoleted sleeps in contrib and PL regression test sequences.
1 parent 41ef1c0 commit bd0a260

File tree

5 files changed

+141
-108
lines changed

5 files changed

+141
-108
lines changed

‎contrib/Makefile

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# $PostgreSQL: pgsql/contrib/Makefile,v 1.76 2007/05/17 19:11:24 momjian Exp $
1+
# $PostgreSQL: pgsql/contrib/Makefile,v 1.77 2007/06/01 19:38:07 tgl Exp $
22

33
subdir = contrib
44
top_builddir = ..
@@ -57,12 +57,9 @@ all install installdirs uninstall distprep clean distclean maintainer-clean:
5757
$(MAKE) -C $$dir $@ || exit; \
5858
done
5959

60-
# We'd like check operations to run all the subtests before failing;
61-
# also insert a sleep to ensure the previous test backend exited before
62-
# we try to drop the regression database.
60+
# We'd like check operations to run all the subtests before failing.
6361
check installcheck:
6462
@CHECKERR=0; for dir in $(WANTED_DIRS); do \
65-
sleep 1; \
6663
$(MAKE) -C $$dir $@ || CHECKERR=$$?; \
6764
done; \
6865
exit $$CHECKERR

‎src/backend/commands/dbcommands.c

Lines changed: 49 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.194 2007/04/12 15:04:35 tgl Exp $
16+
* $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.195 2007/06/01 19:38:07 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -244,17 +244,6 @@ createdb(const CreatedbStmt *stmt)
244244
dbtemplate)));
245245
}
246246

247-
/*
248-
* The source DB can't have any active backends, except this one
249-
* (exception is to allow CREATE DB while connected to template1).
250-
* Otherwise we might copy inconsistent data.
251-
*/
252-
if (DatabaseCancelAutovacuumActivity(src_dboid, true))
253-
ereport(ERROR,
254-
(errcode(ERRCODE_OBJECT_IN_USE),
255-
errmsg("source database \"%s\" is being accessed by other users",
256-
dbtemplate)));
257-
258247
/* If encoding is defaulted, use source's encoding */
259248
if (encoding < 0)
260249
encoding = src_encoding;
@@ -333,6 +322,21 @@ createdb(const CreatedbStmt *stmt)
333322
(errcode(ERRCODE_DUPLICATE_DATABASE),
334323
errmsg("database \"%s\" already exists", dbname)));
335324

325+
/*
326+
* The source DB can't have any active backends, except this one
327+
* (exception is to allow CREATE DB while connected to template1).
328+
* Otherwise we might copy inconsistent data.
329+
*
330+
* This should be last among the basic error checks, because it involves
331+
* potential waiting; we may as well throw an error first if we're gonna
332+
* throw one.
333+
*/
334+
if (CheckOtherDBBackends(src_dboid))
335+
ereport(ERROR,
336+
(errcode(ERRCODE_OBJECT_IN_USE),
337+
errmsg("source database \"%s\" is being accessed by other users",
338+
dbtemplate)));
339+
336340
/*
337341
* Select an OID for the new database, checking that it doesn't have
338342
* a filename conflict with anything already existing in the tablespace
@@ -542,13 +546,6 @@ dropdb(const char *dbname, bool missing_ok)
542546
Relation pgdbrel;
543547
HeapTuple tup;
544548

545-
AssertArg(dbname);
546-
547-
if (strcmp(dbname, get_database_name(MyDatabaseId)) == 0)
548-
ereport(ERROR,
549-
(errcode(ERRCODE_OBJECT_IN_USE),
550-
errmsg("cannot drop the currently open database")));
551-
552549
/*
553550
* Look up the target database's OID, and get exclusive lock on it. We
554551
* need this to ensure that no new backend starts up in the target
@@ -595,11 +592,19 @@ dropdb(const char *dbname, bool missing_ok)
595592
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
596593
errmsg("cannot drop a template database")));
597594

595+
/* Obviously can't drop my own database */
596+
if (db_id == MyDatabaseId)
597+
ereport(ERROR,
598+
(errcode(ERRCODE_OBJECT_IN_USE),
599+
errmsg("cannot drop the currently open database")));
600+
598601
/*
599-
* Check for active backends in the target database. (Because we hold the
602+
* Check for other backends in the target database. (Because we hold the
600603
* database lock, no new ones can start after this.)
604+
*
605+
* As in CREATE DATABASE, check this after other error conditions.
601606
*/
602-
if (DatabaseCancelAutovacuumActivity(db_id, false))
607+
if (CheckOtherDBBackends(db_id))
603608
ereport(ERROR,
604609
(errcode(ERRCODE_OBJECT_IN_USE),
605610
errmsg("database \"%s\" is being accessed by other users",
@@ -699,6 +704,26 @@ RenameDatabase(const char *oldname, const char *newname)
699704
(errcode(ERRCODE_UNDEFINED_DATABASE),
700705
errmsg("database \"%s\" does not exist", oldname)));
701706

707+
/* must be owner */
708+
if (!pg_database_ownercheck(db_id, GetUserId()))
709+
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_DATABASE,
710+
oldname);
711+
712+
/* must have createdb rights */
713+
if (!have_createdb_privilege())
714+
ereport(ERROR,
715+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
716+
errmsg("permission denied to rename database")));
717+
718+
/*
719+
* Make sure the new name doesn't exist. See notes for same error in
720+
* CREATE DATABASE.
721+
*/
722+
if (OidIsValid(get_database_oid(newname)))
723+
ereport(ERROR,
724+
(errcode(ERRCODE_DUPLICATE_DATABASE),
725+
errmsg("database \"%s\" already exists", newname)));
726+
702727
/*
703728
* XXX Client applications probably store the current database somewhere,
704729
* so renaming it could cause confusion. On the other hand, there may not
@@ -713,30 +738,15 @@ RenameDatabase(const char *oldname, const char *newname)
713738
/*
714739
* Make sure the database does not have active sessions. This is the same
715740
* concern as above, but applied to other sessions.
741+
*
742+
* As in CREATE DATABASE, check this after other error conditions.
716743
*/
717-
if (DatabaseCancelAutovacuumActivity(db_id, false))
744+
if (CheckOtherDBBackends(db_id))
718745
ereport(ERROR,
719746
(errcode(ERRCODE_OBJECT_IN_USE),
720747
errmsg("database \"%s\" is being accessed by other users",
721748
oldname)));
722749

723-
/* make sure the new name doesn't exist */
724-
if (OidIsValid(get_database_oid(newname)))
725-
ereport(ERROR,
726-
(errcode(ERRCODE_DUPLICATE_DATABASE),
727-
errmsg("database \"%s\" already exists", newname)));
728-
729-
/* must be owner */
730-
if (!pg_database_ownercheck(db_id, GetUserId()))
731-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_DATABASE,
732-
oldname);
733-
734-
/* must have createdb rights */
735-
if (!have_createdb_privilege())
736-
ereport(ERROR,
737-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
738-
errmsg("permission denied to rename database")));
739-
740750
/* rename */
741751
newtup = SearchSysCacheCopy(DATABASEOID,
742752
ObjectIdGetDatum(db_id),

‎src/backend/storage/ipc/procarray.c

Lines changed: 86 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
*
2424
*
2525
* IDENTIFICATION
26-
* $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.24 2007/04/03 16:34:36 tgl Exp $
26+
* $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.25 2007/06/01 19:38:07 tgl Exp $
2727
*
2828
*-------------------------------------------------------------------------
2929
*/
@@ -683,62 +683,6 @@ GetSnapshotData(Snapshot snapshot, bool serializable)
683683
return snapshot;
684684
}
685685

686-
/*
687-
* DatabaseCancelAutovacuumActivity -- are there any backends running in the
688-
* given DB, apart from autovacuum? If an autovacuum process is running on the
689-
* database, kill it and restart the counting.
690-
*
691-
* If 'ignoreMyself' is TRUE, ignore this particular backend while checking
692-
* for backends in the target database.
693-
*
694-
* This function is used to interlock DROP DATABASE against there being
695-
* any active backends in the target DB --- dropping the DB while active
696-
* backends remain would be a Bad Thing. Note that we cannot detect here
697-
* the possibility of a newly-started backend that is trying to connect
698-
* to the doomed database, so additional interlocking is needed during
699-
* backend startup.
700-
*/
701-
bool
702-
DatabaseCancelAutovacuumActivity(Oid databaseId, bool ignoreMyself)
703-
{
704-
ProcArrayStruct *arrayP = procArray;
705-
int index;
706-
int num;
707-
708-
restart:
709-
num = 0;
710-
711-
CHECK_FOR_INTERRUPTS();
712-
713-
LWLockAcquire(ProcArrayLock, LW_SHARED);
714-
715-
for (index = 0; index < arrayP->numProcs; index++)
716-
{
717-
PGPROC *proc = arrayP->procs[index];
718-
719-
if (proc->databaseId == databaseId)
720-
{
721-
if (ignoreMyself && proc == MyProc)
722-
continue;
723-
724-
num++;
725-
726-
if (proc->isAutovacuum)
727-
{
728-
/* an autovacuum -- kill it and restart */
729-
LWLockRelease(ProcArrayLock);
730-
kill(proc->pid, SIGINT);
731-
pg_usleep(100 * 1000); /* 100ms */
732-
goto restart;
733-
}
734-
}
735-
}
736-
737-
LWLockRelease(ProcArrayLock);
738-
739-
return (num != 0);
740-
}
741-
742686
/*
743687
* GetTransactionsInCommit -- Get the XIDs of transactions that are committing
744688
*
@@ -1005,6 +949,91 @@ CountUserBackends(Oid roleid)
1005949
return count;
1006950
}
1007951

952+
/*
953+
* CheckOtherDBBackends -- check for other backends running in the given DB
954+
*
955+
* If there are other backends in the DB, we will wait a maximum of 5 seconds
956+
* for them to exit. Autovacuum backends are encouraged to exit early by
957+
* sending them SIGINT, but normal user backends are just waited for.
958+
*
959+
* The current backend is always ignored; it is caller's responsibility to
960+
* check whether the current backend uses the given DB, if it's important.
961+
*
962+
* Returns TRUE if there are (still) other backends in the DB, FALSE if not.
963+
*
964+
* This function is used to interlock DROP DATABASE and related commands
965+
* against there being any active backends in the target DB --- dropping the
966+
* DB while active backends remain would be a Bad Thing. Note that we cannot
967+
* detect here the possibility of a newly-started backend that is trying to
968+
* connect to the doomed database, so additional interlocking is needed during
969+
* backend startup. The caller should normally hold an exclusive lock on the
970+
* target DB before calling this, which is one reason we mustn't wait
971+
* indefinitely.
972+
*/
973+
bool
974+
CheckOtherDBBackends(Oid databaseId)
975+
{
976+
ProcArrayStruct *arrayP = procArray;
977+
int tries;
978+
979+
/* 50 tries with 100ms sleep between tries makes 5 sec total wait */
980+
for (tries = 0; tries < 50; tries++)
981+
{
982+
bool found = false;
983+
int index;
984+
985+
CHECK_FOR_INTERRUPTS();
986+
987+
LWLockAcquire(ProcArrayLock, LW_SHARED);
988+
989+
for (index = 0; index < arrayP->numProcs; index++)
990+
{
991+
PGPROC *proc = arrayP->procs[index];
992+
993+
if (proc->databaseId != databaseId)
994+
continue;
995+
if (proc == MyProc)
996+
continue;
997+
998+
found = true;
999+
1000+
if (proc->isAutovacuum)
1001+
{
1002+
/* an autovacuum --- send it SIGINT before sleeping */
1003+
int autopid = proc->pid;
1004+
1005+
/*
1006+
* It's a bit awkward to release ProcArrayLock within the loop,
1007+
* but we'd probably better do so before issuing kill(). We
1008+
* have no idea what might block kill() inside the kernel...
1009+
*/
1010+
LWLockRelease(ProcArrayLock);
1011+
1012+
(void) kill(autopid, SIGINT); /* ignore any error */
1013+
1014+
break;
1015+
}
1016+
else
1017+
{
1018+
LWLockRelease(ProcArrayLock);
1019+
break;
1020+
}
1021+
}
1022+
1023+
/* if found is set, we released the lock within the loop body */
1024+
if (!found)
1025+
{
1026+
LWLockRelease(ProcArrayLock);
1027+
return false; /* no conflicting backends, so done */
1028+
}
1029+
1030+
/* else sleep and try again */
1031+
pg_usleep(100 * 1000L); /* 100ms */
1032+
}
1033+
1034+
return true; /* timed out, still conflicts */
1035+
}
1036+
10081037

10091038
#define XidCacheRemove(i) \
10101039
do { \

‎src/include/storage/procarray.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/storage/procarray.h,v 1.13 2007/04/03 16:34:36 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/storage/procarray.h,v 1.14 2007/06/01 19:38:07 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -32,11 +32,11 @@ extern bool HaveTransactionsInCommit(TransactionId *xids, int nxids);
3232
extern PGPROC *BackendPidGetProc(int pid);
3333
extern int BackendXidGetPid(TransactionId xid);
3434
extern bool IsBackendPid(int pid);
35-
extern bool DatabaseCancelAutovacuumActivity(Oid databaseId, bool ignoreMyself);
3635

3736
extern int CountActiveBackends(void);
3837
extern int CountDBBackends(Oid databaseid);
3938
extern int CountUserBackends(Oid roleid);
39+
extern bool CheckOtherDBBackends(Oid databaseId);
4040

4141
extern void XidCacheRemoveRunningXids(TransactionId xid,
4242
int nxids, TransactionId *xids);

‎src/pl/Makefile

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#
55
# Copyright (c) 1994, Regents of the University of California
66
#
7-
# $PostgreSQL: pgsql/src/pl/Makefile,v 1.25 2007/02/09 15:55:59 petere Exp $
7+
# $PostgreSQL: pgsql/src/pl/Makefile,v 1.26 2007/06/01 19:38:07 tgl Exp $
88
#
99
#-------------------------------------------------------------------------
1010

@@ -32,12 +32,9 @@ all install installdirs uninstall distprep:
3232
clean distclean maintainer-clean:
3333
@for dir in $(DIRS); do $(MAKE) -C $$dir $@; done
3434

35-
# We'd like check operations to run all the subtests before failing;
36-
# also insert a sleep to ensure the previous test backend exited before
37-
# we try to drop the regression database.
35+
# We'd like check operations to run all the subtests before failing.
3836
check installcheck:
3937
@CHECKERR=0; for dir in $(DIRS); do \
40-
sleep 1; \
4138
$(MAKE) -C $$dir $@ || CHECKERR=$$?; \
4239
done; \
4340
exit $$CHECKERR

0 commit comments

Comments
 (0)