Horton

Thread

Commits

  1. Clarify initdb --sync-only help message and docs

  1. Slightly improve initdb --sync-only option's help message

    Gurjeet Singh <gurjeet@singh.im> — 2021-07-07T02:01:07Z

    When reading the output of `initdb --help` I could not clearly
understand what the purpose of the --sync-only option was, until I
read the documentation of initdb.

  -S, --sync-only           only sync data directory

Perhaps the confusion was caused by the fact that sync(hronization)
means different things in different contexts, and many of those
contexts apply to databases, and to data directories; time sync, data
sync, replica sync, etc.

I think it would be helpful if the help message was slightly more
descriptive. Some options:

Used in patch:
     only sync data directory; does not modify any data

To match the wording of --sync-only option:
    write contents of data directory to disk; helpful after --no-sync option

Clearly specify the system operation used for the option
    perform fsync on data directory; helpful after --no-sync option

Best regards,
--
Gurjeet Singh http://gurjeet.singh.im/

  2. Re: Slightly improve initdb --sync-only option's help message

    Nathan Bossart <bossartn@amazon.com> — 2021-07-22T22:32:18Z

    On 7/6/21, 7:02 PM, "Gurjeet Singh" <gurjeet@singh.im> wrote:
> I think it would be helpful if the help message was slightly more
> descriptive. Some options:
>
> Used in patch:
>      only sync data directory; does not modify any data
>
> To match the wording of --sync-only option:
>     write contents of data directory to disk; helpful after --no-sync option
>
> Clearly specify the system operation used for the option
>     perform fsync on data directory; helpful after --no-sync option

I think the help message should say exactly what the option does and
should avoid saying what it does not do or how it may be useful.  I
would suggest the following to match the initdb docs [0]:

          -S, --sync-only           safely write all database files to disk and exit

IMO the note about the option being helpful after using the --no-sync
option would fit better in the docs, but I'm struggling to think of a
use case for using --no-sync and then calling initdb again with
--sync-only.  Why wouldn't you just leave out --no-sync the first
time?

Nathan

[0] https://www.postgresql.org/docs/devel/app-initdb.html

  3. Re: Slightly improve initdb --sync-only option's help message

    Justin Pryzby <pryzby@telsasoft.com> — 2021-07-23T01:31:18Z

    On Thu, Jul 22, 2021 at 10:32:18PM +0000, Bossart, Nathan wrote:
> On 7/6/21, 7:02 PM, "Gurjeet Singh" <gurjeet@singh.im> wrote:
> > I think it would be helpful if the help message was slightly more
> > descriptive. Some options:
> >
> > Used in patch:
> >      only sync data directory; does not modify any data
> >
> > To match the wording of --sync-only option:
> >     write contents of data directory to disk; helpful after --no-sync option
> >
> > Clearly specify the system operation used for the option
> >     perform fsync on data directory; helpful after --no-sync option
> 
> I think the help message should say exactly what the option does and
> should avoid saying what it does not do or how it may be useful.  I
> would suggest the following to match the initdb docs [0]:
> 
>           -S, --sync-only           safely write all database files to disk and exit
> 
> IMO the note about the option being helpful after using the --no-sync
> option would fit better in the docs, but I'm struggling to think of a
> use case for using --no-sync and then calling initdb again with
> --sync-only.  Why wouldn't you just leave out --no-sync the first
> time?

It's to allow safely running bulk loading with fsync=off - if the bulk load
fails, you can wipe out the partially-loaded cluster and start over.
But then transitioning to a durable state requires not just setting fsync=on,
which enables future fsync calls.  It also requires syncing all dirty buffers.

