I made a few amendments (here's v5) and was ready to push, when I noticed that _StartBlobs() does not seem to be doing the right thing. Did you test this with a few large objects?
The reason I noticed is I wondered about the amount of open() calls (plus zlib function calls) we would save by keeping an FD open to /dev/null, rather than opening it over and over for each object -- ie., maybe not touch setFilePath() at all, if possible. That looks perhaps too invasive, so maybe not. But do audit other callsites that may open files. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 5627c18f5a26ad8314f0590ce8ccf3b4093efb74 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Mon, 5 Mar 2018 13:42:29 -0300 Subject: [PATCH v5] Allow parallel dump to go to /dev/null This is useful to quickly determine whether a dump would finish without errors. Author: Michael Banck Discussion: https://postgr.es/m/20180201132446.ga13...@nighthawk.caipicrew.dd-dns.de --- src/bin/pg_dump/compress_io.c | 8 +++++-- src/bin/pg_dump/pg_backup_directory.c | 43 ++++++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c index a96da15dc1..a83b1e2ef5 100644 --- a/src/bin/pg_dump/compress_io.c +++ b/src/bin/pg_dump/compress_io.c @@ -496,7 +496,8 @@ cfopen_read(const char *path, const char *mode) * * If 'compression' is non-zero, a gzip compressed stream is opened, and * 'compression' indicates the compression level used. The ".gz" suffix - * is automatically added to 'path' in that case. + * is automatically added to 'path' in that case (unless it's the null + * device). * * On failure, return NULL with an error code in errno. */ @@ -512,7 +513,10 @@ cfopen_write(const char *path, const char *mode, int compression) #ifdef HAVE_LIBZ char *fname; - fname = psprintf("%s.gz", path); + if (strcmp(path, DEVNULL) != 0) + fname = psprintf("%s.gz", path); + else + fname = pg_strdup(path); fp = cfopen(fname, mode, compression); free_keep_errno(fname); #else diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c index 4aabb40f59..3b77023d77 100644 --- a/src/bin/pg_dump/pg_backup_directory.c +++ b/src/bin/pg_dump/pg_backup_directory.c @@ -54,6 +54,7 @@ typedef struct cfp *blobsTocFH; /* file handle for blobs.toc */ ParallelState *pstate; /* for parallel backup / restore */ + bool discard; /* target is DEVNULL */ } lclContext; typedef struct @@ -158,6 +159,8 @@ InitArchiveFmt_Directory(ArchiveHandle *AH) ctx->directory = AH->fSpec; + ctx->discard = strcmp(ctx->directory, DEVNULL) == 0; + if (AH->mode == archModeWrite) { struct stat st; @@ -192,9 +195,15 @@ InitArchiveFmt_Directory(ArchiveHandle *AH) } } - if (!is_empty && mkdir(ctx->directory, 0700) < 0) - exit_horribly(modulename, "could not create directory \"%s\": %s\n", - ctx->directory, strerror(errno)); + /* + * Create the output directory, unless it exists already and is empty. + * If we are discarding output to the null device, we must not + * create it. + */ + if (!is_empty && !ctx->discard) + if (mkdir(ctx->directory, 0700) < 0) + exit_horribly(modulename, "could not create directory \"%s\": %s\n", + ctx->directory, strerror(errno)); } else { /* Read Mode */ @@ -602,8 +611,10 @@ _CloseArchive(ArchiveHandle *AH) /* * In directory mode, there is no need to sync all the entries * individually. Just recurse once through all the files generated. + * If we are discarding output to the null device, it does not + * need syncing and would emit a warning if we did. */ - if (AH->dosync) + if (AH->dosync && !ctx->discard) fsync_dir_recurse(ctx->directory, progname); } AH->FH = NULL; @@ -659,7 +670,14 @@ _StartBlob(ArchiveHandle *AH, TocEntry *te, Oid oid) lclContext *ctx = (lclContext *) AH->formatData; char fname[MAXPGPATH]; - snprintf(fname, MAXPGPATH, "%s/blob_%u.dat", ctx->directory, oid); + /* + * If we are discarding output to the null device, just use that as + * fname. + */ + if (ctx->discard) + snprintf(fname, MAXPGPATH, DEVNULL); + else + snprintf(fname, MAXPGPATH, "%s/blob_%u.dat", ctx->directory, oid); ctx->dataFH = cfopen_write(fname, PG_BINARY_W, AH->compression); @@ -721,9 +739,18 @@ setFilePath(ArchiveHandle *AH, char *buf, const char *relativeFilename) if (strlen(dname) + 1 + strlen(relativeFilename) + 1 > MAXPGPATH) exit_horribly(modulename, "file name too long: \"%s\"\n", dname); - strcpy(buf, dname); - strcat(buf, "/"); - strcat(buf, relativeFilename); + /* + * If we are discarding output to the null device, we cannot set a file + * path and just set the buffer to the null device. + */ + if (ctx->discard) + strcpy(buf, DEVNULL); + else + { + strcpy(buf, dname); + strcat(buf, "/"); + strcat(buf, relativeFilename); + } } /* -- 2.11.0
