Re: Rethinking locking for database create/drop vs connection startup

This is motivated by Jim Buttafuoco's recent gripe about not being
able to connect while a DROP DATABASE is in progress:
http://archives.postgresql.org/pgsql-hackers/2006-05/msg00074.php
The whole area is really pretty grotty anyway --- the existing interlock
does not prevent an incoming connection from trying to connect to the
victim database, only make sure that we detect it later. This is not
very good, for two reasons. One is that you'll most likely get a very
unfriendly error message due to attempts to access already-missing
system catalogs; when I experimented just now I got

psql: FATAL: could not open relation 1663/104854/1259: No such file or directory

which is really not the way I'd like to report "database foo just got
deleted". The other problem is that I'm not entirely convinced that a
backend trying to do this won't leave any permanent problems behind,
most likely in the form of dirty shared buffers for subsequently-deleted
system catalogs in the victim database. ReverifyMyDatabase tries to
clean that up by doing DropDatabaseBuffers, but that only helps if we
get as far as ReverifyMyDatabase.

It strikes me that we now have a decent tool for solving the problem,
which is LockSharedObject() --- that is, there exists a locktag
convention whereby we can "take a lock" on a database as such, rather
than having to use table-level locks on pg_database as proxy. The
locktag would be in the form of an OID so it would identify a DB by
OID. If dropdb() takes such a lock before it checks for active
backends, then the connection sequence can look like this:

1. read pg_database flat file to find out OID of target DB
2. initialize far enough to be able to start a transaction,
and do so
3. take a shared lock on the target DB by OID
4. re-read pg_database flat file and verify DB still exists

If step 4 fails to find the DB in the flat file, then we can bomb
out before we've made any attempt to touch catalogs of the target
DB. This ensures both a reasonable error message, and no pollution
of shared buffers. If we get past step 4 then we don't have to worry
about concurrent dropdb() anymore. (The shared lock will only last
until we commit the startup transaction, but that's OK --- once we
are listed in the PGPROC array we don't need the lock anymore.)

It's slightly annoying to have to read the flat file twice, but
for reasonable numbers of databases per installation I don't think
this will pose any material performance penalty. The file will
certainly still be sitting in kernel disk cache.

It's still necessary to serialize CREATE/DROP DATABASE commands against
each other, to ensure that only one backend tries to write the flat file
at a time, but with this scheme they'd not need to block connections
being made to unrelated databases.

Thoughts, better ideas?

regards, tom lane

> It's slightly annoying to have to read the flat file twice, but
> for reasonable numbers of databases per installation I don't think
> this will pose any material performance penalty. The file will
> certainly still be sitting in kernel disk cache.

Dropping a db isn't exactly a "common" occurrence anyway no?

On Wed, 2006-05-03 at 16:15 -0400, Tom Lane wrote:
> This is motivated by Jim Buttafuoco's recent gripe about not being
> able to connect while a DROP DATABASE is in progress:
> http://archives.postgresql.org/pgsql-hackers/2006-05/msg00074.php

...

> If dropdb() takes such a lock before it checks for active
> backends, then the connection sequence can look like this:
>
> 1. read pg_database flat file to find out OID of target DB
> 2. initialize far enough to be able to start a transaction,
> and do so
> 3. take a shared lock on the target DB by OID
> 4. re-read pg_database flat file and verify DB still exists

Many people never CREATE or DROP databases. They just do everything in
the default database (name is release dependent) - at least on their
main system(s). It would be valid to optimize for that case.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
> On Wed, 2006-05-03 at 16:15 -0400, Tom Lane wrote:
>> If dropdb() takes such a lock before it checks for active
>> backends, then the connection sequence can look like this:

> Many people never CREATE or DROP databases. They just do everything in
> the default database (name is release dependent) - at least on their
> main system(s). It would be valid to optimize for that case.

I'm not particularly concerned about people with only a couple of
databases --- reading the flat file isn't going to take any meaningful
amount of time for them anyway. It's the folks with hundreds of
databases who might have a beef. But those are exactly the people
who need create/drop database to be bulletproof.

As I've been working on this patch I've found that it will clean up a
whole lot of related issues, so I'm getting more and more convinced
it's the Right Thing. Some points:

* Connecting will actually take RowExclusiveLock (ordinary writer's
lock), while CREATE DATABASE takes ShareLock on the template DB, and
of course DROP/RENAME DATABASE take AccessExclusiveLock. This provides
for the first time an absolute guarantee that CREATE DATABASE gets a
consistent copy of the template: before we could never ensure that
someone didn't connect to the template and change it while the copy was
in progress. At the same time, two CREATE DATABASEs can safely use the
same template, and of course two concurrent connections don't block
each other.

* Since we're trying not to take any table-level exclusive locks on
pg_database anymore, we need a different solution in flatfiles.c to
ensure only one transaction writes the flat file at a time. To do this
I'm going to have a dedicated lock, used only in the flatfile code, that
is taken just before trying to write the file and held till commit
(which is immediately after). This eliminates the former risk of
deadlock associated with manual updates to pg_database, and as a bonus
holds the exclusive lock for a much shorter period of time.

regards, tom lane