doc/src/sgml/config.sgml-       <para>
doc/src/sgml/config.sgml-        For reliable recovery when changing <varname>fsync</varname>
doc/src/sgml/config.sgml-        off to on, it is necessary to force all modified buffers in the
doc/src/sgml/config.sgml-        kernel to durable storage.  This can be done while the cluster
doc/src/sgml/config.sgml-        is shutdown or while <varname>fsync</varname> is on by running <command>initdb
doc/src/sgml/config.sgml:        --sync-only</command>, running <command>sync</command>, unmounting the
doc/src/sgml/config.sgml-        file system, or rebooting the server.
doc/src/sgml/config.sgml-       </para>

-- 
Justin

  4. Re: Slightly improve initdb --sync-only option's help message

    Nathan Bossart <bossartn@amazon.com> — 2021-07-23T16:08:51Z

    On 7/22/21, 6:31 PM, "Justin Pryzby" <pryzby@telsasoft.com> wrote:
> On Thu, Jul 22, 2021 at 10:32:18PM +0000, Bossart, Nathan wrote:
>> IMO the note about the option being helpful after using the --no-sync
>> option would fit better in the docs, but I'm struggling to think of a
>> use case for using --no-sync and then calling initdb again with
>> --sync-only.  Why wouldn't you just leave out --no-sync the first
>> time?
>
> It's to allow safely running bulk loading with fsync=off - if the bulk load
> fails, you can wipe out the partially-loaded cluster and start over.
> But then transitioning to a durable state requires not just setting fsync=on,
> which enables future fsync calls.  It also requires syncing all dirty buffers.

Right.  Perhaps the documentation for --sync-only could mention this
use-case instead.

        Safely write all database files to disk and exit. This does
        not perform any of the normal initdb operations.  Generally,
        this option is useful for ensuring reliable recovery after
        changing fsync from off to on.

Nathan

  5. Re: Slightly improve initdb --sync-only option's help message

    Nathan Bossart <bossartn@amazon.com> — 2021-07-26T18:05:50Z

    On 7/23/21, 9:09 AM, "Bossart, Nathan" <bossartn@amazon.com> wrote:
> On 7/22/21, 6:31 PM, "Justin Pryzby" <pryzby@telsasoft.com> wrote:
>> On Thu, Jul 22, 2021 at 10:32:18PM +0000, Bossart, Nathan wrote:
>>> IMO the note about the option being helpful after using the --no-sync
>>> option would fit better in the docs, but I'm struggling to think of a
>>> use case for using --no-sync and then calling initdb again with
>>> --sync-only.  Why wouldn't you just leave out --no-sync the first
>>> time?
>>
>> It's to allow safely running bulk loading with fsync=off - if the bulk load
>> fails, you can wipe out the partially-loaded cluster and start over.
>> But then transitioning to a durable state requires not just setting fsync=on,
>> which enables future fsync calls.  It also requires syncing all dirty buffers.
>
> Right.  Perhaps the documentation for --sync-only could mention this
> use-case instead.
>
>         Safely write all database files to disk and exit. This does
>         not perform any of the normal initdb operations.  Generally,
>         this option is useful for ensuring reliable recovery after
>         changing fsync from off to on.

Here are my suggestions in patch form.

Nathan

  6. Re: Slightly improve initdb --sync-only option's help message

    Gurjeet Singh <gurjeet@singh.im> — 2021-07-29T02:21:49Z

    On Mon, Jul 26, 2021 at 11:05 AM Bossart, Nathan <bossartn@amazon.com> wrote:
> Here are my suggestions in patch form.

