backup.sgml-cmd-v003.patch

Lists: pgsql-hackers
From: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: backup.sgml-cmd-v003.patch
Date: 2013-07-31 17:08:12
Message-ID: B1DC07C6-38FE-4B07-8246-245C34697403@commandprompt.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hello,

I'd like to properly resubmit this patch, under a new version now. Somehow this effort fell by the wayside.

Project: postgresql
Patch filename: backup.sgml-cmd-v003.patch

The third version of this patch takes into consideration feedback received after original submission (it can be read starting from this message http://www.postgresql.org/message-id/CA+Tgmoaq-9D_mst113TdW=Ar8mpgBc+x6T61AzK3eMhww9gRcQ@mail.gmail.com)

Essentially, it addresses the points that were raised in community feedback and offers better worded statements that avoid implying that some features are being deprecated when it isn't the case. We also spent some more time polishing other details, like making adjustments to the tone of the text so that it sounds more like a manual, and less like a blog post. More importantly, this chapter now makes it clear that superuser privileges are not always required to perform a successful backup because in practice as long as the role used to make a backup has sufficient read privileges on all of the objects a user is interested in it's going to work just fine. We also mention and show examples of usage for pg_restore and pigz alongside with gzip, and probably something else too.

Please, see the patch file (git diff) attached to this message for complete and detailed log of the changes.
An html version of the document generated from this patch is included as a preview for your convenience also.

It is meant for application, and is against master branch.

The patch does pass 'make check' and 'make html' successfully.

Ivan

Attachment Content-Type Size
backup.sgml-cmd-v003.patch.zip application/zip 8.6 KB

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-08-01 00:14:24
Message-ID: CAB7nPqSu8n=RrV3UfAZTtrfavb-iiY5Cc+CsRhX_H5rD9cxZWQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi,

Could you add this documentation patch to the next commit fest such as it
doesn't get lost in the stack?
https://commitfest.postgresql.org/action/commitfest_view?id=19

On Thu, Aug 1, 2013 at 2:08 AM, Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>wrote:

> The patch does pass 'make check' and 'make html' successfully.
>
Your patch does not add new code, but just documentation, so there is no
risk that make check would fail, except if an error has been introduced by
another commit at some point...

Thanks,
--
Michael


From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-08-01 00:39:31
Message-ID: 20130801003931.GD14652@eldon.alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Michael Paquier escribió:

> On Thu, Aug 1, 2013 at 2:08 AM, Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>wrote:
>
> > The patch does pass 'make check' and 'make html' successfully.
> >
> Your patch does not add new code, but just documentation, so there is no
> risk that make check would fail, except if an error has been introduced by
> another commit at some point...

There's "make check" in the doc/src/sgml directory. It checks for SGML
consistency, absence of tabs and such.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-08-01 03:09:33
Message-ID: CAB7nPqSzxfShgAuwrmc5p84FP68gZxTCqZVqZHnq12Cc+UhR-A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Aug 1, 2013 at 9:39 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>wrote:

> Michael Paquier escribió:
>
> > On Thu, Aug 1, 2013 at 2:08 AM, Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com
> >wrote:
> >
> > > The patch does pass 'make check' and 'make html' successfully.
> > >
> > Your patch does not add new code, but just documentation, so there is no
> > risk that make check would fail, except if an error has been introduced
> by
> > another commit at some point...
>
> There's "make check" in the doc/src/sgml directory. It checks for SGML
> consistency, absence of tabs and such.
>
Oh thanks. Sorry for my mistake. This is good to know.
--
Michael


From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-09-03 03:56:54
Message-ID: 1378180614.3137.1@slate
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 07/31/2013 12:08:12 PM, Ivan Lezhnjov IV wrote:

> Patch filename: backup.sgml-cmd-v003.patch
>
> The third version of this patch takes into consideration feedback
> received after original submission (it can be read starting from this
> message http://www.postgresql.org/message-id/CA
> +Tgmoaq-9D_mst113TdW=Ar8mpgBc+x6T61AzK3eMhww9gRcQ(at)mail(dot)gmail(dot)com)
>
> Essentially, it addresses the points that were raised in community
> feedback and offers better worded statements that avoid implying that
> some features are being deprecated when it isn't the case. We also
> spent some more time polishing other details, like making adjustments
> to the tone of the text so that it sounds more like a manual, and
> less
> like a blog post. More importantly, this chapter now makes it clear
> that superuser privileges are not always required to perform a
> successful backup because in practice as long as the role used to
> make
> a backup has sufficient read privileges on all of the objects a user
> is interested in it's going to work just fine. We also mention and
> show examples of usage for pg_restore and pigz alongside with gzip,
> and probably something else too.

Hi Ivan,

I'm reviewing your patch. I did not read the entirety of the
thread referenced above. I apologize if this causes problems.

Attached is backup-sgml-cmnd-v003_1.patch to be applied on top of
backup-sgml-cmnd-v003.patch and containing my edits. You will
eventually want to produce a single patch (but see below).
Meanwhile this might help you keep track of my changes.

Attached also is your original v3 patch.

---

Cleaned up and clarified here and there.

The bit about OIDs being depreciated might properly belong in
a separate patch. The same might be said about adding mention of pigz.
If you submit these as separate patch file attachments
they can always be applied in a single commit, but the reverse is
more work for the committer. (Regardless, I see no reason to
have separate commitfest entries or anything other than multiple
attachments to the email that finalizes our discussion.)

Minimally modified to note the existence of directory dumps. It may
be that the utility/flexibility of directory dumps should also be
mentioned.

My thought is that the part beginning with "The options in detail
are:" should not describe all the possibilities for the --format
option, that being better left to the reference section. Likewise,
this being prose, it might be best to describe all the options
in-line, instead of presented as a list. I have left it as-is
for you to improve as seen fit.

I have frobbed your <programlisting> to adjust the indentation and
line-wrap style. I submit it here for consideration in case this
style is attractive. This is nothing but conceit. We should use the
same style used elsewhere in the documentation. (I can't think
offhand of a place to look for a line-wrapped shell example. If you
can't find one I'll take a look and if neither of us finds one we'll
then have choices.)

I don't know that it's necessary to include pigz examples, because it
sounds like pigz is a drop-in gzip replacement. I've left your
examples in, in case you feel they are necessary.

The existing text of the SQL Dump section could use some alteration to
reduce redundancy and add clarity. I'm thinking specifically of
mention of pg_restore as being required to restore custom format
backups and of the default pg_dump output being not just "plain text"
but being a collection of SQL commands. Yes, the latter is obvious
upon reflection, psql being what it is, but I think it would be
helpful to spell this out. Especially in the context of the current
patch. There could well be other areas like this to be addressed.

Overall I have pretty good feelings about this patch. Backup and
restore is complex and although this patch leaves a lot of issues
unmentioned it seems a step in the right direction.

Regards,

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Attachment Content-Type Size
backup-sgml-cmd-v003_1.patch text/x-patch 7.6 KB
backup.sgml-cmd-v003.patch.zip application/zip 8.6 KB

From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-09-03 04:10:31
Message-ID: 1378181431.3137.2@slate
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 09/02/2013 10:56:54 PM, Karl O. Pinc wrote:
> I have frobbed your <programlisting> to adjust the indentation and
> line-wrap style.

Oops. Somehow left a \ out of this. Anyhow, you get the idea.

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein


From: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
To: Karl O(dot) Pinc <kop(at)meme(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-09-26 17:15:25
Message-ID: 705B639D-76C2-4F9B-BEF8-30E97EB371BB@commandprompt.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On Sep 3, 2013, at 6:56 AM, Karl O. Pinc <kop(at)meme(dot)com> wrote:

> On 07/31/2013 12:08:12 PM, Ivan Lezhnjov IV wrote:
>
>> Patch filename: backup.sgml-cmd-v003.patch
>>
>> The third version of this patch takes into consideration feedback
>> received after original submission (it can be read starting from this
>> message http://www.postgresql.org/message-id/CA
>> +Tgmoaq-9D_mst113TdW=Ar8mpgBc+x6T61AzK3eMhww9gRcQ(at)mail(dot)gmail(dot)com)
>>
>> Essentially, it addresses the points that were raised in community
>> feedback and offers better worded statements that avoid implying that
>> some features are being deprecated when it isn't the case. We also
>> spent some more time polishing other details, like making adjustments
>> to the tone of the text so that it sounds more like a manual, and
>> less
>> like a blog post. More importantly, this chapter now makes it clear
>> that superuser privileges are not always required to perform a
>> successful backup because in practice as long as the role used to
>> make
>> a backup has sufficient read privileges on all of the objects a user
>> is interested in it's going to work just fine. We also mention and
>> show examples of usage for pg_restore and pigz alongside with gzip,
>> and probably something else too.
>
> Hi Ivan,
>
> I'm reviewing your patch. I did not read the entirety of the
> thread referenced above. I apologize if this causes problems.
>
> Attached is backup-sgml-cmnd-v003_1.patch to be applied on top of
> backup-sgml-cmnd-v003.patch and containing my edits. You will
> eventually want to produce a single patch (but see below).
> Meanwhile this might help you keep track of my changes.
>
> Attached also is your original v3 patch.
>
> ---
>
> Cleaned up and clarified here and there.
>
> The bit about OIDs being depreciated might properly belong in
> a separate patch. The same might be said about adding mention of pigz.
> If you submit these as separate patch file attachments
> they can always be applied in a single commit, but the reverse is
> more work for the committer. (Regardless, I see no reason to
> have separate commitfest entries or anything other than multiple
> attachments to the email that finalizes our discussion.)

Hello,

took me a while to get here, but a lot has been going on...

Okay, I'm new and I don't know why a single patch like this is more work for a commiter? Just so I understand and know.

>
> Minimally modified to note the existence of directory dumps. It may
> be that the utility/flexibility of directory dumps should also be
> mentioned.
>
> My thought is that the part beginning with "The options in detail
> are:" should not describe all the possibilities for the --format
> option, that being better left to the reference section. Likewise,
> this being prose, it might be best to describe all the options
> in-line, instead of presented as a list. I have left it as-is
> for you to improve as seen fit.

Agreed, it probably looks better as a sentence.

>
> I have frobbed your <programlisting> to adjust the indentation and
> line-wrap style. I submit it here for consideration in case this
> style is attractive. This is nothing but conceit. We should use the
> same style used elsewhere in the documentation. (I can't think
> offhand of a place to look for a line-wrapped shell example. If you
> can't find one I'll take a look and if neither of us finds one we'll
> then have choices.)

Looks good to me.

>
> I don't know that it's necessary to include pigz examples, because it
> sounds like pigz is a drop-in gzip replacement. I've left your
> examples in, in case you feel they are necessary.

We do. We believe it can encourage more people to consider using it. The way we see it, most people seem to be running mutlicore systems these days, yet many simply are not aware of pigz. We have been routinely informing our customers of pigz as an alternative to tried and tested gzip when helping optimize their configurations, and all of them without exception went for it. I guess everybody just likes to squeeze some extra juice for free.

>
> The existing text of the SQL Dump section could use some alteration to
> reduce redundancy and add clarity. I'm thinking specifically of
> mention of pg_restore as being required to restore custom format
> backups and of the default pg_dump output being not just "plain text"
> but being a collection of SQL commands. Yes, the latter is obvious
> upon reflection, psql being what it is, but I think it would be
> helpful to spell this out. Especially in the context of the current
> patch. There could well be other areas like this to be addressed.

I don't quite follow you here. I mean, I kinda understand what you mean in general, but when I look at the text I fail to see what you had in mind specifically.

For example, pg_restore is mentioned only 3 times in section 24.1. Each mention seems pretty essential to me. And the text flow is pretty natural.

Also, about plain text format being a collection of SQL commands. The very first paragraph of the section 24.1 reads "The idea behind this dump method is to generate a text file with SQL commands that, when fed back to the server, will recreate the database in the same state as it was at the time of the dump. PostgreSQL provides the utility program pg_dump for this purpose."

>
> Overall I have pretty good feelings about this patch. Backup and
> restore is complex and although this patch leaves a lot of issues
> unmentioned it seems a step in the right direction.
>

Thanks for a detailed response. I attached a patch file that builds on your corrections and introduces some of the edits discussed above.

Ivan

Attachment Content-Type Size
backup.sgml-cmd-v003_2.patch application/octet-stream 8.3 KB
unknown_filename text/plain 2 bytes

From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-09-27 03:27:04
Message-ID: 1380252424.10803.4@slate
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 09/26/2013 12:15:25 PM, Ivan Lezhnjov IV wrote:
>
> On Sep 3, 2013, at 6:56 AM, Karl O. Pinc <kop(at)meme(dot)com> wrote:
>
> > On 07/31/2013 12:08:12 PM, Ivan Lezhnjov IV wrote:
> >
> >> Patch filename: backup.sgml-cmd-v003.patch
> >>
> >> The third version of this patch takes into consideration feedback
> >> received after original submission (it can be read starting from
> this
> >> message http://www.postgresql.org/message-id/CA
> >> +Tgmoaq-9D_mst113TdW=Ar8mpgBc+x6T61AzK3eMhww9gRcQ(at)mail(dot)gmail(dot)com)
> >>
> >> Essentially, it addresses the points that were raised in community
> >> feedback and offers better worded statements that avoid implying
> that
> >> some features are being deprecated when it isn't the case. We also
> >> spent some more time polishing other details, like making
> adjustments
> >> to the tone of the text so that it sounds more like a manual, and
> >> less
> >> like a blog post. More importantly, this chapter now makes it
> clear
> >> that superuser privileges are not always required to perform a
> >> successful backup because in practice as long as the role used to
> >> make
> >> a backup has sufficient read privileges on all of the objects a
> user
> >> is interested in it's going to work just fine. We also mention and
> >> show examples of usage for pg_restore and pigz alongside with
> gzip,
> >> and probably something else too.

> > ---
> >
> > Cleaned up and clarified here and there.
> >
> > The bit about OIDs being depreciated might properly belong in
> > a separate patch. The same might be said about adding mention of
> pigz.
> > If you submit these as separate patch file attachments
> > they can always be applied in a single commit, but the reverse is
> > more work for the committer. (Regardless, I see no reason to
> > have separate commitfest entries or anything other than multiple
> > attachments to the email that finalizes our discussion.)
>
> Hello,
>
> took me a while to get here, but a lot has been going on...

No worries.

> Okay, I'm new and I don't know why a single patch like this is more
> work for a commiter? Just so I understand and know.

Different committers have different preferences but the general rule
is that it's work to split a patch into pieces if you don't like the
whole thing but it's easy to apply a bunch of small patches and
commit them all at once. Further each commit should represent
a single "feature" or conceptual change. Again, preferences vary
but I like to think that a good rule is that 1 commit should
be able to be described in a sentence, and not a run-on sentence
either that says I did this and I also did that and something else.
So if there's a question in your mind about whether a committer
will want your entire change, or if your patch changes unrelated
things then it does not hurt to submit it as separate patches.
All the patches can be attached to a single email and part of
a single commitfest tracking entry, usually. No need to get
crazy. These are just things to think about.

In your case I see 3 things happening:

oid depreciation

custom format explanation

pigz promotion

> > My thought is that the part beginning with "The options in detail
> > are:" should not describe all the possibilities for the --format
> > option, that being better left to the reference section. Likewise,
> > this being prose, it might be best to describe all the options
> > in-line, instead of presented as a list. I have left it as-is
> > for you to improve as seen fit.
>
> Agreed, it probably looks better as a sentence.

Looks good.

>
> >
> > I have frobbed your <programlisting> to adjust the indentation and
> > line-wrap style. I submit it here for consideration in case this
> > style is attractive. This is nothing but conceit. We should use
> the
> > same style used elsewhere in the documentation.

> Looks good to me.

I fixed the missing \ I messed up on last time
and slightly re-worded the previous sentence.

I've grep-ed through the sgml looking for multi-line shell scripts
and found only 1 (in sepgsql.sgm). I don't see a conflict with
the formatting/line-break convention used in the patch, although
it does differ slightly in indentation. I'm leaving the shell
script formatting in the patch as-is for the committer to judge.
(I like the way it looks but it is not a traditional style.)

>
> >
> > I don't know that it's necessary to include pigz examples, because
> it
> > sounds like pigz is a drop-in gzip replacement. I've left your
> > examples in, in case you feel they are necessary.
>
> We do. We believe it can encourage more people to consider using it.
> The way we see it, most people seem to be running mutlicore systems
> these days, yet many simply are not aware of pigz.

Ok. It's your patch.

> >
> > The existing text of the SQL Dump section could use some alteration
> to
> > reduce redundancy and add clarity. I'm thinking specifically of
> > mention of pg_restore as being required to restore custom format
> > backups and of the default pg_dump output being not just "plain
> text"
> > but being a collection of SQL commands. Yes, the latter is obvious
> > upon reflection, psql being what it is, but I think it would be
> > helpful to spell this out. Especially in the context of the
> current
> > patch. There could well be other areas like this to be addressed.
>
> I don't quite follow you here. I mean, I kinda understand what you
> mean in general, but when I look at the text I fail to see what you
> had in mind specifically.

Specifically, I was waving my hands about in a general fashion. ;-)

> For example, pg_restore is mentioned only 3 times in section 24.1.
> Each mention seems pretty essential to me. And the text flow is
> pretty
> natural.

Ok.

>
> Also, about plain text format being a collection of SQL commands. The
> very first paragraph of the section 24.1 reads "The idea behind this
> dump method is to generate a text file with SQL commands that, when
> fed back to the server, will recreate the database in the same state
> as it was at the time of the dump. PostgreSQL provides the utility
> program pg_dump for this purpose."

I have added/changed some text at the first use of "plain text"
to be more explicit. (Usually I would use <wordasword>
markup but the pg docs always seem to use <quote> instead,
which is fine so I've done that.)

I also removed some duplicate-ish text towards the end of the
section regards the nature of custom-format dumps in the
context of restoring same with pg_restore.

> Thanks for a detailed response. I attached a patch file that builds
> on
> your corrections and introduces some of the edits discussed above.

Upon reflection it bothered me to have text in the dump section
talking about restore. I've moved this into the restore
section and tweaked the text leading into the paste.
This seems to work well but feel free to disagree.

I've one more thought; whether the business with
confusing --file with <filename> is really important enough
to be marked <important> and included
in-line. In general, too many of these
out of band comments detract from the flow of the document.
I've left the markup as-is, but you might consider changing
this into a <footnote>. (You'll need to move it into the
back of the preceding <para>.) The document currently contains very
few footnotes but this might be an appropriate case.
(I've not looked at the other footnotes to see what they
contain.)

I also see inconsistent use of the SQL term.
In some cases it's marked up with <acronym> and in
some cases not. I am ignoring the issue.

Attached is a patch for your review (applies against HEAD):
backup.sgml-cmd-v003_3.patch

If you like it
let me know and I'll get it sent to the committers
or if you want to make some more changes based on
comments above/break it into 3 patches/whatever
get those back to me.

Regards,

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Attachment Content-Type Size
backup.sgml-cmd-v003_3.patch text/x-patch 9.0 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
Cc: "Karl O(dot) Pinc" <kop(at)meme(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-09-27 10:56:52
Message-ID: CA+TgmoYQXWhN5JgP+MM3MUwH+2gKR5AW5afWZP+3go4r-yae5w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Sep 26, 2013 at 1:15 PM, Ivan Lezhnjov IV
<iliv(at)commandprompt(dot)com> wrote:
> Thanks for a detailed response. I attached a patch file that builds on your corrections and introduces some of the edits discussed above.

This patch makes at least five unrelated sets of changes:

1. Attempting to encourage people to consider custom format dumps.
2. Suggesting that superuser access isn't necessary to dump the database.
3. Adding a note that OIDs on user objects are deprecated.
4. Describing the manner in which pg_dump can be used with pg_dumpall
to back up all databases in a cluster.
5. Promoting pigz.

It's not a good idea to bundle so many unrelated changes together into
a single patch, because if there is disagreement about any item on the
list, then the patch doesn't go in. Moreover, we typically try to
commit one change at a time, so this leaves the eventual committer
with the unenviable task of splitting up the patch for commit.

I think that #3 and #5 have no business are things we shouldn't do at
all. There are many compression utilities in the world other than
gzip, and everyone has their favorites. I see no reason why we should
promote one over the other. Certainly, the fact that you yourself
have found pigz useful is not sufficient evidence that everyone should
prefer it. And, while it's true that OIDs on user objects are
deprecated, I don't think the pg_dump documentation necessarily needs
to recapitulate this point particularly; hopefully, it's documented
elsewhere; if not, this isn't the right place to mention it. If,
contrary to my feelings on the matter, we do decide to make a change
in either of these areas, it's unrelated to the rest of this patch and
needs to be submitted and discussed separately.

I think that #2 is probably a worthwhile change in some form.
However, I don't particularly agree with the way you've worded it
here. This section is trying to give a general overview of how to
back up your whole database; it's not the pg_dump reference page,
which exists elsewhere. I would suggest that the relevant caveat here
is not so much that you might have a non-superuser account that just
happens to have enough privileges to back up the entire database, but
rather that we shouldn't give people the impression that the only
possible use of pg_dump is as for a full backup. Because in my
experience, a full-database backup almost always DOES require
superuser privileges; it's the selective-dump case where you can often
get by without them. I've attached a proposed patch along these lines
for your consideration.

The remaining changes (#1 and #4) can probably be blended together in
a single patch. However, I think that you need to make more of an
effort to use neutral wording. The purpose of the documentation is
not to pass judgement on the tools. Examples:

+ The above example creates a plain text file. This type of dump can be used
+ to restore a database in full. However there are more sophisticated
+ <productname>PostgreSQL</> backup formats which allow for far greater
+ control when working with backups. One of these is
+ the <quote>custom</quote> format, which the following more elaborate
+ example creates:

I don't find it helpful to classify the other formats as more
sophisticated or to say that they allow far greater control, or to
call the example elaborate. What's important is what you can do, and
there's basically one thing: selective restores. So I think you could
replace all that, the following example, and the paragraph after that
with: The above example creates a plain text file. pg_dump can also
create backups in other formats, such as the custom format. This may
be advantageous, since all formats other than the plain text file
format make it possible to selectively restore objects from the dump.
See <xref linkend="app-pgrestore"> for more details.

+ <para>
+ Unfortunately, <application>pg_dumpall</> can only create plaintext
+ backups. However, it is currently the only way to backup the globals in you

Similarly, let's not label pg_dumpall's behavior as unfortunate. I
think it's appropriate to explain the pg_dumpall -g / pg_dump -Fc
combination somewhere in the documentation, as it is widely used and
very useful. But I don't think it's useful to imply to users that the
tools are inadequate; generally, they're not, improvements that we'd
like to make nonwithstanding.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
backup-without-superuser.sgml text/sgml 1.1 KB

From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-09-27 13:49:41
Message-ID: 1380289781.12507.0@slate
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi Robert,

On 09/27/2013 05:56:52 AM, Robert Haas wrote:

> 1. Attempting to encourage people to consider custom format dumps.

> What's important is what you can do...

Your critique seems obvious in retrospect. Sorry you had
to step in here and do my job. The above point is particularly
salient. I will try to keep it in mind in the future.

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
Cc: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-09-27 18:15:20
Message-ID: CA+Tgmobyrcqy21ArT5NfvJ-csih0X20gd_fcaE1b1paAPurs6A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Sep 27, 2013 at 9:49 AM, Karl O. Pinc <kop(at)meme(dot)com> wrote:
> Hi Robert,
> On 09/27/2013 05:56:52 AM, Robert Haas wrote:
>
>> 1. Attempting to encourage people to consider custom format dumps.
>
>> What's important is what you can do...
>
> Your critique seems obvious in retrospect. Sorry you had
> to step in here and do my job. The above point is particularly
> salient. I will try to keep it in mind in the future.

Hey, your reviewing is much appreciated. I actually didn't notice
your reply before I sent mine; I started writing it yesterday
afternoon and finished this morning, so I wasn't intending to preempt
you in any way.

Thanks,

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


From: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>
Cc: "Karl O(dot) Pinc" <kop(at)meme(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-11-07 16:37:54
Message-ID: 527BC1E2.1050601@commandprompt.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 09/27/2013 03:56 AM, Robert Haas wrote:
> On Thu, Sep 26, 2013 at 1:15 PM, Ivan Lezhnjov IV
> <iliv(at)commandprompt(dot)com> wrote:
>> Thanks for a detailed response. I attached a patch file that builds on your corrections and introduces some of the edits discussed above.
>
> This patch makes at least five unrelated sets of changes:
>
> 1. Attempting to encourage people to consider custom format dumps.
> 2. Suggesting that superuser access isn't necessary to dump the database.
> 3. Adding a note that OIDs on user objects are deprecated.
> 4. Describing the manner in which pg_dump can be used with pg_dumpall
> to back up all databases in a cluster.
> 5. Promoting pigz.
>
> It's not a good idea to bundle so many unrelated changes together into
> a single patch,

This isn't software, it is docs. It is ridiculous to suggest we break
this up into 3-4 patches. This is a small doc patch to a single doc file
(backup.sgml).

>
> I think that #3 and #5 have no business are things we shouldn't do at
> all. There are many compression utilities in the world other than
> gzip, and everyone has their favorites. I see no reason why we should
> promote one over the other.

Then a rewording that states that multi-threaded compression is
available perhaps and list a couple? Because pigz over gzip is a no
brainer but there are also the equiv for bzip2 etc...

> Certainly, the fact that you yourself
> have found pigz useful is not sufficient evidence that everyone should
> prefer it.

Smart people would :P At least over gzip. If you have more than one core
it is one of those... duh things.

> And, while it's true that OIDs on user objects are
> deprecated, I don't think the pg_dump documentation necessarily needs
> to recapitulate this point particularly; hopefully, it's documented
> elsewhere; if not, this isn't the right place to mention it.

Fair enough.

> If,
> contrary to my feelings on the matter, we do decide to make a change
> in either of these areas, it's unrelated to the rest of this patch and
> needs to be submitted and discussed separately.
>
> I think that #2 is probably a worthwhile change in some form.
> However, I don't particularly agree with the way you've worded it
> here. This section is trying to give a general overview of how to
> back up your whole database; it's not the pg_dump reference page,

[..]

> superuser privileges; it's the selective-dump case where you can often
> get by without them. I've attached a proposed patch along these lines
> for your consideration.

That's fair.

>
> The remaining changes (#1 and #4) can probably be blended together in
> a single patch. However, I think that you need to make more of an
> effort to use neutral wording. The purpose of the documentation is
> not to pass judgement on the tools. Examples:
>
> + The above example creates a plain text file. This type of dump can be used
> + to restore a database in full. However there are more sophisticated
> + <productname>PostgreSQL</> backup formats which allow for far greater
> + control when working with backups. One of these is
> + the <quote>custom</quote> format, which the following more elaborate
> + example creates:
>
> I don't find it helpful to classify the other formats as more
> sophisticated or to say that they allow far greater control, or to
> call the example elaborate. What's important is what you can do, and
> there's basically one thing: selective restores. So I think you could
> replace all that, the following example, and the paragraph after that
> with: The above example creates a plain text file. pg_dump can also
> create backups in other formats, such as the custom format. This may
> be advantageous, since all formats other than the plain text file
> format make it possible to selectively restore objects from the dump.
> See <xref linkend="app-pgrestore"> for more details.

I can buy into that but I don't think it gives the other formats
justice. Plain text format is rather useless in the sense of a backup.

>
> + <para>
> + Unfortunately, <application>pg_dumpall</> can only create plaintext
> + backups. However, it is currently the only way to backup the globals in you
>
> Similarly, let's not label pg_dumpall's behavior as unfortunate.

Uh, pg_dumpall's behavior is crap. Unfortunate was being polite. Alas
you are correct, in documentation we should be neutral. I agree, let's
remove the word Unfortunately.

> I
> think it's appropriate to explain the pg_dumpall -g / pg_dump -Fc
> combination somewhere in the documentation, as it is widely used and
> very useful. But I don't think it's useful to imply to users that the
> tools are inadequate; generally, they're not, improvements that we'd
> like to make nonwithstanding.

Generally speaking I would agree with you but in this case I do not. The
way we have to back up things, including things that aren't backed up
nor documented (alter database?), are inadequate.

That said, that is an argument for another time. Outside of what I have
said I agree.

Joshua D. Drake

P.S. Sorry for the late reply. I got on Iliv to wrap up this patch and
he asked me if we want to put that much effort into it. I thought I
would respond before we decide.

--
Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc
For my dreams of your image that blossoms
a rose in the deeps of my heart. - W.B. Yeats


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
Cc: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "Karl O(dot) Pinc" <kop(at)meme(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-11-08 14:04:54
Message-ID: CA+TgmoYL5b9Ae3X5vTtoz=d=nOfnrF4eNdJvmkM3Bx=ydsAZhw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Nov 7, 2013 at 11:37 AM, Joshua D. Drake <jd(at)commandprompt(dot)com> wrote:
> This isn't software, it is docs. It is ridiculous to suggest we break this
> up into 3-4 patches. This is a small doc patch to a single doc file
> (backup.sgml).

I don't think it's ridiculous, but you can certainly disagree.

>> superuser privileges; it's the selective-dump case where you can often
>> get by without them. I've attached a proposed patch along these lines
>> for your consideration.
>
> That's fair.

Should I go ahead and apply that portion, then?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


From: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "Karl O(dot) Pinc" <kop(at)meme(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-11-08 18:33:15
Message-ID: 527D2E6B.2050409@commandprompt.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/08/2013 06:04 AM, Robert Haas wrote:
>
> On Thu, Nov 7, 2013 at 11:37 AM, Joshua D. Drake <jd(at)commandprompt(dot)com> wrote:
>> This isn't software, it is docs. It is ridiculous to suggest we break this
>> up into 3-4 patches. This is a small doc patch to a single doc file
>> (backup.sgml).
>
> I don't think it's ridiculous, but you can certainly disagree.
>
>>> superuser privileges; it's the selective-dump case where you can often
>>> get by without them. I've attached a proposed patch along these lines
>>> for your consideration.
>>
>> That's fair.
>
> Should I go ahead and apply that portion, then?

I am certainly not opposed.

--
Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc
For my dreams of your image that blossoms
a rose in the deeps of my heart. - W.B. Yeats


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
Cc: Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "Karl O(dot) Pinc" <kop(at)meme(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-11-08 20:12:56
Message-ID: CA+TgmoZodUpRLYF2fOjaQS7KZFQRWvaYpxzUKcLG1HRP2_1Wig@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Nov 8, 2013 at 1:33 PM, Joshua D. Drake <jd(at)commandprompt(dot)com> wrote:
>>>> superuser privileges; it's the selective-dump case where you can often
>>>> get by without them. I've attached a proposed patch along these lines
>>>> for your consideration.
>>> That's fair.
>> Should I go ahead and apply that portion, then?
> I am certainly not opposed.

OK, done.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-11-08 20:18:44
Message-ID: 1383941924.5911.9@slate
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/08/2013 02:12:56 PM, Robert Haas wrote:
> On Fri, Nov 8, 2013 at 1:33 PM, Joshua D. Drake
> <jd(at)commandprompt(dot)com>
> wrote:
> >>>> superuser privileges; it's the selective-dump case where you can
> often
> >>>> get by without them. I've attached a proposed patch along these
> lines
> >>>> for your consideration.
> >>> That's fair.
> >> Should I go ahead and apply that portion, then?
> > I am certainly not opposed.
>
> OK, done.

So.... Joshusa/Ivan Do you want to send another
version of the patch with the committed changes
omitted for further review?

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein


From: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
To: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-11-08 21:42:56
Message-ID: 527D5AE0.20600@commandprompt.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/08/2013 12:18 PM, Karl O. Pinc wrote:
>
> On 11/08/2013 02:12:56 PM, Robert Haas wrote:
>> On Fri, Nov 8, 2013 at 1:33 PM, Joshua D. Drake
>> <jd(at)commandprompt(dot)com>
>> wrote:
>>>>>> superuser privileges; it's the selective-dump case where you can
>> often
>>>>>> get by without them. I've attached a proposed patch along these
>> lines
>>>>>> for your consideration.
>>>>> That's fair.
>>>> Should I go ahead and apply that portion, then?
>>> I am certainly not opposed.
>>
>> OK, done.
>
> So.... Joshusa/Ivan Do you want to send another
> version of the patch with the committed changes
> omitted for further review?
>

I have no problem having Ivan submit another patch, outside of the
multiple patch requirement. If you guys will accept a single patch file
with the agreed changes, then we are good.

Joshua D. Drake

--
Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc
For my dreams of your image that blossoms
a rose in the deeps of my heart. - W.B. Yeats


From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Ivan Lezhnjov IV <iliv(at)commandprompt(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backup.sgml-cmd-v003.patch
Date: 2013-11-08 21:49:45
Message-ID: 1383947385.5911.11@slate
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/08/2013 03:42:56 PM, Joshua D. Drake wrote:
>
> On 11/08/2013 12:18 PM, Karl O. Pinc wrote:
> >
> > On 11/08/2013 02:12:56 PM, Robert Haas wrote:
> >> On Fri, Nov 8, 2013 at 1:33 PM, Joshua D. Drake
> >> <jd(at)commandprompt(dot)com>
> >> wrote:

> >>>> Should I go ahead and apply that portion, then?
> >>> I am certainly not opposed.
> >>
> >> OK, done.
> >
> > So.... Joshusa/Ivan Do you want to send another
> > version of the patch with the committed changes
> > omitted for further review?
> >
>
> I have no problem having Ivan submit another patch, outside of the
> multiple patch requirement. If you guys will accept a single patch
> file
> with the agreed changes, then we are good.

I can't say what will be accepted and haven't really kept
track of what has been accepted. If there's more that
you want to patch then send that and we'll
work from there.

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein