pg_receivewal documentation

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
37 messages Options
12
Reply | Threaded
Open this post in threaded view
|

pg_receivewal documentation

Jesper Pedersen
Hi,

Here is a patch for the pg_receivewal documentation to highlight that
WAL isn't acknowledged to be applied.

I'll add a CF entry for it.

Best regards,
  Jesper

v1_pgreceivewal-doc.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Laurenz Albe
On Thu, 2019-06-27 at 10:06 -0400, Jesper Pedersen wrote:
> Here is a patch for the pg_receivewal documentation to highlight that
> WAL isn't acknowledged to be applied.

I think it is a good idea to document this, but I have a few quibbles
with the patch as it is:

- I think there shouldn't be commas after the "note" and before the "if".
  Disclaimer: I am not a native speaker, so I am lacking authority.

- The assertion is wrong.  "on" (remote flush) is perfectly fine
  for synchronous_commit, only "remote_apply" is a problem.

- There is already something about "--synchronous" in the "Description"
  section.  It might make sense to add the additional information there.

How about the attached patch?

Yours,
Laurenz Albe

0001-Better-documentation-for-pg_receivewal-synchronous.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Jesper Pedersen
Hi Laurenz,

On 7/9/19 5:16 AM, Laurenz Albe wrote:

> On Thu, 2019-06-27 at 10:06 -0400, Jesper Pedersen wrote:
>> Here is a patch for the pg_receivewal documentation to highlight that
>> WAL isn't acknowledged to be applied.
>
> I think it is a good idea to document this, but I have a few quibbles
> with the patch as it is:
>
> - I think there shouldn't be commas after the "note" and before the "if".
>    Disclaimer: I am not a native speaker, so I am lacking authority.
>
> - The assertion is wrong.  "on" (remote flush) is perfectly fine
>    for synchronous_commit, only "remote_apply" is a problem.
>
> - There is already something about "--synchronous" in the "Description"
>    section.  It might make sense to add the additional information there.
>
> How about the attached patch?
>
Thanks for the review, and the changes.

However, I think it belongs in the --synchronous section, so what about
moving it there as attached ?

Best regards,
  Jesper

v3_pgreceivewal-doc.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Laurenz Albe
Jesper Pedersen wrote:
> Thanks for the review, and the changes.
>
> However, I think it belongs in the --synchronous section, so what about
> moving it there as attached ?

Works for me.

Marked as "ready for committer".

Yours,
Laurenz Albe



Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Michael Paquier-2
On Wed, Jul 10, 2019 at 12:22:02AM +0200, Laurenz Albe wrote:
> Works for me.
>
> Marked as "ready for committer".

Hmm.  synchronous_commit is user-settable, which means that it is
possible to enforce a value in the connection string doing the
connection.  Isn't that something we had better enforce directly in
the tool?  In this case what could be fixed is GetConnection() which
builds the connection string parameters.  One thing that we would need
to be careful about is that if the caller has provided a parameter for
"options" (which is plausible as wal_sender_timeout is user-settable
as of 12), then we need to make sure that the original value is
preserved, and that the enforced of synchronous_commit is appended.

Or, as you say, we just adjust the documentation.  However I would
recommend adding at least an example of connection string which uses
"options" if the server sets synchronous_commit to "remote_apply" in
this case.  Still it seems to me that we have ways to reduce the
confusion automatically.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Jesper Pedersen
In reply to this post by Laurenz Albe
Hi,

On 7/9/19 6:22 PM, Laurenz Albe wrote:
> Works for me.
>
> Marked as "ready for committer".
>

Thank you !

Best regards,
  Jesper



Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Jesper Pedersen
In reply to this post by Michael Paquier-2
Hi,

On 7/10/19 4:04 AM, Michael Paquier wrote:

> On Wed, Jul 10, 2019 at 12:22:02AM +0200, Laurenz Albe wrote:
>> Works for me.
>>
>> Marked as "ready for committer".
>
> Hmm.  synchronous_commit is user-settable, which means that it is
> possible to enforce a value in the connection string doing the
> connection.  Isn't that something we had better enforce directly in
> the tool?  In this case what could be fixed is GetConnection() which
> builds the connection string parameters.  One thing that we would need
> to be careful about is that if the caller has provided a parameter for
> "options" (which is plausible as wal_sender_timeout is user-settable
> as of 12), then we need to make sure that the original value is
> preserved, and that the enforced of synchronous_commit is appended.
>

I think that the above is out-of-scope for this patch. And ...

> Or, as you say, we just adjust the documentation.  However I would
> recommend adding at least an example of connection string which uses
> "options" if the server sets synchronous_commit to "remote_apply" in
> this case.  Still it seems to me that we have ways to reduce the
> confusion automatically.


The patch tries to highlight that if you f.ex. have

postgresql.conf
===============
synchronous_commit = remote_apply
synchronous_standby_names = '*'

and you _only_ have pg_receivewal connected then changes are only
applied locally to the primary instance and any client (psql, ...) won't
get acknowledged. The replay_lsn for the pg_receivewal connection will
keep increasing, so

env PGOPTIONS="-c synchronous_commit=remote_write" pg_receivewal -D
/tmp/wal -S replica1 --synchronous

won't help you.

We could add some wording around 'synchronous_standby_names' if it makes
the case clearer.

Best regards,
  Jesper


Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Alvaro Herrera-9
In reply to this post by Jesper Pedersen
On 2019-Jul-09, Jesper Pedersen wrote:

> +       <para>
> +        Note that while WAL will be flushed with this setting,
> +        it will never be applied, so <xref linkend="guc-synchronous-commit"/> must
> +        not be set to <literal>remote_apply</literal> if <application>pg_receivewal</application>
> +        is the only synchronous standby.
> +       </para>

+1 to document this caveat.

How about
        Note that while WAL will be flushed with this setting,
        <application>pg_receivewal</application> never applies it, so
        <xref linkend="guc-synchronous-commit"/> must not be set to
        <literal>remote_apply</literal> if <application>pg_receivewal</application>
        is the only synchronous standby.
?

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


Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Jesper Pedersen
Hi,

On 7/10/19 10:24 AM, Alvaro Herrera wrote:

> +1 to document this caveat.
>
> How about
>          Note that while WAL will be flushed with this setting,
>          <application>pg_receivewal</application> never applies it, so
>          <xref linkend="guc-synchronous-commit"/> must not be set to
>          <literal>remote_apply</literal> if <application>pg_receivewal</application>
>          is the only synchronous standby.
> ?
>
Sure.

Best regards,
  Jesper

v4_pgreceivewal-doc.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Laurenz Albe
In reply to this post by Michael Paquier-2
On Wed, 2019-07-10 at 17:04 +0900, Michael Paquier wrote:
> Hmm.  synchronous_commit is user-settable, which means that it is
> possible to enforce a value in the connection string doing the
> connection.  Isn't that something we had better enforce directly in
> the tool?  In this case what could be fixed is GetConnection() which
> builds the connection string parameters.

I don't follow.

Are you talking about the replication connection from pg_receivewal
to the PostgreSQL server?  That wouldn't do anything, because it is
the setting of "synchronous_commit" for an independent client
connection that is the problem:

- pg_receivewal starts a replication connection.

- It is added to "synchronous_standby_names" on the server.

- A client connects. It sets "synchronous_commit" to "remote_apply".

- If the client modifies data, COMMIT will hang indefinitely,
  because pg_receivewal will never send confirmation that it has
  applied the changes.

One alternative option I see is for pg_receivewal to confirm that
it has applied the changes as soon as it flushed them.
It would be cheating somewhat, but it would work around the problem
in a way that few people would find surprising.

Yours,
Laurenz Albe



Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Michael Paquier-2
On Wed, Jul 10, 2019 at 09:12:46PM +0200, Laurenz Albe wrote:
> Are you talking about the replication connection from pg_receivewal
> to the PostgreSQL server?  That wouldn't do anything, because it is
> the setting of "synchronous_commit" for an independent client
> connection that is the problem:

Ditto.  My previous message was wrong and you are right.  You are
right that this had better be documented.  I have no thought this ne
through completely.

> One alternative option I see is for pg_receivewal to confirm that
> it has applied the changes as soon as it flushed them.
> It would be cheating somewhat, but it would work around the problem
> in a way that few people would find surprising.

Yes, that's wrong as pg_receivewal applies nothing.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Michael Paquier-2
In reply to this post by Jesper Pedersen
On Wed, Jul 10, 2019 at 11:26:04AM -0400, Jesper Pedersen wrote:

> On 7/10/19 10:24 AM, Alvaro Herrera wrote:
> > +1 to document this caveat.
>>
>> How about
>>          Note that while WAL will be flushed with this setting,
>>          <application>pg_receivewal</application> never applies it, so
>>          <xref linkend="guc-synchronous-commit"/> must not be set to
>>          <literal>remote_apply</literal> if <application>pg_receivewal</application>
>>          is the only synchronous standby.
>> ?
>>
>
> Sure.
This is not true in all cases as since 9.6 it is possible to specify
multiple synchronous standbys.  So if for example pg_receivewal and
another synchronous standby are set in s_s_names and that the number
of a FIRST (priority-based) or ANY (quorum set) is two, then the same
issue exists, but this documentation is incorrect.  I think that we
should have a more extensive wording  here, like "if pg_receivewal is
part of a quorum-based or priority-based set of synchronous standbys."

