pg_combinebackup --incremental
Hi,
When I gave my talk on pg_basebackup at pgconf.eu, people took the
opportunity to mention various improvements which they would find
useful. One of those improvements was the ability to combine several
incremental backups into one. This might be useful if you take very
frequent incremental backups but want to roll up older ones to reduce
storage requirements. For example, you could take an incremental
backup every hour, but then once more than a day has gone by, you
might roll up those up six at a time into bigger incremental backups
so that you store 4 backups per day instead of 24 backups per day.
The attached patch set lets you do this. I have only tested it a
little bit, and before it gets committed, it needs substantially more
testing, as well as code review. I plan to write some TAP tests to
include in the patch, too, but I haven't done that yet. The idea is
that instead of doing this:
$ pg_combinebackup full incr1 incr2 incr3 incr4 incr5 -o result
You could instead do this:
$ pg_combinebackup -i incr1 incr2 incr3 -o incr1-3
$ rm -rf incr{1,2,3}
<time passes>
$ pg_combinebackup full incr1-3 incr5 -o result
If you're interested in this feature, please give this a try and let
me know what you find out!
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v1-0001-pg_combinebackup-Factor-some-code-out-of-write_re.patchapplication/octet-stream; name=v1-0001-pg_combinebackup-Factor-some-code-out-of-write_re.patchDownload
From 6f1734cc0e604d39b8ef17e3fa5d914ecaee07b9 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Thu, 31 Oct 2024 14:23:09 -0400
Subject: [PATCH v1 1/2] pg_combinebackup: Factor some code out of
write_reconstructed_file.
The act of copying a single block moves to a new function copy_block(),
and the logic to generate the detailed debugging output is moves to into
a new function debug_reconstruction_plan(). This is preparatory
refactoring for a future patch, but it's not unreasonable on its own
terms, as write_reconstructed_file() has become a fairly lengthy
function.
---
src/bin/pg_combinebackup/reconstruct.c | 246 ++++++++++++++-----------
1 file changed, 141 insertions(+), 105 deletions(-)
diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c
index 37ae38b6108..17375878e8e 100644
--- a/src/bin/pg_combinebackup/reconstruct.c
+++ b/src/bin/pg_combinebackup/reconstruct.c
@@ -49,6 +49,9 @@ typedef struct rfile
static void debug_reconstruction(int n_source,
rfile **sources,
bool dry_run);
+static void debug_reconstruction_plan(unsigned block_length,
+ rfile **sourcemap,
+ off_t *offsetmap);
static unsigned find_reconstructed_block_length(rfile *s);
static rfile *make_incremental_rfile(char *filename);
static rfile *make_rfile(char *filename, bool missing_ok);
@@ -61,6 +64,9 @@ static void write_reconstructed_file(char *input_filename,
CopyMethod copy_method,
bool debug,
bool dry_run);
+static void copy_block(int fd, char *output_filename, rfile *source,
+ off_t offset, CopyMethod copy_method,
+ pg_checksum_context *checksum_ctx);
static void read_bytes(rfile *rf, void *buffer, unsigned length);
static void write_block(int fd, char *output_filename,
uint8 *buffer,
@@ -428,6 +434,68 @@ debug_reconstruction(int n_source, rfile **sources, bool dry_run)
}
}
+/*
+ * Print out information about the source of each reconstructed block.
+ */
+static void
+debug_reconstruction_plan(unsigned block_length, rfile **sourcemap,
+ off_t *offsetmap)
+{
+ StringInfoData debug_buf;
+ unsigned start_of_range = 0;
+ unsigned current_block = 0;
+
+ /* Print out the plan for reconstructing this file. */
+ initStringInfo(&debug_buf);
+ while (current_block < block_length)
+ {
+ rfile *s = sourcemap[current_block];
+
+ /* Extend range, if possible. */
+ if (current_block + 1 < block_length &&
+ s == sourcemap[current_block + 1])
+ {
+ ++current_block;
+ continue;
+ }
+
+ /* Add details about this range. */
+ if (s == NULL)
+ {
+ if (current_block == start_of_range)
+ appendStringInfo(&debug_buf, " %u:zero", current_block);
+ else
+ appendStringInfo(&debug_buf, " %u-%u:zero",
+ start_of_range, current_block);
+ }
+ else
+ {
+ if (current_block == start_of_range)
+ appendStringInfo(&debug_buf, " %u:%s@" UINT64_FORMAT,
+ current_block, s->filename,
+ (uint64) offsetmap[current_block]);
+ else
+ appendStringInfo(&debug_buf, " %u-%u:%s@" UINT64_FORMAT,
+ start_of_range, current_block,
+ s->filename,
+ (uint64) offsetmap[current_block]);
+ }
+
+ /* Begin new range. */
+ start_of_range = ++current_block;
+
+ /* If the output is very long or we are done, dump it now. */
+ if (current_block == block_length || debug_buf.len > 1024)
+ {
+ pg_log_debug("reconstruction plan:%s", debug_buf.data);
+ resetStringInfo(&debug_buf);
+ }
+ }
+
+ /* Free memory. */
+ pfree(debug_buf.data);
+}
+
/*
* When we perform reconstruction using an incremental file, the output file
* should be at least as long as the truncation_block_length. Any blocks
@@ -565,10 +633,6 @@ write_reconstructed_file(char *input_filename,
/* Debugging output. */
if (debug)
{
- StringInfoData debug_buf;
- unsigned start_of_range = 0;
- unsigned current_block = 0;
-
/* Basic information about the output file to be produced. */
if (dry_run)
pg_log_debug("would reconstruct \"%s\" (%u blocks, checksum %s)",
@@ -579,55 +643,8 @@ write_reconstructed_file(char *input_filename,
output_filename, block_length,
pg_checksum_type_name(checksum_ctx->type));
- /* Print out the plan for reconstructing this file. */
- initStringInfo(&debug_buf);
- while (current_block < block_length)
- {
- rfile *s = sourcemap[current_block];
-
- /* Extend range, if possible. */
- if (current_block + 1 < block_length &&
- s == sourcemap[current_block + 1])
- {
- ++current_block;
- continue;
- }
-
- /* Add details about this range. */
- if (s == NULL)
- {
- if (current_block == start_of_range)
- appendStringInfo(&debug_buf, " %u:zero", current_block);
- else
- appendStringInfo(&debug_buf, " %u-%u:zero",
- start_of_range, current_block);
- }
- else
- {
- if (current_block == start_of_range)
- appendStringInfo(&debug_buf, " %u:%s@" UINT64_FORMAT,
- current_block, s->filename,
- (uint64) offsetmap[current_block]);
- else
- appendStringInfo(&debug_buf, " %u-%u:%s@" UINT64_FORMAT,
- start_of_range, current_block,
- s->filename,
- (uint64) offsetmap[current_block]);
- }
-
- /* Begin new range. */
- start_of_range = ++current_block;
-
- /* If the output is very long or we are done, dump it now. */
- if (current_block == block_length || debug_buf.len > 1024)
- {
- pg_log_debug("reconstruction plan:%s", debug_buf.data);
- resetStringInfo(&debug_buf);
- }
- }
-
- /* Free memory. */
- pfree(debug_buf.data);
+ /* Detailed dump. */
+ debug_reconstruction_plan(block_length, sourcemap, offsetmap);
}
/* Open the output file, except in dry_run mode. */
@@ -657,7 +674,7 @@ write_reconstructed_file(char *input_filename,
if (dry_run)
continue;
- /* Read or zero-fill the block as appropriate. */
+ /* If there's no source for the block, zero fill it. */
if (s == NULL)
{
/*
@@ -673,57 +690,9 @@ write_reconstructed_file(char *input_filename,
continue;
}
- /* Copy the block using the appropriate copy method. */
- if (copy_method != COPY_METHOD_COPY_FILE_RANGE)
- {
- /*
- * Read the block from the correct source file, and then write it
- * out, possibly with a checksum update.
- */
- read_block(s, offsetmap[i], buffer);
- write_block(wfd, output_filename, buffer, checksum_ctx);
- }
- else /* use copy_file_range */
- {
-#if defined(HAVE_COPY_FILE_RANGE)
- /* copy_file_range modifies the offset, so use a local copy */
- off_t off = offsetmap[i];
- size_t nwritten = 0;
-
- /*
- * Retry until we've written all the bytes (the offset is updated
- * by copy_file_range, and so is the wfd file offset).
- */
- do
- {
- int wb;
-
- wb = copy_file_range(s->fd, &off, wfd, NULL, BLCKSZ - nwritten, 0);
-
- if (wb < 0)
- pg_fatal("error while copying file range from \"%s\" to \"%s\": %m",
- input_filename, output_filename);
-
- nwritten += wb;
-
- } while (BLCKSZ > nwritten);
-
- /*
- * When checksum calculation not needed, we're done, otherwise
- * read the block and pass it to the checksum calculation.
- */
- if (checksum_ctx->type == CHECKSUM_TYPE_NONE)
- continue;
-
- read_block(s, offsetmap[i], buffer);
-
- if (pg_checksum_update(checksum_ctx, buffer, BLCKSZ) < 0)
- pg_fatal("could not update checksum of file \"%s\"",
- output_filename);
-#else
- pg_fatal("copy_file_range not supported on this platform");
-#endif
- }
+ /* Copy the block. */
+ copy_block(wfd, output_filename, s, offsetmap[i], copy_method,
+ checksum_ctx);
}
/* Debugging output. */
@@ -740,6 +709,73 @@ write_reconstructed_file(char *input_filename,
pg_fatal("could not close file \"%s\": %m", output_filename);
}
+/*
+ * Copy one block from source to output.
+ *
+ * The block should be written to fd; in case of error, output_filename
+ * should be used in the error message. The block should be written from
+ * source at the given offset. copy_method specifies how the copy is
+ * to be performed, and checksum_ctx should be updated for the bytes written.
+ */
+static void
+copy_block(int fd, char *output_filename, rfile *source, off_t offset,
+ CopyMethod copy_method, pg_checksum_context *checksum_ctx)
+{
+ uint8 buffer[BLCKSZ];
+
+ /* Copy the block using the appropriate copy method. */
+ if (copy_method != COPY_METHOD_COPY_FILE_RANGE)
+ {
+ /*
+ * Read the block from the correct source file, and then write it out,
+ * possibly with a checksum update.
+ */
+ read_block(source, offset, buffer);
+ write_block(fd, output_filename, buffer, checksum_ctx);
+ }
+ else /* use copy_file_range */
+ {
+#if defined(HAVE_COPY_FILE_RANGE)
+ /* copy_file_range modifies the offset, so use a local copy */
+ off_t off = offset;
+ size_t nwritten = 0;
+
+ /*
+ * Retry until we've written all the bytes (the offset is updated by
+ * copy_file_range, and so is the fd file offset).
+ */
+ do
+ {
+ int wb;
+
+ wb = copy_file_range(source->fd, &off, fd, NULL, BLCKSZ - nwritten, 0);
+
+ if (wb < 0)
+ pg_fatal("error while copying file range from \"%s\" to \"%s\": %m",
+ input_filename, output_filename);
+
+ nwritten += wb;
+
+ } while (BLCKSZ > nwritten);
+
+ /*
+ * When checksum calculation not needed, we're done, otherwise read
+ * the block and pass it to the checksum calculation.
+ */
+ if (checksum_ctx->type == CHECKSUM_TYPE_NONE)
+ continue;
+
+ read_block(s, offset, buffer);
+
+ if (pg_checksum_update(checksum_ctx, buffer, BLCKSZ) < 0)
+ pg_fatal("could not update checksum of file \"%s\"",
+ output_filename);
+#else
+ pg_fatal("copy_file_range not supported on this platform");
+#endif
+ }
+}
+
/*
* Write the block into the file (using the file descriptor), and
* if needed update the checksum calculation.
--
2.39.3 (Apple Git-145)
v1-0002-pg_combinebackup-Add-a-incremental-option.patchapplication/octet-stream; name=v1-0002-pg_combinebackup-Add-a-incremental-option.patchDownload
From 29d2d2d9d4e8fd11c709aad9876370d8dfea3af5 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Mon, 4 Nov 2024 12:30:41 -0500
Subject: [PATCH v1 2/2] pg_combinebackup: Add a --incremental option.
Previously, pg_combinebackup could be run only with one full backup
and then any number of incremental backups which together formed a
valid backup chain; the output was always a synthetic backup. That
remains the default behavior, but you can now use --incremental to
consolidate a series of incremental backups, each one based on the
previous one, into a single incremental backup that incorporates the
all the changes.
TODO: Needs TAP tests.
---
doc/src/sgml/ref/pg_combinebackup.sgml | 45 +++--
src/bin/pg_combinebackup/backup_label.c | 32 +++
src/bin/pg_combinebackup/backup_label.h | 2 +
src/bin/pg_combinebackup/pg_combinebackup.c | 106 ++++++++--
src/bin/pg_combinebackup/reconstruct.c | 204 ++++++++++++++++++--
src/bin/pg_combinebackup/reconstruct.h | 2 +
src/bin/pg_combinebackup/t/005_integrity.pl | 4 +-
7 files changed, 346 insertions(+), 49 deletions(-)
diff --git a/doc/src/sgml/ref/pg_combinebackup.sgml b/doc/src/sgml/ref/pg_combinebackup.sgml
index 091982f62ad..779409a2bf6 100644
--- a/doc/src/sgml/ref/pg_combinebackup.sgml
+++ b/doc/src/sgml/ref/pg_combinebackup.sgml
@@ -16,7 +16,7 @@ PostgreSQL documentation
<refnamediv>
<refname>pg_combinebackup</refname>
- <refpurpose>reconstruct a full backup from an incremental backup and dependent backups</refpurpose>
+ <refpurpose>combine incremental backups to form a new full or incremental backup</refpurpose>
</refnamediv>
<refsynopsisdiv>
@@ -30,24 +30,28 @@ PostgreSQL documentation
<refsect1>
<title>Description</title>
<para>
- <application>pg_combinebackup</application> is used to reconstruct a
- synthetic full backup from an
- <link linkend="backup-incremental-backup">incremental backup</link> and the
- earlier backups upon which it depends.
+ <application>pg_combinebackup</application> is used to create a
+ synthetic full or incremnetal backup from an
+ <link linkend="backup-incremental-backup">incremental backup</link> and
+ some or all of the earlier backups upon which it depends.
</para>
<para>
- Specify all of the required backups on the command line from oldest to newest.
- That is, the first backup directory should be the path to the full backup, and
+ Specify all of the required backups on the command line from oldest to
+ newest. When <literal>--incremental</literal> is not used, the first
+ backup directory should be the path to the full backup, and
the last should be the path to the final incremental backup
- that you wish to restore. The reconstructed backup will be written to the
+ restore. The output will be a synthetic full backup. With
+ <literal>--incremental</literal>, all backups should be incremental backups,
+ ordered oldest to newest, and the output will be a synthetic incremental
+ backup. In either case, the reconstructed backup will be written to the
output directory specified by the <option>-o</option> option.
</para>
<para>
<application>pg_combinebackup</application> will attempt to verify
that the backups you specify form a legal backup chain from which a correct
- full backup can be reconstructed. However, it is not designed to help you
+ backup can be reconstructed. However, it is not designed to help you
keep track of which backups depend on which other backups. If you remove
one or more of the previous backups upon which your incremental
backup relies, you will not be able to restore it. Moreover,
@@ -58,11 +62,11 @@ PostgreSQL documentation
</para>
<para>
- Since the output of <application>pg_combinebackup</application> is a
- synthetic full backup, it can be used as an input to a future invocation of
- <application>pg_combinebackup</application>. The synthetic full backup would
- be specified on the command line in lieu of the chain of backups from which
- it was reconstructed.
+ Since the output of <application>pg_combinebackup</application> is itself
+ a backup, it can be used as an input to a future invocation of
+ <application>pg_combinebackup</application>. This new, synthetic backup
+ would be specified on the command line in lieu of the chain of backups from
+ which it was reconstructed.
</para>
</refsect1>
@@ -94,6 +98,19 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-i</option></term>
+ <term><option>--incremental</option></term>
+ <listitem>
+ <para>
+ Instead of creating a synthetic full backup from a full backup
+ plus subsequent incremental backups, this instructs
+ <command>pg_combinebackup</command> to create a synthetic incremental
+ backup from a chain of incremental backups.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-N</option></term>
<term><option>--no-sync</option></term>
diff --git a/src/bin/pg_combinebackup/backup_label.c b/src/bin/pg_combinebackup/backup_label.c
index e676249247d..fd32166a890 100644
--- a/src/bin/pg_combinebackup/backup_label.c
+++ b/src/bin/pg_combinebackup/backup_label.c
@@ -125,6 +125,8 @@ parse_backup_label(char *filename, StringInfo buf,
*/
void
write_backup_label(char *output_directory, StringInfo buf,
+ TimeLineID incremental_from_tli,
+ XLogRecPtr incremental_from_lsn,
pg_checksum_type checksum_type, manifest_writer *mwriter)
{
char output_filename[MAXPGPATH];
@@ -170,6 +172,36 @@ write_backup_label(char *output_directory, StringInfo buf,
buf->cursor = eo;
}
+ if (incremental_from_tli != 0)
+ {
+ StringInfoData ibuf;
+ ssize_t wb;
+
+ Assert(!XLogRecPtrIsInvalid(incremental_from_lsn));
+ initStringInfo(&ibuf);
+ appendStringInfo(&ibuf, "INCREMENTAL FROM LSN: %X/%X\n",
+ LSN_FORMAT_ARGS(incremental_from_lsn));
+ appendStringInfo(&ibuf, "INCREMENTAL FROM TLI: %u\n",
+ incremental_from_tli);
+ wb = write(output_fd, ibuf.data, ibuf.len);
+
+ if (wb != ibuf.len)
+ {
+ if (wb < 0)
+ pg_fatal("could not write file \"%s\": %m", output_filename);
+ else
+ pg_fatal("could not write file \"%s\": wrote %d of %d",
+ output_filename, (int) wb, ibuf.len);
+ }
+
+ if (pg_checksum_update(&checksum_ctx,
+ (uint8 *) ibuf.data, ibuf.len) < 0)
+ pg_fatal("could not update checksum of file \"%s\"",
+ output_filename);
+
+ pfree(ibuf.data);
+ }
+
if (close(output_fd) != 0)
pg_fatal("could not close file \"%s\": %m", output_filename);
diff --git a/src/bin/pg_combinebackup/backup_label.h b/src/bin/pg_combinebackup/backup_label.h
index e2fc8dd4ce1..aa122feaa47 100644
--- a/src/bin/pg_combinebackup/backup_label.h
+++ b/src/bin/pg_combinebackup/backup_label.h
@@ -24,6 +24,8 @@ extern void parse_backup_label(char *filename, StringInfo buf,
TimeLineID *previous_tli,
XLogRecPtr *previous_lsn);
extern void write_backup_label(char *output_directory, StringInfo buf,
+ TimeLineID incremental_from_tli,
+ XLogRecPtr incremental_from_lsn,
pg_checksum_type checksum_type,
struct manifest_writer *mwriter);
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 6183d317151..0510d86b82e 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -72,6 +72,7 @@ typedef struct cb_options
bool debug;
char *output;
bool dry_run;
+ bool incremental;
bool no_sync;
cb_tablespace_mapping *tsmappings;
pg_checksum_type manifest_checksums;
@@ -100,7 +101,10 @@ typedef struct cb_tablespace
static cb_cleanup_dir *cleanup_dir_list = NULL;
static void add_tablespace_mapping(cb_options *opt, char *arg);
-static StringInfo check_backup_label_files(int n_backups, char **backup_dirs);
+static StringInfo check_backup_label_files(int n_backups, char **backup_dirs,
+ bool incremental,
+ TimeLineID *incremental_from_tli,
+ XLogRecPtr *incremental_from_lsn);
static uint64 check_control_files(int n_backups, char **backup_dirs);
static void check_input_dir_permissions(char *dir);
static void cleanup_directories_atexit(void);
@@ -131,6 +135,7 @@ main(int argc, char *argv[])
{
static struct option long_options[] = {
{"debug", no_argument, NULL, 'd'},
+ {"incremental", no_argument, NULL, 'i'},
{"dry-run", no_argument, NULL, 'n'},
{"no-sync", no_argument, NULL, 'N'},
{"output", required_argument, NULL, 'o'},
@@ -160,6 +165,8 @@ main(int argc, char *argv[])
StringInfo last_backup_label;
manifest_data **manifests;
manifest_writer *mwriter;
+ TimeLineID incremental_from_tli;
+ XLogRecPtr incremental_from_lsn;
pg_logging_init(argv[0]);
progname = get_progname(argv[0]);
@@ -172,7 +179,7 @@ main(int argc, char *argv[])
opt.copy_method = COPY_METHOD_COPY;
/* process command-line options */
- while ((c = getopt_long(argc, argv, "dnNo:T:",
+ while ((c = getopt_long(argc, argv, "dinNo:T:",
long_options, &optindex)) != -1)
{
switch (c)
@@ -181,6 +188,9 @@ main(int argc, char *argv[])
opt.debug = true;
pg_logging_increase_verbosity();
break;
+ case 'i':
+ opt.incremental = true;
+ break;
case 'n':
opt.dry_run = true;
break;
@@ -272,8 +282,16 @@ main(int argc, char *argv[])
n_backups = argc - optind;
system_identifier = check_control_files(n_backups, argv + optind);
- /* Sanity-check backup_label files, and get the contents of the last one. */
- last_backup_label = check_backup_label_files(n_backups, argv + optind);
+ /*
+ * Check the contents of the backup label files and extract necessary
+ * details. The return value is the last backup label file. We also
+ * extract the INCREMENTAL FROM TLI and INCREMENTAL FROM LSN fields from
+ * the fist backup label file, which need to be included in the output
+ * backup_label if we're consolidating incrementals.
+ */
+ last_backup_label =
+ check_backup_label_files(n_backups, argv + optind, opt.incremental,
+ &incremental_from_tli, &incremental_from_lsn);
/*
* We'll need the pathnames to the prior backups. By "prior" we mean all
@@ -350,6 +368,7 @@ main(int argc, char *argv[])
pg_log_debug("generating \"%s/backup_label\"", opt.output);
last_backup_label->cursor = 0;
write_backup_label(opt.output, last_backup_label,
+ incremental_from_tli, incremental_from_lsn,
opt.manifest_checksums, mwriter);
}
@@ -494,11 +513,18 @@ add_tablespace_mapping(cb_options *opt, char *arg)
}
/*
- * Check that the backup_label files form a coherent backup chain, and return
- * the contents of the backup_label file from the latest backup.
+ * Check that the backup_label files form a coherent backup chain.
+ *
+ * Return the contents of the backup_label file from the latest backup.
+ * Sets *incremntal_from_tli and *incremental_from_lsn based on the earliest
+ * backup; these will be 0 and InvalidXLogRecPtr unless we're consolidating
+ * incremental backups.
*/
static StringInfo
-check_backup_label_files(int n_backups, char **backup_dirs)
+check_backup_label_files(int n_backups, char **backup_dirs,
+ bool incremental,
+ TimeLineID *incremental_from_tli,
+ XLogRecPtr *incremental_from_lsn)
{
StringInfo buf = makeStringInfo();
StringInfo lastbuf = buf;
@@ -541,6 +567,13 @@ check_backup_label_files(int n_backups, char **backup_dirs)
parse_backup_label(pathbuf, buf, &start_tli, &start_lsn,
&previous_tli, &previous_lsn);
+ /* Save values from first backup. */
+ if (i == 0)
+ {
+ *incremental_from_tli = previous_tli;
+ *incremental_from_lsn = previous_lsn;
+ }
+
/*
* Sanity checks.
*
@@ -550,11 +583,24 @@ check_backup_label_files(int n_backups, char **backup_dirs)
* we don't have that information.
*/
if (i > 0 && previous_tli == 0)
- pg_fatal("backup at \"%s\" is a full backup, but only the first backup should be a full backup",
- backup_dirs[i]);
- if (i == 0 && previous_tli != 0)
- pg_fatal("backup at \"%s\" is an incremental backup, but the first backup should be a full backup",
- backup_dirs[i]);
+ {
+ pg_log_error("backup at \"%s\" is a full backup", backup_dirs[i]);
+ pg_log_error_detail("Only the first backup may be a full backup.");
+ exit(1);
+ }
+ if (i == 0 && !incremental && previous_tli != 0)
+ {
+ pg_log_error("backup at \"%s\" is an incremental backup",
+ backup_dirs[i]);
+ pg_log_error_hint("Use --incremental to combine incremental backups without a full backup.");
+ exit(1);
+ }
+ if (i == 0 && incremental && previous_tli == 0)
+ {
+ pg_fatal("backup at \"%s\" is a full backup", backup_dirs[i]);
+ pg_log_error_hint("Do not use --incremental when combining with a full backup.");
+ exit(1);
+ }
if (i < n_backups - 1 && start_tli != check_tli)
pg_fatal("backup at \"%s\" starts on timeline %u, but expected %u",
backup_dirs[i], start_tli, check_tli);
@@ -761,6 +807,7 @@ help(const char *progname)
printf(_(" %s [OPTION]... DIRECTORY...\n"), progname);
printf(_("\nOptions:\n"));
printf(_(" -d, --debug generate lots of debugging output\n"));
+ printf(_(" -i, --incremental combine incrementals without a full backup\n"));
printf(_(" -n, --dry-run do not actually do anything\n"));
printf(_(" -N, --no-sync do not wait for changes to be written safely to disk\n"));
printf(_(" -o, --output=DIRECTORY output directory\n"));
@@ -936,6 +983,8 @@ process_directory_recursively(Oid tsoid,
PGFileType type;
char ifullpath[MAXPGPATH];
char ofullpath[MAXPGPATH];
+ char ofullpath_incremental[MAXPGPATH];
+ char *ofullpath_actual = ofullpath;
char manifest_path[MAXPGPATH];
Oid oid = InvalidOid;
int checksum_length = 0;
@@ -1015,10 +1064,18 @@ process_directory_recursively(Oid tsoid,
strncmp(de->d_name, INCREMENTAL_PREFIX,
INCREMENTAL_PREFIX_LENGTH) == 0)
{
- /* Output path should not include "INCREMENTAL." prefix. */
+ bool incremental_result;
+
+ /*
+ * The normal output path should not include "INCREMENTAL."
+ * prefix, but if we're combining incremental backups into a
+ * consolidated incremental backup, then it's possible that we
+ * might write an incremental file rather than a full file.
+ */
snprintf(ofullpath, MAXPGPATH, "%s/%s", ofulldir,
de->d_name + INCREMENTAL_PREFIX_LENGTH);
-
+ snprintf(ofullpath_incremental, MAXPGPATH, "%s/%s", ofulldir,
+ de->d_name);
/* Manifest path likewise omits incremental prefix. */
snprintf(manifest_path, MAXPGPATH, "%s%s", manifest_prefix,
@@ -1026,6 +1083,7 @@ process_directory_recursively(Oid tsoid,
/* Reconstruction logic will do the rest. */
reconstruct_from_incremental_file(ifullpath, ofullpath,
+ opt->incremental ? ofullpath_incremental : NULL,
manifest_prefix,
de->d_name + INCREMENTAL_PREFIX_LENGTH,
n_prior_backups,
@@ -1035,9 +1093,25 @@ process_directory_recursively(Oid tsoid,
checksum_type,
&checksum_length,
&checksum_payload,
+ &incremental_result,
opt->copy_method,
opt->debug,
opt->dry_run);
+
+ /*
+ * It's possible that we're combining incrementals without a full
+ * backup; and if so, it's possible that a file backed up
+ * incrementally has no full version in any previous backup. In
+ * that case, we need to update ofullpath_actual and manifest_path
+ * to include the "INCREMENTAL." prefix.
+ */
+ if (incremental_result)
+ {
+ Assert(opt->incremental);
+ ofullpath_actual = ofullpath_incremental;
+ snprintf(manifest_path, MAXPGPATH, "%s%s", manifest_prefix,
+ de->d_name);
+ }
}
else
{
@@ -1127,8 +1201,8 @@ process_directory_recursively(Oid tsoid,
* trickier. Since we have to stat() anyway to get the mtime,
* there's no point in worrying about it.
*/
- if (stat(ofullpath, &sb) < 0)
- pg_fatal("could not stat file \"%s\": %m", ofullpath);
+ if (stat(ofullpath_actual, &sb) < 0)
+ pg_fatal("could not stat file \"%s\": %m", ofullpath_actual);
/* OK, now do the work. */
add_file_to_manifest(mwriter, manifest_path,
diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c
index 17375878e8e..948d6337047 100644
--- a/src/bin/pg_combinebackup/reconstruct.c
+++ b/src/bin/pg_combinebackup/reconstruct.c
@@ -51,10 +51,21 @@ static void debug_reconstruction(int n_source,
bool dry_run);
static void debug_reconstruction_plan(unsigned block_length,
rfile **sourcemap,
- off_t *offsetmap);
+ off_t *offsetmap,
+ bool consolidate);
static unsigned find_reconstructed_block_length(rfile *s);
static rfile *make_incremental_rfile(char *filename);
static rfile *make_rfile(char *filename, bool missing_ok);
+static void write_consolidated_file(char *input_filename,
+ char *output_filename,
+ unsigned block_length,
+ rfile **sourcemap,
+ off_t *offsetmap,
+ unsigned truncation_block_length,
+ pg_checksum_context *checksum_ctx,
+ CopyMethod copy_method,
+ bool debug,
+ bool dry_run);
static void write_reconstructed_file(char *input_filename,
char *output_filename,
unsigned block_length,
@@ -93,6 +104,7 @@ static void read_block(rfile *s, off_t off, uint8 *buffer);
void
reconstruct_from_incremental_file(char *input_filename,
char *output_filename,
+ char *output_filename_incremental,
char *relative_path,
char *bare_file_name,
int n_prior_backups,
@@ -102,6 +114,7 @@ reconstruct_from_incremental_file(char *input_filename,
pg_checksum_type checksum_type,
int *checksum_length,
uint8 **checksum_payload,
+ bool *incremental_result,
CopyMethod copy_method,
bool debug,
bool dry_run)
@@ -338,12 +351,22 @@ reconstruct_from_incremental_file(char *input_filename,
* Otherwise, reconstruct.
*/
if (copy_source != NULL)
+ {
copy_file(copy_source->filename, output_filename,
&checksum_ctx, copy_method, dry_run);
+ *incremental_result = false;
+ }
else if (sidx == 0 && source[0]->header_length != 0)
{
- pg_fatal("full backup contains unexpected incremental file \"%s\"",
- source[0]->filename);
+ if (output_filename_incremental == NULL)
+ pg_fatal("full backup contains unexpected incremental file \"%s\"",
+ source[0]->filename);
+ write_consolidated_file(input_filename, output_filename_incremental,
+ block_length, sourcemap, offsetmap,
+ latest_source->truncation_block_length,
+ &checksum_ctx, copy_method, debug, dry_run);
+ debug_reconstruction(n_prior_backups + 1, source, dry_run);
+ *incremental_result = true;
}
else
{
@@ -352,6 +375,7 @@ reconstruct_from_incremental_file(char *input_filename,
&checksum_ctx, copy_method,
debug, dry_run);
debug_reconstruction(n_prior_backups + 1, source, dry_run);
+ *incremental_result = false;
}
/* Save results of checksum calculation. */
@@ -439,7 +463,7 @@ debug_reconstruction(int n_source, rfile **sources, bool dry_run)
*/
static void
debug_reconstruction_plan(unsigned block_length, rfile **sourcemap,
- off_t *offsetmap)
+ off_t *offsetmap, bool consolidate)
{
StringInfoData debug_buf;
unsigned start_of_range = 0;
@@ -459,16 +483,16 @@ debug_reconstruction_plan(unsigned block_length, rfile **sourcemap,
continue;
}
- /* Add details about this range. */
- if (s == NULL)
- {
- if (current_block == start_of_range)
- appendStringInfo(&debug_buf, " %u:zero", current_block);
- else
- appendStringInfo(&debug_buf, " %u-%u:zero",
- start_of_range, current_block);
- }
- else
+ /*
+ * Add details about this range, if appropriate.
+ *
+ * When consolidate = false, we're reconstructing a full file and any
+ * unsourced blocks must be zeroed. When consolidate = true, we're
+ * consolidating incremental files into a new incremental file, and
+ * unsourced blocks are not included in the output. We therefore don't
+ * include them in the debugging output, either.
+ */
+ if (s != NULL)
{
if (current_block == start_of_range)
appendStringInfo(&debug_buf, " %u:%s@" UINT64_FORMAT,
@@ -480,6 +504,14 @@ debug_reconstruction_plan(unsigned block_length, rfile **sourcemap,
s->filename,
(uint64) offsetmap[current_block]);
}
+ else if (!consolidate)
+ {
+ if (current_block == start_of_range)
+ appendStringInfo(&debug_buf, " %u:zero", current_block);
+ else
+ appendStringInfo(&debug_buf, " %u-%u:zero",
+ start_of_range, current_block);
+ }
/* Begin new range. */
start_of_range = ++current_block;
@@ -487,7 +519,12 @@ debug_reconstruction_plan(unsigned block_length, rfile **sourcemap,
/* If the output is very long or we are done, dump it now. */
if (current_block == block_length || debug_buf.len > 1024)
{
- pg_log_debug("reconstruction plan:%s", debug_buf.data);
+ if (debug_buf.len == 0)
+ appendStringInfoString(&debug_buf, " empty");
+ if (consolidate)
+ pg_log_debug("consolidation plan:%s", debug_buf.data);
+ else
+ pg_log_debug("reconstruction plan:%s", debug_buf.data);
resetStringInfo(&debug_buf);
}
}
@@ -612,6 +649,138 @@ read_bytes(rfile *rf, void *buffer, unsigned length)
}
}
+/*
+ * Write out a consolidated incremental file.
+ */
+static void
+write_consolidated_file(char *input_filename,
+ char *output_filename,
+ unsigned block_length,
+ rfile **sourcemap,
+ off_t *offsetmap,
+ unsigned truncation_block_length,
+ pg_checksum_context *checksum_ctx,
+ CopyMethod copy_method,
+ bool debug,
+ bool dry_run)
+{
+ unsigned magic = INCREMENTAL_MAGIC;
+ int wfd = -1;
+ BlockNumber i;
+ unsigned sourced_blocks = 0;
+ StringInfoData buf;
+
+ /* Count the number of blocks for which we have a source. */
+ for (i = 0; i < block_length; ++i)
+ if (sourcemap[i] != NULL)
+ ++sourced_blocks;
+
+ /* Debugging output. */
+ if (debug)
+ {
+ /* Basic information about the output file to be produced. */
+ if (dry_run)
+ pg_log_debug("would write consolidated file \"%s\" (%u blocks, checksum %s)",
+ output_filename, sourced_blocks,
+ pg_checksum_type_name(checksum_ctx->type));
+ else
+ pg_log_debug("writing consolidated file \"%s\" (%u blocks, checksum %s)",
+ output_filename, sourced_blocks,
+ pg_checksum_type_name(checksum_ctx->type));
+
+ /* Detailed dump. */
+ debug_reconstruction_plan(block_length, sourcemap, offsetmap, true);
+ }
+
+ /* Except in dry-run mode, open output file and write header. */
+ if (!dry_run)
+ {
+ int wb;
+
+ /* Open the output file. */
+ if ((wfd = open(output_filename, O_RDWR | PG_BINARY | O_CREAT | O_EXCL,
+ pg_file_create_mode)) < 0)
+ pg_fatal("could not open file \"%s\": %m", output_filename);
+
+ /* Construct incremental file header. */
+ initStringInfo(&buf);
+ appendBinaryStringInfo(&buf, &magic, sizeof(magic));
+ appendBinaryStringInfo(&buf, &sourced_blocks, sizeof(sourced_blocks));
+ appendBinaryStringInfo(&buf, &truncation_block_length,
+ sizeof(truncation_block_length));
+ for (i = 0; i < block_length; ++i)
+ if (sourcemap[i] != NULL)
+ appendBinaryStringInfo(&buf, &i, sizeof(i));
+
+ /*
+ * Add padding to align header to a multiple of BLCKSZ, but only if
+ * the incremental file has some blocks, and the alignment is actually
+ * needed (i.e. header is not already a multiple of BLCKSZ). If there
+ * are no blocks we don't want to make the file unnecessarily large,
+ * as that might make some filesystem optimizations impossible.
+ */
+ if (sourced_blocks > 0 && (buf.len % BLCKSZ) != 0)
+ {
+ unsigned paddinglen = BLCKSZ - (buf.len % BLCKSZ);
+
+ /*
+ * This logic is similar to appendStringInfoSpaces. Note that we
+ * adda terminating \0 even though we're adding zeroes. That's not
+ * strictly necessary here but might as well be consistent with
+ * usual practice.
+ */
+ enlargeStringInfo(&buf, paddinglen);
+ memset(buf.data + buf.len, '\0', paddinglen + 1);
+ buf.len += paddinglen;
+ }
+
+ /* Write out the file header. */
+ if ((wb = write(wfd, buf.data, buf.len)) != buf.len)
+ {
+ if (wb < 0)
+ pg_fatal("could not write file \"%s\": %m", output_filename);
+ else
+ pg_fatal("could not write file \"%s\": wrote %d of %d",
+ output_filename, wb, BLCKSZ);
+ }
+
+ /* Incorporate file header into checksum computation. */
+ if (pg_checksum_update(checksum_ctx, (uint8 *) buf.data, buf.len) < 0)
+ pg_fatal("could not update checksum of file \"%s\"",
+ output_filename);
+
+ /* Avoid leaking memory. */
+ pfree(buf.data);
+ }
+
+ /* Read and write the blocks as required. */
+ for (i = 0; i < block_length; ++i)
+ {
+ rfile *s = sourcemap[i];
+
+ /* Disregard unsourced blocks. */
+ if (s == NULL)
+ continue;
+
+ /* Update accounting information. */
+ s->num_blocks_read++;
+ s->highest_offset_read = Max(s->highest_offset_read,
+ offsetmap[i] + BLCKSZ);
+
+ /* Skip the rest of this in dry-run mode. */
+ if (dry_run)
+ continue;
+
+ /* Copy the block. */
+ copy_block(wfd, output_filename, s, offsetmap[i], copy_method,
+ checksum_ctx);
+ }
+
+ /* Close the output file. */
+ if (wfd >= 0 && close(wfd) != 0)
+ pg_fatal("could not close file \"%s\": %m", output_filename);
+}
+
/*
* Write out a reconstructed file.
*/
@@ -644,7 +813,7 @@ write_reconstructed_file(char *input_filename,
pg_checksum_type_name(checksum_ctx->type));
/* Detailed dump. */
- debug_reconstruction_plan(block_length, sourcemap, offsetmap);
+ debug_reconstruction_plan(block_length, sourcemap, offsetmap, false);
}
/* Open the output file, except in dry_run mode. */
@@ -657,7 +826,6 @@ write_reconstructed_file(char *input_filename,
/* Read and write the blocks as required. */
for (i = 0; i < block_length; ++i)
{
- uint8 buffer[BLCKSZ];
rfile *s = sourcemap[i];
/* Update accounting information. */
@@ -677,6 +845,8 @@ write_reconstructed_file(char *input_filename,
/* If there's no source for the block, zero fill it. */
if (s == NULL)
{
+ uint8 buffer[BLCKSZ];
+
/*
* New block not mentioned in the WAL summary. Should have been an
* uninitialized block, so just zero-fill it.
diff --git a/src/bin/pg_combinebackup/reconstruct.h b/src/bin/pg_combinebackup/reconstruct.h
index c878febbb38..a02ace2a655 100644
--- a/src/bin/pg_combinebackup/reconstruct.h
+++ b/src/bin/pg_combinebackup/reconstruct.h
@@ -19,6 +19,7 @@
extern void reconstruct_from_incremental_file(char *input_filename,
char *output_filename,
+ char *output_filename_incremental,
char *relative_path,
char *bare_file_name,
int n_prior_backups,
@@ -28,6 +29,7 @@ extern void reconstruct_from_incremental_file(char *input_filename,
pg_checksum_type checksum_type,
int *checksum_length,
uint8 **checksum_payload,
+ bool *incremental_result,
CopyMethod copy_method,
bool debug,
bool dry_run);
diff --git a/src/bin/pg_combinebackup/t/005_integrity.pl b/src/bin/pg_combinebackup/t/005_integrity.pl
index 25ebb8c0406..db0a466a30d 100644
--- a/src/bin/pg_combinebackup/t/005_integrity.pl
+++ b/src/bin/pg_combinebackup/t/005_integrity.pl
@@ -88,7 +88,7 @@ $node1->command_fails_like(
'pg_combinebackup', $backup1path, $backup1path, '-o',
$resultpath, $mode
],
- qr/is a full backup, but only the first backup should be a full backup/,
+ qr/is a full backup.*Only the first backup may be a full backup/s,
"can't combine full backups");
# Can't combine 2 incremental backups.
@@ -97,7 +97,7 @@ $node1->command_fails_like(
'pg_combinebackup', $backup2path, $backup2path, '-o',
$resultpath, $mode
],
- qr/is an incremental backup, but the first backup should be a full backup/,
+ qr/is an incremental backup.*Use --incremental to combine/s,
"can't combine full backups");
# Can't combine full backup with an incremental backup from a different system.
--
2.39.3 (Apple Git-145)
Hi,
On Mon, Nov 04, 2024 at 12:53:05PM -0500, Robert Haas wrote:
Hi,
When I gave my talk on pg_basebackup at pgconf.eu, people took the
opportunity to mention various improvements which they would find
useful. One of those improvements was the ability to combine several
incremental backups into one. This might be useful if you take very
frequent incremental backups but want to roll up older ones to reduce
storage requirements.
+1, I think that could be useful too.
For example, you could take an incremental
backup every hour, but then once more than a day has gone by, you
might roll up those up six at a time into bigger incremental backups
so that you store 4 backups per day instead of 24 backups per day.The attached patch set lets you do this.
Thanks for the patch!
The idea is that instead of doing this:
$ pg_combinebackup full incr1 incr2 incr3 incr4 incr5 -o result
You could instead do this:
$ pg_combinebackup -i incr1 incr2 incr3 -o incr1-3
$ rm -rf incr{1,2,3}
<time passes>
$ pg_combinebackup full incr1-3 incr5 -o result
Did not test but I guess that one benefit is that the last pg_combinebackup
could be faster than the one that would be needed without the new feature in
place?
If you're interested in this feature, please give this a try and let
me know what you find out!
It looks like that it currently does not compile on master, I get things like:
"
reconstruct.c: In function ‘copy_block’:
reconstruct.c:755:50: error: ‘input_filename’ undeclared (first use in this function); did you mean ‘output_filename’?
755 | input_filename, output_filename);
| ^~~~~~~~~~~~~~
../../../src/include/common/logging.h:152:62: note: in definition of macro ‘pg_fatal’
152 | pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
| ^~~~~~~~~~~
reconstruct.c:755:50: note: each undeclared identifier is reported only once for each function it appears in
755 | input_filename, output_filename);
| ^~~~~~~~~~~~~~
../../../src/include/common/logging.h:152:62: note: in definition of macro ‘pg_fatal’
152 | pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
| ^~~~~~~~~~~
reconstruct.c:766:25: error: continue statement not within a loop
766 | continue;
| ^~~~~~~~
reconstruct.c:768:28: error: ‘s’ undeclared (first use in this function)
768 | read_block(s, offset, buffer);
| ^
make[3]: *** [../../../src/Makefile.global:961: reconstruct.o] Error 1
"
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Mon, Nov 4, 2024 at 6:53 PM Robert Haas <robertmhaas@gmail.com> wrote:
Hi Robert,
[..]
1. Well, I have also the same bug as Bertrand which seems to be because
MacOS was used development rather than Linux (and thus MacOS doesnt have
copy_file_range(2)/HAVE_COPY_FILE_RANGE) --> I've simply fallen back to
undefHAVE_COPY_FILE_RANGE in my case, but patch needs to be fixed. I
haven't run any longer or more data-intensive tests as the
copy_file_range() seems to be missing and from my point of view that thing
is crucial.
2. While interleaving several incremental backups with pgbench, I've
noticed something strange by accident:
This will work:
$ pg_combinebackup -o /var/lib/postgresql/18/main fulldbbackup incr1 incr2
incr3 incr4
This will also work (new):
$ pg_combinebackup -i incr1 incr2 incr3 incr4 -o good_incr1_4
$ rm -rf /var/lib/postgresql/18/main && pg_combinebackup -o
/var/lib/postgresql/18/main fulldbbackup good_incr1_4
This will also work (new):
$ pg_combinebackup -i incr1 incr2 -o incr_12 #ok
$ pg_combinebackup -i incr_12 incr3 -o incr_13 #ok
$ rm -rf /var/lib/postgresql/18/main && pg_combinebackup -o
/var/lib/postgresql/18/main fulldbbackup incr_13
BUT, if I make intentional user error and if I merge the same incr2 into
both into two sets of incremental backups it won't reconstruct that:
$ pg_combinebackup -i incr1 incr2 -o incr1_2 # contains 1 + 2
$ pg_combinebackup -i incr2 incr3 -o incr2_3 # contains 2(!) + 3
$ rm -rf /var/lib/postgresql/18/main && pg_combinebackup -o
/var/lib/postgresql/18/main fulldbbackup incr1_2 # ofc works
$ rm -rf /var/lib/postgresql/18/main && pg_combinebackup -o
/var/lib/postgresql/18/main fulldbbackup incr1_2 incr2_3 # fails?
pg_combinebackup: error: backup at "incr1_2" starts at LSN 0/B000028, but
expected 0/9000028
It seems to be taking LSN from incr1_2 and ignoring incr2_3 ?
$ find incr1 incr2 incr3 incr1_2 incr2_3 fulldbbackup -name backup_manifest
-exec grep -H LSN {} \;
incr1/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/9000028", "End-LSN":
"0/9000120" }
incr2/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/B000028", "End-LSN":
"0/B000120" }
incr3/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/D000028", "End-LSN":
"0/D000120" }
incr1_2/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/B000028",
"End-LSN": "0/B000120" }
incr2_3/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/D000028",
"End-LSN": "0/D000120" }
fulldbbackup/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/70000D8",
"End-LSN": "0/70001D0" }
So not sure should we cover that scenario or not ?
$ rm -rf /var/lib/postgresql/18/main && pg_combinebackup -o
/var/lib/postgresql/18/main fulldbbackup incr1_2 incr3 # works
$ rm -rf /var/lib/postgresql/18/main && pg_combinebackup -o
/var/lib/postgresql/18/main fulldbbackup incr1_2 incr3_4 # works
$ rm -rf /var/lib/postgresql/18/main && pg_combinebackup -o
/var/lib/postgresql/18/main fulldbbackup incr1_2 incr3_4 # two combined
sets - also work
4. Space saving feature seems to be there (I've tried to merge up to ~40
backups with rolling merging incr backup always after each incremental
backup), which seems to be the primary objective of the patch:
$ du -sm incr? incr?? incr1_38
38 incr1
25 incr2
[..]
24 incr37
24 incr38
[..above would be ~38*~25M = ~950MB]
87 incr1_38 # instead of ~950MB just 87MB
5. I've run accidently into independent problem when using
"pg_combinebackup -i `ls -1vd incr*` -o incr_ALL" (when dealing with dozens
of incrementals that are merged, I bet this is going to be pretty used
pattern), that pg_combinebackup was failing with
$ pg_combinebackup -i `ls -1vd incr*` -o incr_ALL
pg_combinebackup: error: incr26/global/pg_control: expected system
identifier 7436752340350991865, but found 7436753510269539237
The issue for me is that the check if the output directory should not exist
first, because it is taking incr_ALL here into ls(1) first while looking
for System-Identifiers and blowing up with error , before checking if that
-o dir doesn't exit:
$ grep System-Id ./incr_ALL/backup_manifest
"System-Identifier": 7436752340350991865,
So the issue is sequencing: first it should check if the incr_ALL does not
exist and only maybe later start inspecting backups to be combined?
6. Not sure, if that's critical, but it seems to require incremental
backups in order to be merging correctly , is that a limitation by design
or not ? (note --reverse in ls(1)):
$ rm -rf incr_ALL && pg_combinebackup -i `ls -1vd incr*` -o incr_ALL
$ rm -rf incr_ALL && pg_combinebackup -i `ls -1rvd incr*` -o incr_ALL
pg_combinebackup: error: backup at "incr2" starts at LSN 0/B000028, but
expected 0/70000D8
simpler:
$ rm -rf incr_ALL && pg_combinebackup -i incr1 incr2 incr3 -o incr_ALL
$ rm -rf incr_ALL && pg_combinebackup -i incr3 incr2 incr1 -o incr_ALL
pg_combinebackup: error: backup at "incr2" starts at LSN 0/B000028, but
expected 0/70000D8
$ find incr1 incr2 incr3 -name backup_manifest -exec grep -H LSN {} \; |
sort -nk 1
incr1/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/9000028", "End-LSN":
"0/9000120" }
incr2/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/B000028", "End-LSN":
"0/B000120" }
incr3/backup_manifest:{ "Timeline": 1, "Start-LSN": "0/D000028", "End-LSN":
"0/D000120" }
Nitpicking and other possibly not important things:
a. I'm still a fan of `--merge-incremental[-backups]` over `--incremental`
switch in pg_combinebackup and disabling the short `-i` switch :^)
b. pg_combinebackup help message has:
-i, --incremental combine incrementals without a full backup
Maybe s/combine incrementals/merge incrementals backups/ as the
"incrementals" misses the "incremental of <what>"
c. If we are at copy_file_blocks(), couldn't we here simply report also
strerror(errno) in one of the parameters to pg_fatal during short write ? I
bet ENOSPC error message would be less vague:
pg_combinebackup: error: could not write to file "incr1_39/base/5/2613",
offset 9011200: wrote 327680 of 409600
pg_combinebackup: removing output directory "incr1_39"
-J.