On Fri, Jan 6, 2017 at 11:07 PM, Magnus Hagander <mag...@hagander.net> wrote:
> A few further notes:
Thanks for the review.
> You are using the filemode to gzopen and the mode_compression variable to
> set the compression level. The pre-existing code in pg_basebackup uses
> gzsetparams(). Is there a particular reason you didn't do it the same way?
>
> Small comment:
> - if (pad_to_size)
> + if (pad_to_size && dir_data->compression == 0)
> {
> /* Always pre-pad on regular files */
>
>
> That "always" is not true anymore. Commit-time cleanup can be done of that.
>
> The rest of this looks good to me, but please comment on the gzopen part
> before we proceed to commit :)
Yes using gzsetparams() looks cleaner. I actually thought about using
the same logic as pg_dump. Attached is an updated patch.
There is something I forgot. With this patch,
FindStreamingStart()@pg_receivexlog.c is actually broken. In short it
forgets to consider files that have been compressed at the last run of
pg_receivexlog and will try to stream changes from the beginning. I
can see that gzip -l provides this information... But I have yet to
find something in zlib that allows a cheap lookup as startup of
streaming should be fast. Looking at how gzip -l does it may be faster
than looking at the docs.
--
Michael
diff --git a/doc/src/sgml/ref/pg_receivexlog.sgml b/doc/src/sgml/ref/pg_receivexlog.sgml
index bfa055b58b..8c1ea9a2e2 100644
--- a/doc/src/sgml/ref/pg_receivexlog.sgml
+++ b/doc/src/sgml/ref/pg_receivexlog.sgml
@@ -180,6 +180,19 @@ PostgreSQL documentation
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
+ <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
+ <listitem>
+ <para>
+ Enables gzip compression of transaction logs, and specifies the
+ compression level (0 through 9, 0 being no compression and 9 being best
+ compression). The suffix <filename>.gz</filename> will
+ automatically be added to all filenames.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
<para>
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 3f83d87e50..87ab1bb92f 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -472,7 +472,7 @@ LogStreamerMain(logstreamer_param *param)
stream.partial_suffix = NULL;
if (format == 'p')
- stream.walmethod = CreateWalDirectoryMethod(param->xlog, do_sync);
+ stream.walmethod = CreateWalDirectoryMethod(param->xlog, 0, do_sync);
else
stream.walmethod = CreateWalTarMethod(param->xlog, compresslevel, do_sync);
diff --git a/src/bin/pg_basebackup/pg_receivexlog.c b/src/bin/pg_basebackup/pg_receivexlog.c
index b6f57a878c..9d18f22c59 100644
--- a/src/bin/pg_basebackup/pg_receivexlog.c
+++ b/src/bin/pg_basebackup/pg_receivexlog.c
@@ -34,6 +34,7 @@
/* Global options */
static char *basedir = NULL;
static int verbose = 0;
+static int compresslevel = 0;
static int noloop = 0;
static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
static volatile bool time_to_abort = false;
@@ -75,6 +76,7 @@ usage(void)
printf(_(" --synchronous flush transaction log immediately after writing\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
+ printf(_(" -Z, --compress=0-9 compress logs with given compression level\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\nConnection options:\n"));
printf(_(" -d, --dbname=CONNSTR connection string\n"));
@@ -338,7 +340,8 @@ StreamLog(void)
stream.synchronous = synchronous;
stream.do_sync = true;
stream.mark_done = false;
- stream.walmethod = CreateWalDirectoryMethod(basedir, stream.do_sync);
+ stream.walmethod = CreateWalDirectoryMethod(basedir, compresslevel,
+ stream.do_sync);
stream.partial_suffix = ".partial";
ReceiveXlogStream(conn, &stream);
@@ -389,6 +392,7 @@ main(int argc, char **argv)
{"status-interval", required_argument, NULL, 's'},
{"slot", required_argument, NULL, 'S'},
{"verbose", no_argument, NULL, 'v'},
+ {"compress", required_argument, NULL, 'Z'},
/* action */
{"create-slot", no_argument, NULL, 1},
{"drop-slot", no_argument, NULL, 2},
@@ -419,7 +423,7 @@ main(int argc, char **argv)
}
}
- while ((c = getopt_long(argc, argv, "D:d:h:p:U:s:S:nwWv",
+ while ((c = getopt_long(argc, argv, "D:d:h:p:U:s:S:nwWvZ:",
long_options, &option_index)) != -1)
{
switch (c)
@@ -469,6 +473,15 @@ main(int argc, char **argv)
case 'v':
verbose++;
break;
+ case 'Z':
+ compresslevel = atoi(optarg);
+ if (compresslevel < 0 || compresslevel > 9)
+ {
+ fprintf(stderr, _("%s: invalid compression level \"%s\"\n"),
+ progname, optarg);
+ exit(1);
+ }
+ break;
/* action */
case 1:
do_create_slot = true;
@@ -535,6 +548,16 @@ main(int argc, char **argv)
exit(1);
}
+#ifndef HAVE_LIBZ
+ if (compresslevel != 0)
+ {
+ fprintf(stderr,
+ _("%s: this build does not support compression\n"),
+ progname);
+ exit(1);
+ }
+#endif
+
/*
* Check existence of destination folder.
*/
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index 88ee603b8b..f0726f845b 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -41,6 +41,7 @@
typedef struct DirectoryMethodData
{
char *basedir;
+ int compression;
bool sync;
} DirectoryMethodData;
static DirectoryMethodData *dir_data = NULL;
@@ -55,6 +56,9 @@ typedef struct DirectoryMethodFile
char *pathname;
char *fullpath;
char *temp_suffix;
+#ifdef HAVE_LIBZ
+ gzFile gzfp;
+#endif
} DirectoryMethodFile;
static char *
@@ -70,17 +74,40 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
static char tmppath[MAXPGPATH];
int fd;
DirectoryMethodFile *f;
+#ifdef HAVE_LIBZ
+ gzFile gzfp;
+#endif
- snprintf(tmppath, sizeof(tmppath), "%s/%s%s",
- dir_data->basedir, pathname, temp_suffix ? temp_suffix : "");
+ snprintf(tmppath, sizeof(tmppath), "%s/%s%s%s",
+ dir_data->basedir, pathname,
+ dir_data->compression > 0 ? ".gz" : "",
+ temp_suffix ? temp_suffix : "");
- fd = open(tmppath, O_WRONLY | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR);
- if (fd < 0)
- return NULL;
+#ifdef HAVE_LIBZ
+ if (dir_data->compression > 0)
+ {
+ gzfp = gzopen(tmppath, "wb");
+ if (gzfp == NULL)
+ return NULL;
- if (pad_to_size)
+ if (gzsetparams(gzfp, dir_data->compression,
+ Z_DEFAULT_STRATEGY) != Z_OK)
+ {
+ gzclose(gzfp);
+ return NULL;
+ }
+ }
+ else
+#endif
+ {
+ fd = open(tmppath, O_WRONLY | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR);
+ if (fd < 0)
+ return NULL;
+ }
+
+ /* Do pre-padding on non-compressed files */
+ if (pad_to_size && dir_data->compression == 0)
{
- /* Always pre-pad on regular files */
char *zerobuf;
int bytes;
@@ -120,13 +147,23 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
if (fsync_fname(tmppath, false, progname) != 0 ||
fsync_parent_path(tmppath, progname) != 0)
{
- close(fd);
+#ifdef HAVE_LIBZ
+ if (dir_data->compression > 0)
+ gzclose(gzfp);
+ else
+#endif
+ close(fd);
return NULL;
}
}
f = pg_malloc0(sizeof(DirectoryMethodFile));
- f->fd = fd;
+#ifdef HAVE_LIBZ
+ if (dir_data->compression > 0)
+ f->gzfp = gzfp;
+ else
+#endif
+ f->fd = fd;
f->currpos = 0;
f->pathname = pg_strdup(pathname);
f->fullpath = pg_strdup(tmppath);
@@ -144,7 +181,12 @@ dir_write(Walfile f, const void *buf, size_t count)
Assert(f != NULL);
- r = write(df->fd, buf, count);
+#ifdef HAVE_LIBZ
+ if (dir_data->compression > 0)
+ r = (ssize_t) gzwrite(df->gzfp, buf, count);
+ else
+#endif
+ r = write(df->fd, buf, count);
if (r > 0)
df->currpos += r;
return r;
@@ -169,7 +211,12 @@ dir_close(Walfile f, WalCloseMethod method)
Assert(f != NULL);
- r = close(df->fd);
+#ifdef HAVE_LIBZ
+ if (dir_data->compression > 0)
+ r = gzclose(df->gzfp);
+ else
+#endif
+ r = close(df->fd);
if (r == 0)
{
@@ -180,17 +227,22 @@ dir_close(Walfile f, WalCloseMethod method)
* If we have a temp prefix, normal operation is to rename the
* file.
*/
- snprintf(tmppath, sizeof(tmppath), "%s/%s%s",
- dir_data->basedir, df->pathname, df->temp_suffix);
- snprintf(tmppath2, sizeof(tmppath2), "%s/%s",
- dir_data->basedir, df->pathname);
+ snprintf(tmppath, sizeof(tmppath), "%s/%s%s%s",
+ dir_data->basedir, df->pathname,
+ dir_data->compression > 0 ? ".gz" : "",
+ df->temp_suffix);
+ snprintf(tmppath2, sizeof(tmppath2), "%s/%s%s",
+ dir_data->basedir, df->pathname,
+ dir_data->compression > 0 ? ".gz" : "");
r = durable_rename(tmppath, tmppath2, progname);
}
else if (method == CLOSE_UNLINK)
{
/* Unlink the file once it's closed */
- snprintf(tmppath, sizeof(tmppath), "%s/%s%s",
- dir_data->basedir, df->pathname, df->temp_suffix ? df->temp_suffix : "");
+ snprintf(tmppath, sizeof(tmppath), "%s/%s%s%s",
+ dir_data->basedir, df->pathname,
+ dir_data->compression > 0 ? ".gz" : "",
+ df->temp_suffix ? df->temp_suffix : "");
r = unlink(tmppath);
}
else
@@ -277,7 +329,7 @@ dir_finish(void)
WalWriteMethod *
-CreateWalDirectoryMethod(const char *basedir, bool sync)
+CreateWalDirectoryMethod(const char *basedir, int compression, bool sync)
{
WalWriteMethod *method;
@@ -293,6 +345,7 @@ CreateWalDirectoryMethod(const char *basedir, bool sync)
method->getlasterror = dir_getlasterror;
dir_data = pg_malloc0(sizeof(DirectoryMethodData));
+ dir_data->compression = compression;
dir_data->basedir = pg_strdup(basedir);
dir_data->sync = sync;
diff --git a/src/bin/pg_basebackup/walmethods.h b/src/bin/pg_basebackup/walmethods.h
index c1723d53b5..2cd8b6d755 100644
--- a/src/bin/pg_basebackup/walmethods.h
+++ b/src/bin/pg_basebackup/walmethods.h
@@ -41,7 +41,8 @@ struct WalWriteMethod
* (only implements the methods required for pg_basebackup,
* not all those required for pg_receivexlog)
*/
-WalWriteMethod *CreateWalDirectoryMethod(const char *basedir, bool sync);
+WalWriteMethod *CreateWalDirectoryMethod(const char *basedir,
+ int compression, bool sync);
WalWriteMethod *CreateWalTarMethod(const char *tarbase, int compression, bool sync);
/* Cleanup routines for previously-created methods */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