Thoughts?
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Laurenz Albe
On Tue, 2019-07-16 at 14:05 +0900, Michael Paquier wrote:

> >> How about
> >>          Note that while WAL will be flushed with this setting,
> >>          <application>pg_receivewal</application> never applies it, so
> >>          <xref linkend="guc-synchronous-commit"/> must not be set to
> >>          <literal>remote_apply</literal> if <application>pg_receivewal</application>
> >>          is the only synchronous standby.
>
> This is not true in all cases as since 9.6 it is possible to specify
> multiple synchronous standbys.  So if for example pg_receivewal and
> another synchronous standby are set in s_s_names and that the number
> of a FIRST (priority-based) or ANY (quorum set) is two, then the same
> issue exists, but this documentation is incorrect.  I think that we
> should have a more extensive wording  here, like "if pg_receivewal is
> part of a quorum-based or priority-based set of synchronous standbys."

I think this would be overly complicated.
The wording above seems to cover the priority-based base sufficiently
in my opinion.
Maybe a second sentence with more detail would be better:

  ... must not be set to <literal>remote_apply</literal> if
  <application>pg_receivewal</application> is the only synchronous standby.
  Similarly, if <application>pg_receivewal</application> is part of
  a quorum-based set of synchronous standbys, it won't count towards
  the quorum if <xref linkend="guc-synchronous-commit"/> is set to
  <literal>remote_apply</literal>.

Yours,
Laurenz Albe



Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Jesper Pedersen
Hi,

On 7/16/19 12:28 PM, Laurenz Albe wrote:

>> This is not true in all cases as since 9.6 it is possible to specify
>> multiple synchronous standbys.  So if for example pg_receivewal and
>> another synchronous standby are set in s_s_names and that the number
>> of a FIRST (priority-based) or ANY (quorum set) is two, then the same
>> issue exists, but this documentation is incorrect.  I think that we
>> should have a more extensive wording  here, like "if pg_receivewal is
>> part of a quorum-based or priority-based set of synchronous standbys."
>
> I think this would be overly complicated.
> The wording above seems to cover the priority-based base sufficiently
> in my opinion.
> Maybe a second sentence with more detail would be better:
>
>    ... must not be set to <literal>remote_apply</literal> if
>    <application>pg_receivewal</application> is the only synchronous standby.
>    Similarly, if <application>pg_receivewal</application> is part of
>    a quorum-based set of synchronous standbys, it won't count towards
>    the quorum if <xref linkend="guc-synchronous-commit"/> is set to
>    <literal>remote_apply</literal>.
>
Here is the patch for that.

Best regards,
  Jesper



v5_pgreceivewal-doc.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Michael Paquier-2
On Tue, Jul 16, 2019 at 01:03:12PM -0400, Jesper Pedersen wrote:
> Here is the patch for that.

+       <para>
+        Note that while WAL will be flushed with this setting,
+        <application>pg_receivewal</application> never applies it, so
+        <xref linkend="guc-synchronous-commit"/> must not be set to
+        <literal>remote_apply</literal> if <application>pg_receivewal</application>
+        is the only synchronous standby. Similarly, if
+        <application>pg_receivewal</application> is part of a quorum-based
+        set of synchronous standbys, it won't count towards the quorum if
+        <xref linkend="guc-synchronous-commit"/> is set to
+        <literal>remote_apply</literal>.
+       </para>

I think we should really document the caveat with priority-based sets
of standbys as much as quorum-based sets.  For example if a user sets
synchronous_commit = remote_apply in postgresql.conf, and then sets
s_s_names to '2(pg_receivewal, my_connected_standby)' to get a
priority-based set, then you have the same problem, and pg_receivewal
is not the only synchronous standby in this configuration.  The patch
does not cover that case properly.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Laurenz Albe
On Wed, 2019-07-17 at 10:38 +0900, Michael Paquier wrote:

