Re: [HACKERS] pg_basebackup wish list
On Thu, Sep 15, 2016 at 9:26 AM, Peter Eisentraut wrote: > On 9/13/16 7:24 PM, Michael Paquier wrote: >> PostgresNode.pm had better use the new --noclean option in its calls, >> the new default is not useful for debugging. > > We don't do it for initdb either. Is that a problem? Right. In case of failure it may prove to be useful for debugging if we'd use it. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On 9/13/16 7:24 PM, Michael Paquier wrote: > PostgresNode.pm had better use the new --noclean option in its calls, > the new default is not useful for debugging. We don't do it for initdb either. Is that a problem? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Wed, Sep 14, 2016 at 6:52 AM, Peter Eisentraut wrote: > On 9/12/16 3:31 PM, Tom Lane wrote: >> Hm, there was just a kerfuffle about spelling things like this >> "--no-clean" etc. I wasn't on board with removing existing spellings, >> but surely all newly added switches should use the "no-" spelling? > > This is supposed to be parallel to initdb's option. So if we rename or > migrate the one in initdb, then we can just hard-rename this one. But > until that is decided, it seems better to keep it the same as initdb has > right now. PostgresNode.pm had better use the new --noclean option in its calls, the new default is not useful for debugging. Please see attached. -- Michael diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index fede1e6..0b8c8e6 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -476,7 +476,7 @@ sub backup print "# Taking pg_basebackup $backup_name from node \"$name\"\n"; TestLib::system_or_bail('pg_basebackup', '-D', $backup_path, '-p', $port, - '-x'); + '-x', '--noclean'); print "# Backup finished\n"; } -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On 9/12/16 3:31 PM, Tom Lane wrote: > Hm, there was just a kerfuffle about spelling things like this > "--no-clean" etc. I wasn't on board with removing existing spellings, > but surely all newly added switches should use the "no-" spelling? This is supposed to be parallel to initdb's option. So if we rename or migrate the one in initdb, then we can just hard-rename this one. But until that is decided, it seems better to keep it the same as initdb has right now. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
Peter Eisentraut writes: > On 8/19/16 1:04 AM, Masahiko Sawada wrote: >> I agree with adding this as an option and removing directory by default. >> And it looks good to me except for missing new line in usage output. >> >> printf(_(" -l, --label=LABEL set backup label\n")); >> + printf(_(" -n, --noclean do not clean up after errors")); >> printf(_(" -P, --progress show progress information\n")); > Committed with that fix. Thanks. Hm, there was just a kerfuffle about spelling things like this "--no-clean" etc. I wasn't on board with removing existing spellings, but surely all newly added switches should use the "no-" spelling? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On 8/19/16 1:04 AM, Masahiko Sawada wrote: > I agree with adding this as an option and removing directory by default. > And it looks good to me except for missing new line in usage output. > > printf(_(" -l, --label=LABEL set backup label\n")); > + printf(_(" -n, --noclean do not clean up after errors")); > printf(_(" -P, --progress show progress information\n")); Committed with that fix. Thanks. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On 19 August 2016 at 08:46, Michael Paquier wrote: > On Fri, Aug 19, 2016 at 2:04 PM, Masahiko Sawada > wrote: >> I agree with adding this as an option and removing directory by default. >> And it looks good to me except for missing new line in usage output. >> >> printf(_(" -l, --label=LABEL set backup label\n")); >> + printf(_(" -n, --noclean do not clean up after errors")); >> printf(_(" -P, --progress show progress information\n")); >> >> Registered this patch to CF1. > > +1 for the idea. Looking at the patch it is taking a sane approach. Apart from this one liner change we look good to go. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Fri, Aug 19, 2016 at 2:04 PM, Masahiko Sawada wrote: > I agree with adding this as an option and removing directory by default. > And it looks good to me except for missing new line in usage output. > > printf(_(" -l, --label=LABEL set backup label\n")); > + printf(_(" -n, --noclean do not clean up after errors")); > printf(_(" -P, --progress show progress information\n")); > > Registered this patch to CF1. +1 for the idea. Looking at the patch it is taking a sane approach. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Fri, Aug 19, 2016 at 7:06 AM, Peter Eisentraut wrote: > On 7/12/16 9:55 PM, Masahiko Sawada wrote: >> And what I think is pg_baseback never remove the directory specified >> by -D option even if execution is failed. initdb command behaves so. >> I think it's helpful for backup operation. > > This has been bothering me as well. > > How about the attached patch as a start? > Thank you for the patch! I agree with adding this as an option and removing directory by default. And it looks good to me except for missing new line in usage output. printf(_(" -l, --label=LABEL set backup label\n")); + printf(_(" -n, --noclean do not clean up after errors")); printf(_(" -P, --progress show progress information\n")); Registered this patch to CF1. Regards, -- Masahiko Sawada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On 7/12/16 9:55 PM, Masahiko Sawada wrote: > And what I think is pg_baseback never remove the directory specified > by -D option even if execution is failed. initdb command behaves so. > I think it's helpful for backup operation. This has been bothering me as well. How about the attached patch as a start? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >From 910310b7eab88af8972906307a55e02e64618da7 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 18 Aug 2016 12:00:00 -0400 Subject: [PATCH] pg_basebackup: Clean created directories on failure Like initdb, clean up created data and xlog directories, unless the new -n/--noclean option is specified. Tablespace directories are not cleaned up, but a message is written about that. --- doc/src/sgml/ref/pg_basebackup.sgml | 18 + src/bin/pg_basebackup/pg_basebackup.c| 98 ++-- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 10 ++- 3 files changed, 119 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 03615da..9f1eae1 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -399,6 +399,24 @@ Options + -n + --noclean + + +By default, when pg_basebackup aborts with an +error, it removes any directories it might have created before +discovering that it cannot finish the job (for example, data directory +and transaction log directory). This option inhibits tidying-up and is +thus useful for debugging. + + + +Note that tablespace directories are not cleaned up either way. + + + + + -P --progress diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index ed41db8..d13a9a3 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -57,6 +57,7 @@ static TablespaceList tablespace_dirs = {NULL, NULL}; static char *xlog_dir = ""; static char format = 'p'; /* p(lain)/t(ar) */ static char *label = "pg_basebackup base backup"; +static bool noclean = false; static bool showprogress = false; static int verbose = 0; static int compresslevel = 0; @@ -68,6 +69,13 @@ static int standby_message_timeout = 10 * 1000; /* 10 sec = default */ static pg_time_t last_progress_report = 0; static int32 maxrate = 0; /* no limit by default */ +static bool success = false; +static bool made_new_pgdata = false; +static bool found_existing_pgdata = false; +static bool made_new_xlogdir = false; +static bool found_existing_xlogdir = false; +static bool made_tablespace_dirs = false; +static bool found_tablespace_dirs = false; /* Progress counters */ static uint64 totalsize; @@ -81,6 +89,7 @@ static int bgpipe[2] = {-1, -1}; /* Handle to child process */ static pid_t bgchild = -1; +static bool in_log_streamer = false; /* End position for xlog streaming, empty string if unknown yet */ static XLogRecPtr xlogendptr; @@ -97,7 +106,7 @@ static PQExpBuffer recoveryconfcontents = NULL; /* Function headers */ static void usage(void); static void disconnect_and_exit(int code); -static void verify_dir_is_empty_or_create(char *dirname); +static void verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found); static void progress_report(int tablespacenum, const char *filename, bool force); static void ReceiveTarFile(PGconn *conn, PGresult *res, int rownum); @@ -114,6 +123,69 @@ static void tablespace_list_append(const char *arg); static void +cleanup_directories_atexit(void) +{ + if (success || in_log_streamer) + return; + + if (!noclean) + { + if (made_new_pgdata) + { + fprintf(stderr, _("%s: removing data directory \"%s\"\n"), + progname, basedir); + if (!rmtree(basedir, true)) +fprintf(stderr, _("%s: failed to remove data directory\n"), + progname); + } + else if (found_existing_pgdata) + { + fprintf(stderr, + _("%s: removing contents of data directory \"%s\"\n"), + progname, basedir); + if (!rmtree(basedir, false)) +fprintf(stderr, _("%s: failed to remove contents of data directory\n"), + progname); + } + + if (made_new_xlogdir) + { + fprintf(stderr, _("%s: removing transaction log directory \"%s\"\n"), + progname, xlog_dir); + if (!rmtree(xlog_dir, true)) +fprintf(stderr, _("%s: failed to remove transaction log directory\n"), + progname); + } + else if (found_existing_xlogdir) + { + fprintf(stderr, + _("%s: removing contents of transaction log directory \"%s\"\n"), + progname, xlog_dir); + if (!rmtree(xlog_dir, false)) +fprintf(stderr, _("%s: failed to remove contents of transaction log directory\n"), + progname); + } + } + else + { + if (made_new_pgdata || found_existing_pgdata) + fprintf(stderr, +
Re: [HACKERS] pg_basebackup wish list
On Thu, Jul 28, 2016 at 4:44 AM, Amit Kapila wrote: > On Tue, Jul 26, 2016 at 11:58 AM, Fujii Masao wrote: >> On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes wrote: >>> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >>> wrote: On 7/12/16 12:53 PM, Jeff Janes wrote: > The --help message for pg_basebackup says: > > -Z, --compress=0-9 compress tar output with given compression level > > But -Z0 is then rejected as 'invalid compression level "0"'. The real > docs do say 1-9, only the --help message has this bug. Trivial patch > attached. pg_dump --help and man page say it supports 0..9. Maybe we should make that more consistent. >>> >>> pg_dump actually does support -Z0, though. Well, sort of. It outputs >>> plain text. Rather than plain text wrapped in some kind of dummy gzip >>> header, which is what I had naively expected. >>> >>> Is that what -Z0 in pg_basebackup should do as well, just output >>> uncompressed tar data, and not add the ".gz" to the "base.tar" file >>> name? >> >> Yes, I think. What about the attached patch? >> > > What if user tries to use -Z 0 with format as tar, won't it generate > base.tar without any compression? I am not sure if that is what Jeff > intends to say in his proposal. My initial proposal was just to change the "usage" message to match reality. I think the current proposal is to make -Z0 be identical to having no -Z specified at all, in other words produce a .tar file, not a .tar.gz file. I had thought we could make a .gz file which didn't actually use compression, just packaged up the data behind a gzip header, but after looking at it I don't think libz actually supports that. Plus, it would be pretty silly to have uncompressed data that would then have to be "uncompressed" merely to unwrap it. It could be useful for testing where you don't want to write for special cases in your shell script (which is where I discovered this, I wanted to test all values between 0 and 9 and see which was fastest given my combination of CPU, network, data and disks), but not useful for practical use. I am happy with the code as currently committed. Cheers, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Fri, Jul 29, 2016 at 11:01 PM, Amit Kapila wrote: > On Thu, Jul 28, 2016 at 7:34 PM, Fujii Masao wrote: >> On Thu, Jul 28, 2016 at 10:16 PM, Amit Kapila >> wrote: >> >>> I think there is some value in providing >>> .tar for -Z 0, >> >> I was thinking that "-Ft -Z0" is something like an alias of "-Ft". >> That is, the backup is taken in uncompressed tar format. >> >>> however in that case how should we define usage of -F p >>> -Z 0? Shall we say with plain format -Z 0 gets ignored or throw error >>> or do something else? If first, then I think it is better to mention >>> the same in docs. >> >> ISTM that it's better to ignore that case, like pg_dump -Ft -Z0 >> doesn't throw an error. >> > > Okay, then you can go ahead with your patch. Thanks for the comment! I pushed the patch. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Thu, Jul 28, 2016 at 7:34 PM, Fujii Masao wrote: > On Thu, Jul 28, 2016 at 10:16 PM, Amit Kapila wrote: > >> I think there is some value in providing >> .tar for -Z 0, > > I was thinking that "-Ft -Z0" is something like an alias of "-Ft". > That is, the backup is taken in uncompressed tar format. > >> however in that case how should we define usage of -F p >> -Z 0? Shall we say with plain format -Z 0 gets ignored or throw error >> or do something else? If first, then I think it is better to mention >> the same in docs. > > ISTM that it's better to ignore that case, like pg_dump -Ft -Z0 > doesn't throw an error. > Okay, then you can go ahead with your patch. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Thu, Jul 28, 2016 at 10:16 PM, Amit Kapila wrote: > On Thu, Jul 28, 2016 at 5:37 PM, Fujii Masao wrote: >> >> Maybe I failed to parse his proposal. It's helpful if you elaborate it. >> > > As per mail [1], it seems the proposal is not to use .tar for -Z 0. I was thinking that the proposal is "output uncompressed tar data, and not add the ".gz" to the "base.tar" file name" part. So, if -Z 0 is specified with tar format, .gz should not be added as a file extension. > Now here actually we are on the fence, one can argue that if user > doesn't want compression, he or she can use -F p (plain format). > OTOH, without compression getting the backup as a single .tar file > makes it simple to manage. Right now we are providing both methods, plain and tar formats (without compression, i.e., neither -z nor -Z options are specified). > I think there is some value in providing > .tar for -Z 0, I was thinking that "-Ft -Z0" is something like an alias of "-Ft". That is, the backup is taken in uncompressed tar format. > however in that case how should we define usage of -F p > -Z 0? Shall we say with plain format -Z 0 gets ignored or throw error > or do something else? If first, then I think it is better to mention > the same in docs. ISTM that it's better to ignore that case, like pg_dump -Ft -Z0 doesn't throw an error. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Thu, Jul 28, 2016 at 5:37 PM, Fujii Masao wrote: > > Maybe I failed to parse his proposal. It's helpful if you elaborate it. > As per mail [1], it seems the proposal is not to use .tar for -Z 0. Now here actually we are on the fence, one can argue that if user doesn't want compression, he or she can use -F p (plain format). OTOH, without compression getting the backup as a single .tar file makes it simple to manage. I think there is some value in providing .tar for -Z 0, however in that case how should we define usage of -F p -Z 0? Shall we say with plain format -Z 0 gets ignored or throw error or do something else? If first, then I think it is better to mention the same in docs. [1] - https://www.postgresql.org/message-id/CAMkU%3D1zzj0et2x9fCqxMGJ6XP-FtMSUwtNQGwF01698FRWQ6uA%40mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Thu, Jul 28, 2016 at 8:44 PM, Amit Kapila wrote: > On Tue, Jul 26, 2016 at 11:58 AM, Fujii Masao wrote: >> On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes wrote: >>> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >>> wrote: On 7/12/16 12:53 PM, Jeff Janes wrote: > The --help message for pg_basebackup says: > > -Z, --compress=0-9 compress tar output with given compression level > > But -Z0 is then rejected as 'invalid compression level "0"'. The real > docs do say 1-9, only the --help message has this bug. Trivial patch > attached. pg_dump --help and man page say it supports 0..9. Maybe we should make that more consistent. >>> >>> pg_dump actually does support -Z0, though. Well, sort of. It outputs >>> plain text. Rather than plain text wrapped in some kind of dummy gzip >>> header, which is what I had naively expected. >>> >>> Is that what -Z0 in pg_basebackup should do as well, just output >>> uncompressed tar data, and not add the ".gz" to the "base.tar" file >>> name? >> >> Yes, I think. What about the attached patch? >> > > What if user tries to use -Z 0 with format as tar, won't it generate > base.tar without any compression? Yes, with -Z 0 -F t options, the patched version of pg_basebackup generate base.tar without compression. > I am not sure if that is what Jeff > intends to say in his proposal. Maybe I failed to parse his proposal. It's helpful if you elaborate it. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Tue, Jul 26, 2016 at 11:58 AM, Fujii Masao wrote: > On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes wrote: >> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >> wrote: >>> On 7/12/16 12:53 PM, Jeff Janes wrote: The --help message for pg_basebackup says: -Z, --compress=0-9 compress tar output with given compression level But -Z0 is then rejected as 'invalid compression level "0"'. The real docs do say 1-9, only the --help message has this bug. Trivial patch attached. >>> >>> pg_dump --help and man page say it supports 0..9. Maybe we should make >>> that more consistent. >> >> pg_dump actually does support -Z0, though. Well, sort of. It outputs >> plain text. Rather than plain text wrapped in some kind of dummy gzip >> header, which is what I had naively expected. >> >> Is that what -Z0 in pg_basebackup should do as well, just output >> uncompressed tar data, and not add the ".gz" to the "base.tar" file >> name? > > Yes, I think. What about the attached patch? > What if user tries to use -Z 0 with format as tar, won't it generate base.tar without any compression? I am not sure if that is what Jeff intends to say in his proposal. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Tue, Jul 26, 2016 at 3:28 PM, Fujii Masao wrote: > On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes wrote: >> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >> wrote: >>> On 7/12/16 12:53 PM, Jeff Janes wrote: The --help message for pg_basebackup says: -Z, --compress=0-9 compress tar output with given compression level But -Z0 is then rejected as 'invalid compression level "0"'. The real docs do say 1-9, only the --help message has this bug. Trivial patch attached. >>> >>> pg_dump --help and man page say it supports 0..9. Maybe we should make >>> that more consistent. >> >> pg_dump actually does support -Z0, though. Well, sort of. It outputs >> plain text. Rather than plain text wrapped in some kind of dummy gzip >> header, which is what I had naively expected. >> >> Is that what -Z0 in pg_basebackup should do as well, just output >> uncompressed tar data, and not add the ".gz" to the "base.tar" file >> name? > > Yes, I think. What about the attached patch? Barring any objection, I will commit this patch. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes wrote: > On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut > wrote: >> On 7/12/16 12:53 PM, Jeff Janes wrote: >>> The --help message for pg_basebackup says: >>> >>> -Z, --compress=0-9 compress tar output with given compression level >>> >>> But -Z0 is then rejected as 'invalid compression level "0"'. The real >>> docs do say 1-9, only the --help message has this bug. Trivial patch >>> attached. >> >> pg_dump --help and man page say it supports 0..9. Maybe we should make >> that more consistent. > > pg_dump actually does support -Z0, though. Well, sort of. It outputs > plain text. Rather than plain text wrapped in some kind of dummy gzip > header, which is what I had naively expected. > > Is that what -Z0 in pg_basebackup should do as well, just output > uncompressed tar data, and not add the ".gz" to the "base.tar" file > name? Yes, I think. What about the attached patch? Regards, -- Fujii Masao *** a/doc/src/sgml/ref/pg_basebackup.sgml --- b/doc/src/sgml/ref/pg_basebackup.sgml *** *** 364,370 PostgreSQL documentation Enables gzip compression of tar file output, and specifies the ! compression level (1 through 9, 9 being best compression). Compression is only available when using the tar format. --- 364,370 Enables gzip compression of tar file output, and specifies the ! compression level (0 through 9, 0 being no compression and 9 being best compression). Compression is only available when using the tar format. *** a/src/bin/pg_basebackup/pg_basebackup.c --- b/src/bin/pg_basebackup/pg_basebackup.c *** *** 2073,2079 main(int argc, char **argv) break; case 'Z': compresslevel = atoi(optarg); ! if (compresslevel <= 0 || compresslevel > 9) { fprintf(stderr, _("%s: invalid compression level \"%s\"\n"), progname, optarg); --- 2073,2079 break; case 'Z': compresslevel = atoi(optarg); ! if (compresslevel < 0 || compresslevel > 9) { fprintf(stderr, _("%s: invalid compression level \"%s\"\n"), progname, optarg); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Tue, Jul 12, 2016 at 10:23 PM, Jeff Janes wrote: > I've been having some adventures with pg_basebackup lately, and had > some suggestions based on those. > > The --help message for pg_basebackup says: > > -Z, --compress=0-9 compress tar output with given compression level > > But -Z0 is then rejected as 'invalid compression level "0"'. The real > docs do say 1-9, only the --help message has this bug. Trivial patch > attached. > > These ones I have not written code for yet: > > The progress reporting for pg_basebackup is pretty terse: > > 858117/7060099 kB (12%), 0/1 tablespace > > I think we should at least add a count-up timer showing the seconds it > has been running. I can always use my own stopwatch, but that is not > very friendly and easy to forget to start. > Another possibility is to enhance -P option as -P sec, such that it will display progress after ever 'sec' seconds. Something like we have for pgbench. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Wed, Jul 13, 2016 at 1:53 AM, Jeff Janes wrote: > I've been having some adventures with pg_basebackup lately, and had > some suggestions based on those. And what I think is pg_baseback never remove the directory specified by -D option even if execution is failed. initdb command behaves so. I think it's helpful for backup operation. Regards, Masahiko Sawada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Tue, Jul 12, 2016 at 11:06:39AM -0700, Jeff Janes wrote: > On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut > wrote: > > On 7/12/16 12:53 PM, Jeff Janes wrote: > >> The --help message for pg_basebackup says: > >> > >> -Z, --compress=0-9 compress tar output with given compression level > >> > >> But -Z0 is then rejected as 'invalid compression level "0"'. The real > >> docs do say 1-9, only the --help message has this bug. Trivial patch > >> attached. > > > > pg_dump --help and man page say it supports 0..9. Maybe we should make > > that more consistent. > > pg_dump actually does support -Z0, though. Well, sort of. It outputs > plain text. Rather than plain text wrapped in some kind of dummy gzip > header, which is what I had naively expected. > > Is that what -Z0 in pg_basebackup should do as well, just output > uncompressed tar data, and not add the ".gz" to the "base.tar" file > name? > > Cheers, > > Jeff > Hi, Yes, please support the no compression option. It can be useful in situations where the bottleneck is the compression itself (quite easily done with zlib based options, another plug for a higher performance option). Regards, Ken -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut wrote: > On 7/12/16 12:53 PM, Jeff Janes wrote: >> The --help message for pg_basebackup says: >> >> -Z, --compress=0-9 compress tar output with given compression level >> >> But -Z0 is then rejected as 'invalid compression level "0"'. The real >> docs do say 1-9, only the --help message has this bug. Trivial patch >> attached. > > pg_dump --help and man page say it supports 0..9. Maybe we should make > that more consistent. pg_dump actually does support -Z0, though. Well, sort of. It outputs plain text. Rather than plain text wrapped in some kind of dummy gzip header, which is what I had naively expected. Is that what -Z0 in pg_basebackup should do as well, just output uncompressed tar data, and not add the ".gz" to the "base.tar" file name? Cheers, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup wish list
On 7/12/16 12:53 PM, Jeff Janes wrote: > The --help message for pg_basebackup says: > > -Z, --compress=0-9 compress tar output with given compression level > > But -Z0 is then rejected as 'invalid compression level "0"'. The real > docs do say 1-9, only the --help message has this bug. Trivial patch > attached. pg_dump --help and man page say it supports 0..9. Maybe we should make that more consistent. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_basebackup wish list
I've been having some adventures with pg_basebackup lately, and had some suggestions based on those. The --help message for pg_basebackup says: -Z, --compress=0-9 compress tar output with given compression level But -Z0 is then rejected as 'invalid compression level "0"'. The real docs do say 1-9, only the --help message has this bug. Trivial patch attached. These ones I have not written code for yet: The progress reporting for pg_basebackup is pretty terse: 858117/7060099 kB (12%), 0/1 tablespace I think we should at least add a count-up timer showing the seconds it has been running. I can always use my own stopwatch, but that is not very friendly and easy to forget to start. And maybe change the reporting units from kB to MB when the pre-scan says the total size exceeds some threshold? At the same limit pg_size_pretty does? If I use the verbose flag, then the progress message includes the name of the file being written to by the client. However, in -Ft mode, this is always "something/base.tar" (unless you are using tablespaces), which is not terribly useful. Should it instead report the name of the file being read on the server end? When using pg_basebackup from the wrong version, the error message it reports is pretty unhelpful: pg_basebackup: could not initiate base backup: ERROR: syntax error Could we have a newer version of pg_basebackup capture that error and inject a HINT, or is there a better solution for getting a better error message? Cheers, Jeff pg_basebackup_compress.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers