On Tue, Oct 6, 2026 at 5:42 PM Heikki Linnakangas <[email protected]> wrote:
> I think that comment is wrong. A short read is possible for other > reasons than a truncated file, e.g. if the read is interrupted by a > signal. In the past, we've made that same assumption that you never get > a short read on a BLCKSZ-sized read in other places too, but it was > always questionable. Thomas Munro fixed it for smgrread() in commit > 4908c58720. > > I think we should now fix that in read_file_data_into_buffer(), too. > Make it retry, so that it never returns fewer bytes than requested > except for EOF. With a larger buffer, a short read becomes more likely. OK, I see, thanks, so I've tried this in the attached 0004. 0001..0004 without asserts, still give ~1.67x on hot DB fully in VFS cache: pg_basebackup -c fast -v -h 127.0.0.1 --target=client-blackhole -Xnone \ -c fast --no-sync --no-verify-checksums --manifest-checksums=NONE \ 2>&1 | grep MB/s (client-blackhole still transfers to CLI, so it's receieving just ignoring write path at all) 256kb (as within 0004): pg_basebackup: base backup completed (avg 5503.2 MB/s) pg_basebackup: base backup completed (avg 5331.5 MB/s) pg_basebackup: base backup completed (avg 5205.6 MB/s) 32kB: pg_basebackup: base backup completed (avg 3110.2 MB/s) pg_basebackup: base backup completed (avg 3307.3 MB/s) pg_basebackup: base backup completed (avg 3191.2 MB/s) I've tried a couple of other integrity tests too for this new retry code: 1. high concurrency pgbench with loop of parallel pg_basebackup + pg_verifybackup for couple of minutes (to see if that backing up files being appended works OK). It went OK. 2. I've tried failure injection on 3rd invocation of pread of certain file: strace -ff -p $POSTMASTERPID -P $PGDATA/base/1/1255 \ -e trace=pread64 -k -e inject=pread64:error=EINTR:when=3 and in first version of my patch I've got: WARNING: aborting backup due to backend exiting before pg_backup_stop .. ERROR: could not read file "./base/1/1255": Interrupted system call strace got it exactly there: pread64(14, ..., 262144, 524288) = -1 EINTR (Interrupted system call) (INJECTED) > /usr/lib/x86_64-linux-gnu/libc.so.6(pread64+0x17) [0xfa577] > .../bin/postgres(basebackup_read_file+0x61) [0x23b9a1] > .../bin/postgres(read_file_data_into_buffer+0x4d) [0x23bcfd] so right, today even master seems to be vulnerable to this I think. Anyway, I've added to 0004 the EINTR guard (FileReadV() had goto for this) and I have retested this using the EINTR injection for way more pgdata files at specifc blocks and then checked if the resulting DB was fine: rm -rf /tmp/full1 # ~10GB DB pg_basebackup -c fast -v -h 127.0.0.1 -D /tmp/full1 -c fast # strace on some relations with "when=1..2+1" has reported INJECTED # messages (causing restart of the syscall in code) pg_verifybackup /tmp/full1 and got "backup successfully verified" 3. With SIMULATE_SHORT_READ it also works OK. -J.
From 50c46cac197626270279b3738bb458dac5f82f2a Mon Sep 17 00:00:00 2001 From: Jakub Wartak <[email protected]> Date: Wed, 7 Oct 2026 10:36:21 +0200 Subject: [PATCH v07102026 4/4] basebackup: bump SINK_BUFFER_LENGTH to 256kB By increasing SINK_BUFFER_LENGTH size to be closer to modern L2 CPU cache sizes, we are also reducing frequency of the pread() system call which also helps avoids performance penalties (and is visible as one of the top functions when sending backups). In isolated performance runs where pg_basebackup client did not have write it's backup to disk, this gives ~180% of the baseline (3.1GB/s -> 5.5GB/s). Along the way try to improve the code for handling short reads in basebackup's basebackup_read_file() as higher buffer size could translate to higher risk of pread() interruptions. Author: Jakub Wartak <[email protected]> Reviewed-by: Heikki Linnakangas <[email protected]> Discussion: https://postgr.es/m/CAKZiRmwwW-hDc3B6ERJB%2BpaX7RNSBcQLheq1KdsTf42cGuRvuA%40mail.gmail.com --- src/backend/backup/basebackup.c | 69 ++++++++++++++++-------- src/backend/backup/basebackup_progress.c | 2 +- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index e3c04ecd810..9103026f676 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -59,7 +59,7 @@ * NB: The buffer size is required to be a multiple of the system block * size, so use that value instead if it's bigger than our preference. */ -#define SINK_BUFFER_LENGTH Max(32768, BLCKSZ) +#define SINK_BUFFER_LENGTH Max(256 * 1024, BLCKSZ) typedef struct { @@ -1738,11 +1738,12 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, &checksum_failures); /* - * If we get a partial read, that must mean that the relation is - * being truncated. Ultimately, it should be truncated to a - * multiple of BLCKSZ, since this path should only be reached for - * relation files, but we might transiently observe an - * intermediate value. + * read_file_data_into_buffer() retries short reads, so if we got + * fewer bytes than requested we must have hit EOF (which means + * that the relation is being truncated). Ultimately, it should be + * truncated to a multiple of BLCKSZ, since this path should only + * be reached for relation files, but we might transiently observe + * an intermediate value. * * It should be fine to treat this just as if the entire block had * been truncated away - i.e. fill this and all later blocks with @@ -1847,8 +1848,9 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, * 'offset' is the file offset from which we should begin to read, and * 'length' is the amount of data that should be read. The actual amount * of data read will be less than the requested amount if the bbsink's - * buffer isn't big enough to hold it all, or if the underlying file has - * been truncated. The return value is the number of bytes actually read. + * buffer isn't big enough to hold it all, or if we hit EOF, which means + * that the underlying file has been truncated. The return value is the + * number of bytes actually read. * * 'blkno' is the block number of the first page in the bbsink's buffer * relative to the start of the relation. @@ -2176,7 +2178,8 @@ convert_link_to_directory(const char *pathbuf, struct stat *statbuf) /* * Read some data from a file, setting a wait event and reporting any error - * encountered. + * encountered. Short reads are retried until either all requested bytes have + * been read or EOF is hit ( potentially due to concurrent truncation). * * If partial_read_ok is false, also report an error if the number of bytes * read is not equal to the number of bytes requested. @@ -2187,21 +2190,45 @@ static ssize_t basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset, const char *filename, bool partial_read_ok) { - ssize_t rc; + ssize_t transferred = 0; - pgstat_report_wait_start(WAIT_EVENT_BASEBACKUP_READ); - rc = pg_pread(fd, buf, nbytes, offset); - pgstat_report_wait_end(); + /* + * Loop to continue after a short read. Short reads are more likely to + * show up with large read requests such as ours due to e.g. signals. This + * tries to mirror what mdreadv() does. + */ + while (transferred < nbytes) + { + ssize_t rc; - if (rc < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not read file \"%s\": %m", filename))); - if (!partial_read_ok && rc > 0 && rc != nbytes) + pgstat_report_wait_start(WAIT_EVENT_BASEBACKUP_READ); + rc = pg_pread(fd, buf + transferred, nbytes - transferred, + offset + transferred); + pgstat_report_wait_end(); + +#ifdef SIMULATE_SHORT_READ + rc = Min(rc, 4096); +#endif + if (rc < 0 && errno == EINTR) + continue; + + if (rc < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not read file \"%s\": %m", filename))); + + /* EOF */ + if (rc == 0) + break; + + transferred += rc; + } + + if (!partial_read_ok && transferred > 0 && transferred != nbytes) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not read file \"%s\": read %zd of %zu", - filename, rc, nbytes))); + errmsg("could not read file \"%s\": read %zu of %zu", + filename, transferred, nbytes))); - return rc; + return transferred; } diff --git a/src/backend/backup/basebackup_progress.c b/src/backend/backup/basebackup_progress.c index f74459181d0..1d8b24dfdc9 100644 --- a/src/backend/backup/basebackup_progress.c +++ b/src/backend/backup/basebackup_progress.c @@ -63,7 +63,7 @@ bbsink_progress_new(bbsink *next, bool estimate_backup_size, bool incremental) Assert(next != NULL); - sink = palloc0_object(bbsink); + sink = palloc_aligned(sizeof(bbsink), PG_IO_ALIGN_SIZE, MCXT_ALLOC_ZERO); *((const bbsink_ops **) &sink->bbs_ops) = &bbsink_progress_ops; sink->bbs_next = next; -- 2.43.0
From 4529d51e1db5c23dc880ede70ed0b3eaf7e99fe2 Mon Sep 17 00:00:00 2001 From: Jakub Wartak <[email protected]> Date: Fri, 24 Jul 2026 08:30:20 +0200 Subject: [PATCH v07102026 3/4] pg_basebackup: report average data transfer throughput in verbose mode Add the average data transfer rate to the "base backup completed" message. The transfer is timed with instr_time from just before the archive data is received until all data has arrived (but before fsync is called). Author: Jakub Wartak <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/CAKZiRmwwW-hDc3B6ERJB%2BpaX7RNSBcQLheq1KdsTf42cGuRvuA%40mail.gmail.com --- src/bin/pg_basebackup/pg_basebackup.c | 31 ++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 4628900da10..b6e469d47bb 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -37,6 +37,7 @@ #include "fe_utils/recovery_gen.h" #include "getopt_long.h" #include "libpq/protocol.h" +#include "portability/instr_time.h" #include "receivelog.h" #include "streamutil.h" @@ -1790,6 +1791,8 @@ BaseBackup(char *compression_algorithm, char *compression_detail, int writing_to_stdout; bool use_new_option_syntax = false; PQExpBufferData buf; + instr_time transfer_start; + instr_time transfer_elapsed; Assert(conn != NULL); initPQExpBuffer(&buf); @@ -2145,6 +2148,9 @@ BaseBackup(char *compression_algorithm, char *compression_detail, wal_compress_level); } + /* Start timing the data transfer, for the average rate report. */ + INSTR_TIME_SET_CURRENT(transfer_start); + if (serverMajor >= 1500) { /* Receive a single tar stream with everything. */ @@ -2197,6 +2203,13 @@ BaseBackup(char *compression_algorithm, char *compression_detail, ReceiveBackupManifest(conn); } + /* + * All backup data has been received, so measure how long the transfer + * took for the average rate report shown at completion. + */ + INSTR_TIME_SET_CURRENT(transfer_elapsed); + INSTR_TIME_SUBTRACT(transfer_elapsed, transfer_start); + if (showprogress) { progress_update_filename(NULL); @@ -2365,7 +2378,22 @@ BaseBackup(char *compression_algorithm, char *compression_detail, } if (verbose) - pg_log_info("base backup completed"); + { + double elapsed_sec = INSTR_TIME_GET_DOUBLE(transfer_elapsed); + + /* + * Avoids potential division by zero. + * + * Timing does not include potential fsync()/syncfs(), so data might be + * still in-flight from pagecache when total_done was calculated, therefore + * we make it clear to the user what we are measuring. + */ + if (elapsed_sec > 0.0) + pg_log_info("base backup completed (avg %.1f MB/s)", + (double) totaldone / (1024 * 1024) / elapsed_sec); + else + pg_log_info("base backup completed"); + } } @@ -2422,6 +2450,7 @@ main(int argc, char **argv) pg_logging_init(argv[0]); progname = get_progname(argv[0]); set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup")); + pg_initialize_timing(); if (argc > 1) { -- 2.43.0
From 57d30a23c4aac7a54c3c46fed541fa96df8e1b48 Mon Sep 17 00:00:00 2001 From: Jakub Wartak <[email protected]> Date: Thu, 30 Jul 2026 11:02:13 +0200 Subject: [PATCH v07102026 1/4] pg_basebackup: rename the "blackhole" backup target to "server-blackhole" The built-in "blackhole" backup target discards the backup on the server side. Rename it to "server-blackhole" to make the location of the discard explicit and to leave room for a "client-blackhole" in the follow-up commit. Author: Jakub Wartak <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/CAKZiRmwwW-hDc3B6ERJB%2BpaX7RNSBcQLheq1KdsTf42cGuRvuA%40mail.gmail.com --- doc/src/sgml/protocol.sgml | 2 +- doc/src/sgml/ref/pg_basebackup.sgml | 6 +++--- src/backend/backup/basebackup_target.c | 4 ++-- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 8ced69f283b..4995a2425b5 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -3163,7 +3163,7 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" sent to the client. If it is <literal>server</literal>, the backup data is written to the server at the pathname specified by the <literal>TARGET_DETAIL</literal> option. If it is - <literal>blackhole</literal>, the backup data is not sent + <literal>server-blackhole</literal>, the backup data is not sent anywhere; it is simply discarded. </para> diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 3117968d125..6ae7637afb9 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -268,9 +268,9 @@ PostgreSQL documentation <literal>/some/path</literal> directory. Storing a backup on the server requires superuser privileges or having privileges of the <literal>pg_write_server_files</literal> role. If the target is set to - <literal>blackhole</literal>, the contents are discarded and not - stored anywhere. This should only be used for testing purposes, as you - will not end up with an actual backup. + <literal>server-blackhole</literal>, the contents are discarded by the + server and not stored anywhere. This should only be used for testing + purposes, as you will not end up with an actual backup. </para> <para> diff --git a/src/backend/backup/basebackup_target.c b/src/backend/backup/basebackup_target.c index 1c250d2895c..61465458638 100644 --- a/src/backend/backup/basebackup_target.c +++ b/src/backend/backup/basebackup_target.c @@ -40,7 +40,7 @@ static void *server_check_detail(char *target, char *target_detail); static BaseBackupTargetType builtin_backup_targets[] = { { - "blackhole", reject_target_detail, blackhole_get_sink + "server-blackhole", reject_target_detail, blackhole_get_sink }, { "server", server_check_detail, server_get_sink @@ -185,7 +185,7 @@ initialize_target_list(void) /* * Normally, a get_sink function should construct and return a new bbsink that - * implements the backup target, but the 'blackhole' target just throws the + * implements the backup target, but the 'server-blackhole' target just throws the * data away. We could implement that by adding a bbsink that does nothing * but forward, but it's even cheaper to implement that by not adding a bbsink * at all. diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 2442131e179..17977b474b8 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -683,13 +683,13 @@ $node->command_ok( 'pg_basebackup --wal-method fetch runs'); $node->command_fails_like( - [ @pg_basebackup_defs, '--target' => 'blackhole' ], + [ @pg_basebackup_defs, '--target' => 'server-blackhole' ], qr/WAL cannot be streamed when a backup target is specified/, 'backup target requires --wal-method'); $node->command_fails_like( [ @pg_basebackup_defs, - '--target' => 'blackhole', + '--target' => 'server-blackhole', '--wal-method' => 'stream' ], qr/WAL cannot be streamed when a backup target is specified/, @@ -701,7 +701,7 @@ $node->command_fails_like( $node->command_fails_like( [ @pg_basebackup_defs, - '--target' => 'blackhole', + '--target' => 'server-blackhole', '--wal-method' => 'none', '--pgdata' => "$tempdir/blackhole" ], @@ -710,7 +710,7 @@ $node->command_fails_like( $node->command_fails_like( [ @pg_basebackup_defs, - '--target' => 'blackhole', + '--target' => 'server-blackhole', '--wal-method' => 'none', '--format' => 'tar' ], @@ -719,7 +719,7 @@ $node->command_fails_like( $node->command_ok( [ @pg_basebackup_defs, - '--target' => 'blackhole', + '--target' => 'server-blackhole', '--wal-method' => 'none' ], 'backup target blackhole'); @@ -780,7 +780,7 @@ $node->command_fails_like( $node->command_fails_like( [ @pg_basebackup_defs, - '--target' => 'blackhole', + '--target' => 'server-blackhole', '--pgdata' => "$tempdir/blackhole" ], qr/cannot specify both output directory and backup target/, -- 2.43.0
From c98db88fda52d4e6754a03fe4a5c951ce790ce46 Mon Sep 17 00:00:00 2001 From: Jakub Wartak <[email protected]> Date: Thu, 30 Jul 2026 11:02:42 +0200 Subject: [PATCH v07102026 2/4] pg_basebackup: add new "client-blackhole" (benchmarking) backup target Accept a new "client-blackhole" target value for pg_basebackup's --target option. Unlike the server-side "server-blackhole" target, the whole backup is sent to pg_basebackup as usual, but instead of being written to disk it is discarded: regular file contents go to the /dev/null device and directories/symbolic links/backup manifest are not created. In tar format the archive itself is sent to the /dev/null device too. This is a testing and development feature. It allows measuring how fast a backup can be produced by the server and network without being limited by the speed of the local storage (but subject to pg_basebackup single-threaded limitations like TLS decryption and/or checksum validation). The client-side discard is implemented in the astreamer extractors, which is controlled by a new backup_target_clientblackhole flag (backup_target stays NULL). Author: Jakub Wartak <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/CAKZiRmwwW-hDc3B6ERJB%2BpaX7RNSBcQLheq1KdsTf42cGuRvuA%40mail.gmail.com --- doc/src/sgml/ref/pg_basebackup.sgml | 7 +- src/bin/pg_basebackup/pg_basebackup.c | 115 +++++++++++++++---- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 18 +++ src/fe_utils/astreamer_file.c | 36 ++++-- src/include/fe_utils/astreamer.h | 3 +- 5 files changed, 150 insertions(+), 29 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 6ae7637afb9..fbb249b6382 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -269,7 +269,12 @@ PostgreSQL documentation server requires superuser privileges or having privileges of the <literal>pg_write_server_files</literal> role. If the target is set to <literal>server-blackhole</literal>, the contents are discarded by the - server and not stored anywhere. This should only be used for testing + server and not stored anywhere. If the target is set to + <literal>client-blackhole</literal>, the backup is sent to + <application>pg_basebackup</application> as usual, but discarded instead + of being written to disk; this can be used to measure how fast a backup + can be produced without being limited by the speed of the local + storage. Both blackhole targets should only be used for testing purposes, as you will not end up with an actual backup. </para> diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index c3b87a19e76..4628900da10 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -156,6 +156,7 @@ static bool manifest = true; static bool manifest_force_encode = false; static char *manifest_checksums = NULL; static DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC; +static bool backup_target_clientblackhole = false; static bool success = false; static bool made_new_pgdata = false; @@ -1146,7 +1147,8 @@ CreateBackupStreamer(char *archive_name, char *spclocation, directory = get_tablespace_mapping(spclocation); streamer = astreamer_extractor_new(directory, get_tablespace_mapping, - progress_update_filename); + progress_update_filename, + backup_target_clientblackhole); } else { @@ -1158,8 +1160,15 @@ CreateBackupStreamer(char *archive_name, char *spclocation, * Normally, we write it to the archive name provided by the caller, * but when the base directory is "-" that means we need to write to * standard output. + * + * When discarding writes, we send the archive to the null device. */ - if (strcmp(basedir, "-") == 0) + if (backup_target_clientblackhole) + { + snprintf(archive_filename, sizeof(archive_filename), "%s", DEVNULL); + archive_file = NULL; + } + else if (strcmp(basedir, "-") == 0) { snprintf(archive_filename, sizeof(archive_filename), "-"); archive_file = stdout; @@ -1171,25 +1180,32 @@ CreateBackupStreamer(char *archive_name, char *spclocation, archive_file = NULL; } + /* + * Setup streamer. In case of using client blackhole, it does not make sense + * to append compression suffixes. + */ if (compress->algorithm == PG_COMPRESSION_NONE) streamer = astreamer_plain_writer_new(archive_filename, archive_file); else if (compress->algorithm == PG_COMPRESSION_GZIP) { - strlcat(archive_filename, ".gz", sizeof(archive_filename)); + if (!backup_target_clientblackhole) + strlcat(archive_filename, ".gz", sizeof(archive_filename)); streamer = astreamer_gzip_writer_new(archive_filename, archive_file, compress); } else if (compress->algorithm == PG_COMPRESSION_LZ4) { - strlcat(archive_filename, ".lz4", sizeof(archive_filename)); + if (!backup_target_clientblackhole) + strlcat(archive_filename, ".lz4", sizeof(archive_filename)); streamer = astreamer_plain_writer_new(archive_filename, archive_file); streamer = astreamer_lz4_compressor_new(streamer, compress); } else if (compress->algorithm == PG_COMPRESSION_ZSTD) { - strlcat(archive_filename, ".zst", sizeof(archive_filename)); + if (!backup_target_clientblackhole) + strlcat(archive_filename, ".zst", sizeof(archive_filename)); streamer = astreamer_plain_writer_new(archive_filename, archive_file); streamer = astreamer_zstd_compressor_new(streamer, compress); @@ -1470,6 +1486,15 @@ ReceiveArchiveStreamChunk(size_t r, char *copybuf, void *callback_data) */ if (state->manifest_inject_streamer != NULL) state->manifest_buffer = createPQExpBuffer(); + else if (backup_target_clientblackhole) + { + /* Throw away the manifest too */ + snprintf(state->manifest_filename, + sizeof(state->manifest_filename), "%s", DEVNULL); + state->manifest_file = fopen(DEVNULL, "wb"); + if (state->manifest_file == NULL) + pg_fatal("could not open file \"%s\": %m", DEVNULL); + } else { snprintf(state->manifest_filename, @@ -1683,11 +1708,22 @@ ReceiveBackupManifest(PGconn *conn) { WriteManifestState state; - snprintf(state.filename, sizeof(state.filename), - "%s/backup_manifest.tmp", basedir); - state.file = fopen(state.filename, "wb"); - if (state.file == NULL) - pg_fatal("could not create file \"%s\": %m", state.filename); + if (backup_target_clientblackhole) + { + /* Throw away the manifest */ + snprintf(state.filename, sizeof(state.filename), "%s", DEVNULL); + state.file = fopen(DEVNULL, "wb"); + if (state.file == NULL) + pg_fatal("could not open file \"%s\": %m", DEVNULL); + } + else + { + snprintf(state.filename, sizeof(state.filename), + "%s/backup_manifest.tmp", basedir); + state.file = fopen(state.filename, "wb"); + if (state.file == NULL) + pg_fatal("could not create file \"%s\": %m", state.filename); + } ReceiveCopyData(conn, ReceiveBackupManifestChunk, &state); @@ -1970,6 +2006,9 @@ BaseBackup(char *compression_algorithm, char *compression_detail, compression_detail); } + if (verbose && backup_target_clientblackhole) + pg_log_info("the backup is being discarded and cannot be used for recovery"); + if (verbose) pg_log_info("initiating base backup, waiting for checkpoint to complete"); @@ -2052,7 +2091,8 @@ BaseBackup(char *compression_algorithm, char *compression_detail, * won't be storing anything into these directories and thus should * not create them. */ - if (backup_target == NULL && format == 'p' && !PQgetisnull(res, i, 1)) + if (backup_target == NULL && !backup_target_clientblackhole && format == 'p' && + !PQgetisnull(res, i, 1)) { char *path = PQgetvalue(res, i, 1); @@ -2276,11 +2316,8 @@ BaseBackup(char *compression_algorithm, char *compression_detail, * synced after being completed. In plain format, all the data of the * base directory is synced, taking into account all the tablespaces. * Errors are not considered fatal. - * - * If, however, there's a backup target, we're not writing anything - * locally, so in that case we skip this step. */ - if (do_sync && backup_target == NULL) + if (do_sync && backup_target == NULL && !backup_target_clientblackhole) { if (verbose) pg_log_info("syncing data to disk ..."); @@ -2302,7 +2339,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail, * without a backup_manifest file, decreasing the chances that a directory * we leave behind will be mistaken for a valid backup. */ - if (!writing_to_stdout && manifest && backup_target == NULL) + if (!writing_to_stdout && manifest && backup_target == NULL && !backup_target_clientblackhole) { char tmp_filename[MAXPGPATH]; char filename[MAXPGPATH]; @@ -2479,7 +2516,15 @@ main(int argc, char **argv) temp_replication_slot = false; break; case 't': - backup_target = pg_strdup(optarg); + + /* + * Target: everything else than "client-blackhole" is passed + * through to the server as the backup target. + */ + if (strcmp(optarg, "client-blackhole") == 0) + backup_target_clientblackhole = true; + else + backup_target = pg_strdup(optarg); break; case 'T': tablespace_list_append(optarg); @@ -2579,6 +2624,17 @@ main(int argc, char **argv) backup_target = NULL; } + if (backup_target_clientblackhole) + { + if (basedir != NULL || backup_target != NULL) + { + pg_log_error("cannot specify both output directory and backup target"); + pg_log_error_hint("Try \"%s --help\" for more information.", progname); + exit(1); + } + basedir = pg_strdup(DEVNULL); + } + /* * Can't use --format with --target. Without --target, default format is * tar. @@ -2671,6 +2727,23 @@ main(int argc, char **argv) exit(1); } + /* + * The "client-blackhole" target throws the backup away as it is received. + * As we do not have directories prepared, we cannot stream or write recovery + * configuration. + */ + if (backup_target_clientblackhole) + { + if (includewal == STREAM_WAL) + { + pg_log_error("WAL cannot be streamed with the \"client-blackhole\" backup target"); + pg_log_error_hint("Use \"%s\" or \"%s\".", "-X none", "-X fetch"); + exit(1); + } + if (writerecoveryconf) + pg_fatal("recovery configuration cannot be written with the \"client-blackhole\" backup target"); + } + /* * Sanity checks for WAL method. */ @@ -2824,9 +2897,11 @@ main(int argc, char **argv) /* * If an output directory was specified, verify that it exists, or create * it. Note that for a tar backup, an output directory of "-" means we are - * writing to stdout, so do nothing in that case. + * writing to stdout, so do nothing in that case. When discarding writes, + * we don't create anything on disk at all. */ - if (basedir != NULL && (format == 'p' || strcmp(basedir, "-") != 0)) + if (basedir != NULL && !backup_target_clientblackhole && + (format == 'p' || strcmp(basedir, "-") != 0)) verify_dir_is_empty_or_create(basedir, &made_new_pgdata, &found_existing_pgdata); /* determine remote server's xlog segment size */ @@ -2834,7 +2909,7 @@ main(int argc, char **argv) exit(1); /* Create pg_wal symlink, if required */ - if (xlog_dir) + if (xlog_dir && !backup_target_clientblackhole) { char *linkloc; diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 17977b474b8..33fb446ba52 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -324,6 +324,24 @@ $node->command_ok( ok(-f "$tempdir/tarbackup/base.tar", 'backup tar was created'); rmtree("$tempdir/tarbackup"); +# The "client-blackhole" target receives the whole backup but throws it away. +$node->command_ok( + [ + @pg_basebackup_defs, + '--target' => 'client-blackhole', + '--format' => 'plain', + '--wal-method' => 'none' + ], + 'client-blackhole target in plain format'); +$node->command_ok( + [ + @pg_basebackup_defs, + '--target' => 'client-blackhole', + '--format' => 'tar', + '--wal-method' => 'fetch' + ], + 'client-blackhole target in tar format'); + $node->command_fails_like( [ @pg_basebackup_defs, diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c index fb36cecc22a..e4fa2596b08 100644 --- a/src/fe_utils/astreamer_file.c +++ b/src/fe_utils/astreamer_file.c @@ -37,6 +37,7 @@ typedef struct astreamer_extractor void (*report_output_file) (const char *); char filename[MAXPGPATH]; FILE *file; + bool discard_backup; } astreamer_extractor; static void astreamer_plain_writer_content(astreamer *streamer, @@ -60,7 +61,8 @@ static void astreamer_extractor_finalize(astreamer *streamer); static void astreamer_extractor_free(astreamer *streamer); static void extract_directory(const char *filename, mode_t mode); static void extract_link(const char *filename, const char *linktarget); -static FILE *create_file_for_extract(const char *filename, mode_t mode); +static FILE *create_file_for_extract(const char *filename, mode_t mode, + bool discard_backup); static const astreamer_ops astreamer_extractor_ops = { .content = astreamer_extractor_content, @@ -181,11 +183,15 @@ astreamer_plain_writer_free(astreamer *streamer) * 'report_output_file' is a function that will be called each time we open a * new output file. The pathname to that file is passed as an argument. If * NULL, the call is skipped. + * + * If 'discard_backup' is true, the extracted archive is thrown away rather + * than written to the filesystem. */ astreamer * astreamer_extractor_new(const char *basepath, const char *(*link_map) (const char *), - void (*report_output_file) (const char *)) + void (*report_output_file) (const char *), + bool discard_backup) { astreamer_extractor *streamer; @@ -195,6 +201,7 @@ astreamer_extractor_new(const char *basepath, streamer->basepath = pstrdup(basepath); streamer->link_map = link_map; streamer->report_output_file = report_output_file; + streamer->discard_backup = discard_backup; return &streamer->base; } @@ -231,13 +238,19 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member, if (mystreamer->filename[fnamelen - 1] == '/') mystreamer->filename[fnamelen - 1] = '\0'; - /* Dispatch based on file type. */ + /* + * Dispatch based on file type. + */ if (member->is_regular) mystreamer->file = create_file_for_extract(mystreamer->filename, - member->mode); + member->mode, + mystreamer->discard_backup); else if (member->is_directory) - extract_directory(mystreamer->filename, member->mode); + { + if (!mystreamer->discard_backup) + extract_directory(mystreamer->filename, member->mode); + } else if (member->is_symlink) { const char *linktarget = member->linktarget; @@ -252,7 +265,8 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member, member->linktarget); } - extract_link(mystreamer->filename, linktarget); + if (!mystreamer->discard_backup) + extract_link(mystreamer->filename, linktarget); } /* Report output file change. */ @@ -369,10 +383,18 @@ extract_link(const char *filename, const char *linktarget) * Return the resulting handle so we can write the content to the file. */ static FILE * -create_file_for_extract(const char *filename, mode_t mode) +create_file_for_extract(const char *filename, mode_t mode, bool discard_backup) { FILE *file; + if (discard_backup) + { + file = fopen(DEVNULL, "wb"); + if (file == NULL) + pg_fatal("could not open file \"%s\": %m", DEVNULL); + return file; + } + file = fopen(filename, "wb"); if (file == NULL) pg_fatal("could not create file \"%s\": %m", filename); diff --git a/src/include/fe_utils/astreamer.h b/src/include/fe_utils/astreamer.h index 8329e4efbc5..7206dc0c48b 100644 --- a/src/include/fe_utils/astreamer.h +++ b/src/include/fe_utils/astreamer.h @@ -215,7 +215,8 @@ extern astreamer *astreamer_gzip_writer_new(char *pathname, FILE *file, pg_compress_specification *compress); extern astreamer *astreamer_extractor_new(const char *basepath, const char *(*link_map) (const char *), - void (*report_output_file) (const char *)); + void (*report_output_file) (const char *), + bool discard_backup); extern astreamer *astreamer_gzip_decompressor_new(astreamer *next); extern astreamer *astreamer_lz4_compressor_new(astreamer *next, -- 2.43.0