> +       <para>
> +        Note that while WAL will be flushed with this setting,
> +        <application>pg_receivewal</application> never applies it, so
> +        <xref linkend="guc-synchronous-commit"/> must not be set to
> +        <literal>remote_apply</literal> if <application>pg_receivewal</application>
> +        is the only synchronous standby. Similarly, if
> +        <application>pg_receivewal</application> is part of a quorum-based
> +        set of synchronous standbys, it won't count towards the quorum if
> +        <xref linkend="guc-synchronous-commit"/> is set to
> +        <literal>remote_apply</literal>.
> +       </para>
>
> I think we should really document the caveat with priority-based sets
> of standbys as much as quorum-based sets.  For example if a user sets
> synchronous_commit = remote_apply in postgresql.conf, and then sets
> s_s_names to '2(pg_receivewal, my_connected_standby)' to get a
> priority-based set, then you have the same problem, and pg_receivewal
> is not the only synchronous standby in this configuration.  The patch
> does not cover that case properly.

I understand the concern, I'm just worried that too much accuracy may
render the sentence hard to read.

How about adding "or priority-based" after "quorum-based"?

Yours,
Laurenz Albe



Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Michael Paquier-2
On Wed, Jul 17, 2019 at 07:40:48AM +0200, Laurenz Albe wrote:
> I understand the concern, I'm just worried that too much accuracy may
> render the sentence hard to read.
>
> How about adding "or priority-based" after "quorum-based"?

I would be fine with that for the first part.  I am not sure of what a
good formulation would be for the second part of the sentence.  Now it
only refers to quorum, but with priority sets that does not apply.
And I am not sure what "won't count towards the quorum" actually
means.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Jesper Pedersen
Hi,

On 7/17/19 4:04 AM, Michael Paquier wrote:
>> How about adding "or priority-based" after "quorum-based"?
>
> I would be fine with that for the first part.  I am not sure of what a
> good formulation would be for the second part of the sentence.  Now it
> only refers to quorum, but with priority sets that does not apply.
> And I am not sure what "won't count towards the quorum" actually
> means.

Maybe something like the attached ?  Although it doesn't help we need to
include <literal>on</literal> as well...

Best regards,
  Jesper



v6_pgreceivewal-doc.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Laurenz Albe
On Wed, 2019-07-17 at 13:59 -0400, Jesper Pedersen wrote:

> +       <para>
> +        Note that while WAL will be flushed with this setting,
> +        <application>pg_receivewal</application> never applies it, so
> +        <xref linkend="guc-synchronous-commit"/> must not be set to
> +        <literal>remote_apply</literal> or <literal>on</literal>
> +        if <application>pg_receivewal</application> is the only synchronous standby.
> +        Similarly, if <application>pg_receivewal</application> is part of a
> +        priority-based synchronous replication setup (<literal>FIRST</literal>),
> +        or a quorum-based setup (<literal>ANY</literal>) it won't count towards
> +        the policy specified if <xref linkend="guc-synchronous-commit"/> is
> +        set to <literal>remote_apply</literal> or <literal>on</literal>.
> +       </para>

That's factually wrong.  "on" (wait for WAL flush) works fine with
pg_receivewal, only "remote_apply" doesn't.

Ok, here's another attempt:

   Note that while WAL will be flushed with this setting,
   <application>pg_receivewal</application> never applies it, so
   <xref linkend="guc-synchronous-commit"/> must not be set to
   <literal>remote_apply</literal> if <application>pg_receivewal</application>
   is the only synchronous standby.
   Similarly, it is no use adding <application>pg_receivewal</application> to a
   priority-based (<literal>FIRST</literal>) or a quorum-based
   (<literal>ANY</literal>) synchronous replication setup if
   <xref linkend="guc-synchronous-commit"/> is set to <literal>remote_apply</literal>.

Yours,
Laurenz Albe



Reply | Threaded
Open this post in threaded view
|

Re: pg_receivewal documentation

Michael Paquier-2
On Wed, Jul 17, 2019 at 11:21:06PM +0200, Laurenz Albe wrote:

> Ok, here's another attempt:
>
>    Note that while WAL will be flushed with this setting,
>    <application>pg_receivewal</application> never applies it, so
>    <xref linkend="guc-synchronous-commit"/> must not be set to
>    <literal>remote_apply</literal> if <application>pg_receivewal</application>
>    is the only synchronous standby.
>    Similarly, it is no use adding <application>pg_receivewal</application> to a
>    priority-based (<literal>FIRST</literal>) or a quorum-based
>    (<literal>ANY</literal>) synchronous replication setup if
>    <xref linkend="guc-synchronous-commit"/> is set to <literal>remote_apply</literal>.
Or more simply like that?
"Note that while WAL will be flushed with this setting,
pg_receivewal never applies it, so synchronous_commit must not be set
to remote_apply if pg_receivewal is a synchronous standby, be it a
member of a priority-based (FIRST) or a quorum-based (ANY) synchronous
replication setup."
--
Michael

signature.asc (849 bytes) Download Attachment
12