Thread: 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
Attachment
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
On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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
On Tue, Jul 12, 2016 at 11:06:39AM -0700, Jeff Janes wrote: > On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> 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
On Wed, Jul 13, 2016 at 1:53 AM, Jeff Janes <jeff.janes@gmail.com> 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
On Tue, Jul 12, 2016 at 10:23 PM, Jeff Janes <jeff.janes@gmail.com> 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
On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes <jeff.janes@gmail.com> wrote: > On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> 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
Attachment
On Tue, Jul 26, 2016 at 3:28 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes <jeff.janes@gmail.com> wrote: >> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >> <peter.eisentraut@2ndquadrant.com> 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
On Tue, Jul 26, 2016 at 11:58 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes <jeff.janes@gmail.com> wrote: >> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >> <peter.eisentraut@2ndquadrant.com> 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
On Thu, Jul 28, 2016 at 8:44 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Tue, Jul 26, 2016 at 11:58 AM, Fujii Masao <masao.fujii@gmail.com> wrote: >> On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes <jeff.janes@gmail.com> wrote: >>> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >>> <peter.eisentraut@2ndquadrant.com> 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
On Thu, Jul 28, 2016 at 5:37 PM, Fujii Masao <masao.fujii@gmail.com> 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
On Thu, Jul 28, 2016 at 10:16 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Thu, Jul 28, 2016 at 5:37 PM, Fujii Masao <masao.fujii@gmail.com> 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
On Thu, Jul 28, 2016 at 7:34 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Thu, Jul 28, 2016 at 10:16 PM, Amit Kapila <amit.kapila16@gmail.com> 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
On Fri, Jul 29, 2016 at 11:01 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Thu, Jul 28, 2016 at 7:34 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >> On Thu, Jul 28, 2016 at 10:16 PM, Amit Kapila <amit.kapila16@gmail.com> 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
On Thu, Jul 28, 2016 at 4:44 AM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Tue, Jul 26, 2016 at 11:58 AM, Fujii Masao <masao.fujii@gmail.com> wrote: >> On Wed, Jul 13, 2016 at 3:06 AM, Jeff Janes <jeff.janes@gmail.com> wrote: >>> On Tue, Jul 12, 2016 at 10:48 AM, Peter Eisentraut >>> <peter.eisentraut@2ndquadrant.com> 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
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
Attachment
On Fri, Aug 19, 2016 at 7:06 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> 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
showprogress information\n"));
Registered this patch to CF1.
Regards,
--
Masahiko Sawada
On Fri, Aug 19, 2016 at 2:04 PM, Masahiko Sawada <sawada.mshk@gmail.com> 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
On 19 August 2016 at 08:46, Michael Paquier <michael.paquier@gmail.com> wrote:
> On Fri, Aug 19, 2016 at 2:04 PM, Masahiko Sawada <sawada.mshk@gmail.com> 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 Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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
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
On Wed, Sep 14, 2016 at 6:52 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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
Attachment
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
On Thu, Sep 15, 2016 at 9:26 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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