Skip to content

Commit 0fc4ecf

Browse files
committed
Finish up the flat-files project: get rid of GetRawDatabaseInfo() hack
in favor of looking at the flat file copy of pg_database during backend startup. This should finally eliminate the various corner cases in which backend startup fails unexpectedly because it isn't able to distinguish live and dead tuples in pg_database. Simplify locking on pg_database to be similar to the rules used with pg_shadow and pg_group, and eliminate FlushRelationBuffers operations that were used only to reduce the odds of failure of GetRawDatabaseInfo. initdb forced due to addition of a trigger to pg_database.
1 parent ffef9a9 commit 0fc4ecf

File tree

12 files changed

+184
-316
lines changed

12 files changed

+184
-316
lines changed

src/backend/commands/dbcommands.c

Lines changed: 29 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,19 @@
33
* dbcommands.c
44
* Database management commands (create/drop database).
55
*
6+
* Note: database creation/destruction commands take ExclusiveLock on
7+
* pg_database to ensure that no two proceed in parallel. We must use
8+
* at least this level of locking to ensure that no two backends try to
9+
* write the flat-file copy of pg_database at once. We avoid using
10+
* AccessExclusiveLock since there's no need to lock out ordinary readers
11+
* of pg_database.
612
*
713
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
814
* Portions Copyright (c) 1994, Regents of the University of California
915
*
1016
*
1117
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.150 2005/02/20 02:21:34 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.151 2005/02/26 18:43:33 tgl Exp $
1319
*
1420
*-------------------------------------------------------------------------
1521
*/
@@ -446,13 +452,13 @@ createdb(const CreatedbStmt *stmt)
446452
/*
447453
* Now OK to grab exclusive lock on pg_database.
448454
*/
449-
pg_database_rel = heap_openr(DatabaseRelationName, AccessExclusiveLock);
455+
pg_database_rel = heap_openr(DatabaseRelationName, ExclusiveLock);
450456

451457
/* Check to see if someone else created same DB name meanwhile. */
452458
if (get_db_info(dbname, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL))
453459
{
454460
/* Don't hold lock while doing recursive remove */
455-
heap_close(pg_database_rel, AccessExclusiveLock);
461+
heap_close(pg_database_rel, ExclusiveLock);
456462
remove_dbtablespaces(dboid);
457463
ereport(ERROR,
458464
(errcode(ERRCODE_DUPLICATE_DATABASE),
@@ -498,13 +504,6 @@ createdb(const CreatedbStmt *stmt)
498504
/* Update indexes */
499505
CatalogUpdateIndexes(pg_database_rel, tuple);
500506

501-
/*
502-
* Force dirty buffers out to disk, so that newly-connecting backends
503-
* will see the new database in pg_database right away. (They'll see
504-
* an uncommitted tuple, but they don't care; see GetRawDatabaseInfo.)
505-
*/
506-
FlushRelationBuffers(pg_database_rel, MaxBlockNumber);
507-
508507
/* Close pg_database, but keep exclusive lock till commit */
509508
heap_close(pg_database_rel, NoLock);
510509

@@ -542,12 +541,15 @@ dropdb(const char *dbname)
542541
* Obtain exclusive lock on pg_database. We need this to ensure that
543542
* no new backend starts up in the target database while we are
544543
* deleting it. (Actually, a new backend might still manage to start
545-
* up, because it will read pg_database without any locking to
546-
* discover the database's OID. But it will detect its error in
547-
* ReverifyMyDatabase and shut down before any serious damage is done.
548-
* See postinit.c.)
544+
* up, because it isn't able to lock pg_database while starting. But
545+
* it will detect its error in ReverifyMyDatabase and shut down before
546+
* any serious damage is done. See postinit.c.)
547+
*
548+
* An ExclusiveLock, rather than AccessExclusiveLock, is sufficient
549+
* since ReverifyMyDatabase takes RowShareLock. This allows ordinary
550+
* readers of pg_database to proceed in parallel.
549551
*/
550-
pgdbrel = heap_openr(DatabaseRelationName, AccessExclusiveLock);
552+
pgdbrel = heap_openr(DatabaseRelationName, ExclusiveLock);
551553

552554
if (!get_db_info(dbname, &db_id, &db_owner, NULL,
553555
&db_istemplate, NULL, NULL, NULL, NULL))
@@ -638,14 +640,6 @@ dropdb(const char *dbname)
638640
*/
639641
remove_dbtablespaces(db_id);
640642

641-
/*
642-
* Force dirty buffers out to disk, so that newly-connecting backends
643-
* will see the database tuple marked dead in pg_database right away.
644-
* (They'll see an uncommitted deletion, but they don't care; see
645-
* GetRawDatabaseInfo.)
646-
*/
647-
FlushRelationBuffers(pgdbrel, MaxBlockNumber);
648-
649643
/* Close pg_database, but keep exclusive lock till commit */
650644
heap_close(pgdbrel, NoLock);
651645

@@ -671,10 +665,10 @@ RenameDatabase(const char *oldname, const char *newname)
671665
key2;
672666

673667
/*
674-
* Obtain AccessExclusiveLock so that no new session gets started
668+
* Obtain ExclusiveLock so that no new session gets started
675669
* while the rename is in progress.
676670
*/
677-
rel = heap_openr(DatabaseRelationName, AccessExclusiveLock);
671+
rel = heap_openr(DatabaseRelationName, ExclusiveLock);
678672

679673
ScanKeyInit(&key,
680674
Anum_pg_database_datname,
@@ -742,14 +736,6 @@ RenameDatabase(const char *oldname, const char *newname)
742736

743737
systable_endscan(scan);
744738

745-
/*
746-
* Force dirty buffers out to disk, so that newly-connecting backends
747-
* will see the renamed database in pg_database right away. (They'll
748-
* see an uncommitted tuple, but they don't care; see
749-
* GetRawDatabaseInfo.)
750-
*/
751-
FlushRelationBuffers(rel, MaxBlockNumber);
752-
753739
/* Close pg_database, but keep exclusive lock till commit */
754740
heap_close(rel, NoLock);
755741

@@ -779,9 +765,10 @@ AlterDatabaseSet(AlterDatabaseSetStmt *stmt)
779765
valuestr = flatten_set_variable_args(stmt->variable, stmt->value);
780766

781767
/*
782-
* We need AccessExclusiveLock so we can safely do FlushRelationBuffers.
768+
* We don't need ExclusiveLock since we aren't updating the
769+
* flat file.
783770
*/
784-
rel = heap_openr(DatabaseRelationName, AccessExclusiveLock);
771+
rel = heap_openr(DatabaseRelationName, RowExclusiveLock);
785772
ScanKeyInit(&scankey,
786773
Anum_pg_database_datname,
787774
BTEqualStrategyNumber, F_NAMEEQ,
@@ -840,15 +827,7 @@ AlterDatabaseSet(AlterDatabaseSetStmt *stmt)
840827

841828
systable_endscan(scan);
842829

843-
/*
844-
* Force dirty buffers out to disk, so that newly-connecting backends
845-
* will see the altered row in pg_database right away. (They'll
846-
* see an uncommitted tuple, but they don't care; see
847-
* GetRawDatabaseInfo.)
848-
*/
849-
FlushRelationBuffers(rel, MaxBlockNumber);
850-
851-
/* Close pg_database, but keep exclusive lock till commit */
830+
/* Close pg_database, but keep lock till commit */
852831
heap_close(rel, NoLock);
853832

854833
/*
@@ -871,9 +850,10 @@ AlterDatabaseOwner(const char *dbname, AclId newOwnerSysId)
871850
Form_pg_database datForm;
872851

873852
/*
874-
* We need AccessExclusiveLock so we can safely do FlushRelationBuffers.
853+
* We don't need ExclusiveLock since we aren't updating the
854+
* flat file.
875855
*/
876-
rel = heap_openr(DatabaseRelationName, AccessExclusiveLock);
856+
rel = heap_openr(DatabaseRelationName, RowExclusiveLock);
877857
ScanKeyInit(&scankey,
878858
Anum_pg_database_datname,
879859
BTEqualStrategyNumber, F_NAMEEQ,
@@ -937,22 +917,11 @@ AlterDatabaseOwner(const char *dbname, AclId newOwnerSysId)
937917
CatalogUpdateIndexes(rel, newtuple);
938918

939919
heap_freetuple(newtuple);
940-
941-
/* must release buffer pins before FlushRelationBuffers */
942-
systable_endscan(scan);
943-
944-
/*
945-
* Force dirty buffers out to disk, so that newly-connecting backends
946-
* will see the altered row in pg_database right away. (They'll
947-
* see an uncommitted tuple, but they don't care; see
948-
* GetRawDatabaseInfo.)
949-
*/
950-
FlushRelationBuffers(rel, MaxBlockNumber);
951920
}
952-
else
953-
systable_endscan(scan);
954921

955-
/* Close pg_database, but keep exclusive lock till commit */
922+
systable_endscan(scan);
923+
924+
/* Close pg_database, but keep lock till commit */
956925
heap_close(rel, NoLock);
957926

958927
/*

src/backend/commands/vacuum.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.301 2005/02/20 02:21:34 tgl Exp $
16+
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.302 2005/02/26 18:43:33 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -704,11 +704,12 @@ vac_update_relstats(Oid relid, BlockNumber num_pages, double num_tuples,
704704
* We violate no-overwrite semantics here by storing new values for the
705705
* statistics columns directly into the tuple that's already on the page.
706706
* As with vac_update_relstats, this avoids leaving dead tuples behind
707-
* after a VACUUM; which is good since GetRawDatabaseInfo
708-
* can get confused by finding dead tuples in pg_database.
707+
* after a VACUUM.
709708
*
710709
* This routine is shared by full and lazy VACUUM. Note that it is only
711710
* applied after a database-wide VACUUM operation.
711+
*
712+
* Note that we don't bother to update the flat-file copy of pg_database.
712713
*/
713714
static void
714715
vac_update_dbstats(Oid dbid,

src/backend/libpq/hba.c

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/libpq/hba.c,v 1.139 2005/02/20 04:45:57 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/libpq/hba.c,v 1.140 2005/02/26 18:43:33 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
1717
#include "postgres.h"
1818

19+
#include <ctype.h>
1920
#include <pwd.h>
2021
#include <fcntl.h>
2122
#include <sys/param.h>
@@ -37,6 +38,8 @@
3738
#include "utils/guc.h"
3839

3940

41+
#define atooid(x) ((Oid) strtoul((x), NULL, 10))
42+
4043
/* Max size of username ident server can return */
4144
#define IDENT_USERNAME_MAX 512
4245

@@ -1059,6 +1062,51 @@ load_hba(void)
10591062
FreeFile(file);
10601063
}
10611064

1065+
/*
1066+
* Read and parse one line from the flat pg_database file.
1067+
*
1068+
* Returns TRUE on success, FALSE if EOF; bad data causes elog(FATAL).
1069+
*
1070+
* Output parameters:
1071+
* dbname: gets database name (must be of size NAMEDATALEN bytes)
1072+
* dboid: gets database OID
1073+
* dbtablespace: gets database's default tablespace's OID
1074+
*
1075+
* This is not much related to the other functions in hba.c, but we put it
1076+
* here because it uses the next_token() infrastructure.
1077+
*/
1078+
bool
1079+
read_pg_database_line(FILE *fp, char *dbname,
1080+
Oid *dboid, Oid *dbtablespace)
1081+
{
1082+
char buf[MAX_TOKEN];
1083+
1084+
if (feof(fp))
1085+
return false;
1086+
next_token(fp, buf, sizeof(buf));
1087+
if (!buf[0])
1088+
return false;
1089+
if (strlen(buf) >= NAMEDATALEN)
1090+
elog(FATAL, "bad data in flat pg_database file");
1091+
strcpy(dbname, buf);
1092+
next_token(fp, buf, sizeof(buf));
1093+
if (!isdigit((unsigned char) buf[0]))
1094+
elog(FATAL, "bad data in flat pg_database file");
1095+
*dboid = atooid(buf);
1096+
next_token(fp, buf, sizeof(buf));
1097+
if (!isdigit((unsigned char) buf[0]))
1098+
elog(FATAL, "bad data in flat pg_database file");
1099+
*dbtablespace = atooid(buf);
1100+
/* discard datfrozenxid */
1101+
next_token(fp, buf, sizeof(buf));
1102+
if (!isdigit((unsigned char) buf[0]))
1103+
elog(FATAL, "bad data in flat pg_database file");
1104+
/* expect EOL next */
1105+
next_token(fp, buf, sizeof(buf));
1106+
if (buf[0])
1107+
elog(FATAL, "bad data in flat pg_database file");
1108+
return true;
1109+
}
10621110

10631111
/*
10641112
* Process one line from the ident config file.

src/backend/utils/init/flatfiles.c

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
2323
* Portions Copyright (c) 1994, Regents of the University of California
2424
*
25-
* $PostgreSQL: pgsql/src/backend/utils/init/flatfiles.c,v 1.3 2005/02/20 22:02:19 tgl Exp $
25+
* $PostgreSQL: pgsql/src/backend/utils/init/flatfiles.c,v 1.4 2005/02/26 18:43:33 tgl Exp $
2626
*
2727
*-------------------------------------------------------------------------
2828
*/
@@ -243,10 +243,12 @@ write_database_file(Relation drel)
243243
Form_pg_database dbform = (Form_pg_database) GETSTRUCT(tuple);
244244
char *datname;
245245
Oid datoid;
246+
Oid dattablespace;
246247
TransactionId datfrozenxid;
247248

248249
datname = NameStr(dbform->datname);
249250
datoid = HeapTupleGetOid(tuple);
251+
dattablespace = dbform->dattablespace;
250252
datfrozenxid = dbform->datfrozenxid;
251253

252254
/*
@@ -276,13 +278,13 @@ write_database_file(Relation drel)
276278
}
277279

278280
/*
279-
* The file format is: "dbname" oid frozenxid
281+
* The file format is: "dbname" oid tablespace frozenxid
280282
*
281283
* The xid is not needed for backend startup, but may be of use
282284
* for forensic purposes.
283285
*/
284286
fputs_quote(datname, fp);
285-
fprintf(fp, " %u %u\n", datoid, datfrozenxid);
287+
fprintf(fp, " %u %u %u\n", datoid, dattablespace, datfrozenxid);
286288
}
287289
heap_endscan(scan);
288290

@@ -830,15 +832,3 @@ flatfile_update_trigger(PG_FUNCTION_ARGS)
830832

831833
return PointerGetDatum(NULL);
832834
}
833-
834-
835-
/*
836-
* Old version of trigger --- remove after we can force an initdb
837-
*/
838-
extern Datum update_pg_pwd_and_pg_group(PG_FUNCTION_ARGS);
839-
840-
Datum
841-
update_pg_pwd_and_pg_group(PG_FUNCTION_ARGS)
842-
{
843-
return flatfile_update_trigger(fcinfo);
844-
}

0 commit comments

Comments
 (0)