On 7/12/16 9:55 PM, Masahiko Sawada wrote:
> And what I think is pg_baseback never remove the directory specified
> by -D option even if execution is failed. initdb command behaves so.
> I think it's helpful for backup operation.
This has been bothering me as well.
How about the attached patch as a start?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 910310b7eab88af8972906307a55e02e64618da7 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 18 Aug 2016 12:00:00 -0400
Subject: [PATCH] pg_basebackup: Clean created directories on failure
Like initdb, clean up created data and xlog directories, unless the new
-n/--noclean option is specified.
Tablespace directories are not cleaned up, but a message is written
about that.
---
doc/src/sgml/ref/pg_basebackup.sgml | 18 +++++
src/bin/pg_basebackup/pg_basebackup.c | 98 ++++++++++++++++++++++++++--
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 10 ++-
3 files changed, 119 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 03615da..9f1eae1 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -399,6 +399,24 @@ <title>Options</title>
</varlistentry>
<varlistentry>
+ <term><option>-n</option></term>
+ <term><option>--noclean</option></term>
+ <listitem>
+ <para>
+ By default, when <command>pg_basebackup</command> aborts with an
+ error, it removes any directories it might have created before
+ discovering that it cannot finish the job (for example, data directory
+ and transaction log directory). This option inhibits tidying-up and is
+ thus useful for debugging.
+ </para>
+
+ <para>
+ Note that tablespace directories are not cleaned up either way.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-P</option></term>
<term><option>--progress</option></term>
<listitem>
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index ed41db8..d13a9a3 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -57,6 +57,7 @@ static TablespaceList tablespace_dirs = {NULL, NULL};
static char *xlog_dir = "";
static char format = 'p'; /* p(lain)/t(ar) */
static char *label = "pg_basebackup base backup";
+static bool noclean = false;
static bool showprogress = false;
static int verbose = 0;
static int compresslevel = 0;
@@ -68,6 +69,13 @@ static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
static pg_time_t last_progress_report = 0;
static int32 maxrate = 0; /* no limit by default */
+static bool success = false;
+static bool made_new_pgdata = false;
+static bool found_existing_pgdata = false;
+static bool made_new_xlogdir = false;
+static bool found_existing_xlogdir = false;
+static bool made_tablespace_dirs = false;
+static bool found_tablespace_dirs = false;
/* Progress counters */
static uint64 totalsize;
@@ -81,6 +89,7 @@ static int bgpipe[2] = {-1, -1};
/* Handle to child process */
static pid_t bgchild = -1;
+static bool in_log_streamer = false;
/* End position for xlog streaming, empty string if unknown yet */
static XLogRecPtr xlogendptr;
@@ -97,7 +106,7 @@ static PQExpBuffer recoveryconfcontents = NULL;
/* Function headers */
static void usage(void);
static void disconnect_and_exit(int code);
-static void verify_dir_is_empty_or_create(char *dirname);
+static void verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found);
static void progress_report(int tablespacenum, const char *filename, bool force);
static void ReceiveTarFile(PGconn *conn, PGresult *res, int rownum);
@@ -114,6 +123,69 @@ static void tablespace_list_append(const char *arg);
static void
+cleanup_directories_atexit(void)
+{
+ if (success || in_log_streamer)
+ return;
+
+ if (!noclean)
+ {
+ if (made_new_pgdata)
+ {
+ fprintf(stderr, _("%s: removing data directory \"%s\"\n"),
+ progname, basedir);
+ if (!rmtree(basedir, true))
+ fprintf(stderr, _("%s: failed to remove data directory\n"),
+ progname);
+ }
+ else if (found_existing_pgdata)
+ {
+ fprintf(stderr,
+ _("%s: removing contents of data directory \"%s\"\n"),
+ progname, basedir);
+ if (!rmtree(basedir, false))
+ fprintf(stderr, _("%s: failed to remove contents of data directory\n"),
+ progname);
+ }
+
+ if (made_new_xlogdir)
+ {
+ fprintf(stderr, _("%s: removing transaction log directory \"%s\"\n"),
+ progname, xlog_dir);
+ if (!rmtree(xlog_dir, true))
+ fprintf(stderr, _("%s: failed to remove transaction log directory\n"),
+ progname);
+ }
+ else if (found_existing_xlogdir)
+ {
+ fprintf(stderr,
+ _("%s: removing contents of transaction log directory \"%s\"\n"),
+ progname, xlog_dir);
+ if (!rmtree(xlog_dir, false))
+ fprintf(stderr, _("%s: failed to remove contents of transaction log directory\n"),
+ progname);
+ }
+ }
+ else
+ {
+ if (made_new_pgdata || found_existing_pgdata)
+ fprintf(stderr,
+ _("%s: data directory \"%s\" not removed at user's request\n"),
+ progname, basedir);
+
+ if (made_new_xlogdir || found_existing_xlogdir)
+ fprintf(stderr,
+ _("%s: transaction log directory \"%s\" not removed at user's request\n"),
+ progname, xlog_dir);
+ }
+
+ if (made_tablespace_dirs || found_tablespace_dirs)
+ fprintf(stderr,
+ _("%s: changes to tablespace directories will not be undone"),
+ progname);
+}
+
+static void
disconnect_and_exit(int code)
{
if (conn != NULL)
@@ -252,6 +324,7 @@ usage(void)
printf(_(" -c, --checkpoint=fast|spread\n"
" set fast or spread checkpointing\n"));
printf(_(" -l, --label=LABEL set backup label\n"));
+ printf(_(" -n, --noclean do not clean up after errors"));
printf(_(" -P, --progress show progress information\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -374,6 +447,8 @@ LogStreamerMain(logstreamer_param *param)
{
StreamCtl stream;
+ in_log_streamer = true;
+
MemSet(&stream, 0, sizeof(stream));
stream.startpos = param->startptr;
stream.timeline = param->timeline;
@@ -500,7 +575,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier)
* be give and the process ended.
*/
static void
-verify_dir_is_empty_or_create(char *dirname)
+verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found)
{
switch (pg_check_dir(dirname))
{
@@ -516,12 +591,16 @@ verify_dir_is_empty_or_create(char *dirname)
progname, dirname, strerror(errno));
disconnect_and_exit(1);
}
+ if (created)
+ *created = true;
return;
case 1:
/*
* Exists, empty
*/
+ if (found)
+ *found = true;
return;
case 2:
case 3:
@@ -1746,7 +1825,7 @@ BaseBackup(void)
{
char *path = (char *) get_tablespace_mapping(PQgetvalue(res, i, 1));
- verify_dir_is_empty_or_create(path);
+ verify_dir_is_empty_or_create(path, &made_tablespace_dirs, &found_tablespace_dirs);
}
}
@@ -1955,6 +2034,7 @@ main(int argc, char **argv)
{"gzip", no_argument, NULL, 'z'},
{"compress", required_argument, NULL, 'Z'},
{"label", required_argument, NULL, 'l'},
+ {"noclean", no_argument, NULL, 'n'},
{"dbname", required_argument, NULL, 'd'},
{"host", required_argument, NULL, 'h'},
{"port", required_argument, NULL, 'p'},
@@ -1989,7 +2069,9 @@ main(int argc, char **argv)
}
}
- while ((c = getopt_long(argc, argv, "D:F:r:RT:xX:l:zZ:d:c:h:p:U:s:S:wWvP",
+ atexit(cleanup_directories_atexit);
+
+ while ((c = getopt_long(argc, argv, "D:F:r:RT:xX:l:nzZ:d:c:h:p:U:s:S:wWvP",
long_options, &option_index)) != -1)
{
switch (c)
@@ -2064,6 +2146,9 @@ main(int argc, char **argv)
case 'l':
label = pg_strdup(optarg);
break;
+ case 'n':
+ noclean = true;
+ break;
case 'z':
#ifdef HAVE_LIBZ
compresslevel = Z_DEFAULT_COMPRESSION;
@@ -2233,14 +2318,14 @@ main(int argc, char **argv)
* unless we are writing to stdout.
*/
if (format == 'p' || strcmp(basedir, "-") != 0)
- verify_dir_is_empty_or_create(basedir);
+ verify_dir_is_empty_or_create(basedir, &made_new_pgdata, &found_existing_pgdata);
/* Create transaction log symlink, if required */
if (strcmp(xlog_dir, "") != 0)
{
char *linkloc;
- verify_dir_is_empty_or_create(xlog_dir);
+ verify_dir_is_empty_or_create(xlog_dir, &made_new_xlogdir, &found_existing_xlogdir);
/* form name of the place where the symlink must go */
linkloc = psprintf("%s/pg_xlog", basedir);
@@ -2261,5 +2346,6 @@ main(int argc, char **argv)
BaseBackup();
+ success = true;
return 0;
}
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 6c33936..fd9857d 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -4,7 +4,7 @@
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 51;
+use Test::More tests => 54;
program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup');
@@ -40,6 +40,14 @@
[ 'pg_basebackup', '-D', "$tempdir/backup" ],
'pg_basebackup fails because of WAL configuration');
+ok(! -d "$tempdir/backup", 'backup directory was cleaned up');
+
+$node->command_fails(
+ [ 'pg_basebackup', '-D', "$tempdir/backup", '-n' ],
+ 'failing run with noclean option');
+
+ok(-d "$tempdir/backup", 'backup directory was created and left behind');
+
open CONF, ">>$pgdata/postgresql.conf";
print CONF "max_replication_slots = 10\n";
print CONF "max_wal_senders = 10\n";
--
2.9.3
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