+    printf(_("  -S, --sync-only           safely write all database
files to disk and exit\n"));

Not your patch's fault, but the word "write" does not seem to convey
the true intent of the option, because generally a "write" operation
is still limited to dirtying the OS buffers, and does not guarantee
sync-to-disk.

It'd be better if the help message said, either "flush all database
files to disk and exit",or "sync all database files to disk and exit".

Best regards,
--
Gurjeet Singh http://gurjeet.singh.im/

  7. Re: Slightly improve initdb --sync-only option's help message

    Daniel Gustafsson <daniel@yesql.se> — 2021-07-29T09:10:25Z

    > On 26 Jul 2021, at 20:05, Bossart, Nathan <bossartn@amazon.com> wrote:

> Here are my suggestions in patch form.

-	printf(_("  -S, --sync-only           only sync data directory\n"));
+	printf(_("  -S, --sync-only           safely write all database files to disk and exit\n"));

I think removing the word "only" here is a net loss, since it IMO clearly
conveyed that this option isn't doing any of the other initdb duties.  The
documentation states it in more words, but the help output must be brief so I
think "only" is a good keyword.

--
Daniel Gustafsson		https://vmware.com/

  8. Re: Slightly improve initdb --sync-only option's help message

    Nathan Bossart <bossartn@amazon.com> — 2021-07-29T23:37:56Z

    On 7/29/21, 2:11 AM, "Daniel Gustafsson" <daniel@yesql.se> wrote:
> I think removing the word "only" here is a net loss, since it IMO clearly
> conveyed that this option isn't doing any of the other initdb duties.  The
> documentation states it in more words, but the help output must be brief so I
> think "only" is a good keyword.

I've attached a new version of the patch in which I've attempted to
address both sets of feedback.

Nathan

  9. Re: Slightly improve initdb --sync-only option's help message

    Daniel Gustafsson <daniel@yesql.se> — 2021-07-30T09:21:50Z

    > On 30 Jul 2021, at 01:37, Bossart, Nathan <bossartn@amazon.com> wrote:
> 
> On 7/29/21, 2:11 AM, "Daniel Gustafsson" <daniel@yesql.se> wrote:
>> I think removing the word "only" here is a net loss, since it IMO clearly
>> conveyed that this option isn't doing any of the other initdb duties.  The
>> documentation states it in more words, but the help output must be brief so I
>> think "only" is a good keyword.
> 
> I've attached a new version of the patch in which I've attempted to
> address both sets of feedback.

LGTM.  I took the liberty to rephrase the "and exit" part of the initdb help
output match the other exiting options in there.  Barring objections, I think
this is ready.

--
Daniel Gustafsson		https://vmware.com/

  10. Re: Slightly improve initdb --sync-only option's help message

    Nathan Bossart <bossartn@amazon.com> — 2021-07-30T16:27:18Z

    On 7/30/21, 2:22 AM, "Daniel Gustafsson" <daniel@yesql.se> wrote:
> LGTM.  I took the liberty to rephrase the "and exit" part of the initdb help
> output match the other exiting options in there.  Barring objections, I think
> this is ready.

LGTM.  Thanks!

Nathan

  11. Re: Slightly improve initdb --sync-only option's help message

    Daniel Gustafsson <daniel@yesql.se> — 2021-08-16T11:42:01Z

    > On 30 Jul 2021, at 18:27, Bossart, Nathan <bossartn@amazon.com> wrote:
> 
> On 7/30/21, 2:22 AM, "Daniel Gustafsson" <daniel@yesql.se> wrote:
>> LGTM.  I took the liberty to rephrase the "and exit" part of the initdb help
>> output match the other exiting options in there.  Barring objections, I think
>> this is ready.
> 
> LGTM.  Thanks!

Pushed to master, thanks!

--
Daniel Gustafsson		https://vmware.com/

  12. Re: Slightly improve initdb --sync-only option's help message

    Gurjeet Singh <gurjeet@singh.im> — 2021-08-16T16:06:08Z

    On Mon, Aug 16, 2021 at 4:42 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 30 Jul 2021, at 18:27, Bossart, Nathan <bossartn@amazon.com> wrote:
> >
> > On 7/30/21, 2:22 AM, "Daniel Gustafsson" <daniel@yesql.se> wrote:
> >> LGTM.  I took the liberty to rephrase the "and exit" part of the initdb help
> >> output match the other exiting options in there.  Barring objections, I think
> >> this is ready.
> >
> > LGTM.  Thanks!
>
> Pushed to master, thanks!

Thank you Daniel and Nathan! Much appreciated.

Best regards,
--
Gurjeet Singh http://gurjeet.singh.im/