Re: PITR failing to stop before DROP DATABASE

From: Christoph Berg <cb(at)df7cb(dot)de>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PITR failing to stop before DROP DATABASE
Date: 2014-11-26 11:35:27
Message-ID: 20141126113527.GC20042@msg.df7cb.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Re: Heikki Linnakangas 2014-11-26 <54759BC0(dot)4070505(at)vmware(dot)com>
> >Oh ok. So this is an artifact of the non-transactionality (is this a
> >word?) of CREATE DATABASE.
>
> DROP DATABASE. CREATE DATABASE is a different story. It does similar
> non-transactional tricks and has similar issues, but it's a completely
> different codepath and could be fixed independently of DROP DATABASE.

Err right. Too early in the morning...

> >So my suggestion for a simple fix would be to make DROP DATABASE
> >execute a short fake transaction before it starts deleting files and
> >then continue as before. This would serve as a stopping point for
> >recovery_target_time to run into. (We could still fix this properly
> >later, but this idea seems like a good fix for a practical problem
> >that doesn't break anything else.)
>
> Yeah, seems reasonable.

Here's a first shot at a patch. It's not working yet because I think
the commit isn't doing anything because no work was done in the
transaction yet.

*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
*************** dropdb(const char *dbname, bool missing_
*** 778,783 ****
--- 778,798 ----
nslots_active;

/*
+ * Commit now to cause a commit xlog record to be logged. (We are outside
+ * any transaction so this is safe to do.) If we don't do this here, doing
+ * a PITR restore to just before DROP DATABASE will cause the files on disk
+ * to be deleted, while PITR stops before removing the database from the
+ * system catalogs, so the database is still visible while it is in fact
+ * already deleted. It is still possible to get to this intermediate state
+ * by selecting the correct transaction number in recovery.conf, but this
+ * fixes the common use case of specifying a recovery target time just
+ * before DROP DATABASE.
+ */
+ PopActiveSnapshot();
+ CommitTransactionCommand();
+ StartTransactionCommand();
+
+ /*
* Look up the target database's OID, and get exclusive lock on it. We
* need this to ensure that no new backend starts up in the target
* database while we are deleting it (see postinit.c), and that no one is

Christoph
--
cb(at)df7cb(dot)de | http://www.df7cb.de/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2014-11-26 11:41:41 Re: [pgsql-packagers] Palle Girgensohn's ICU patch
Previous Message Ants Aasma 2014-11-26 11:34:18 Using RTLD_DEEPBIND to handle symbol conflicts in loaded libraries