Thread

Commits

Doc: Clarify interactions of pg_receivexlog with remote_apply
- 4a25ed16400c 9.6.15 landed
Doc: Clarify interactions of pg_receivewal with remote_apply
- 1c423463e4b9 10.10 landed
- 059e22693b23 11.5 landed
- a15a40aaaedf 12.0 landed
- fd7d387e0548 13.0 landed

pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-06-27T14:06:46Z

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

Re: pg_receivewal documentation

Laurenz Albe <laurenz.albe@cybertec.at> — 2019-07-09T09:16:55Z

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

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-09T17:18:52Z

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

Re: pg_receivewal documentation

Laurenz Albe <laurenz.albe@cybertec.at> — 2019-07-09T22:22:02Z

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

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-10T08:04:23Z

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

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-10T12:31:59Z

Hi,

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

Thank you !

Best regards,
  Jesper

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-10T12:48:12Z

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

Re: pg_receivewal documentation

Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-07-10T14:24:21Z

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

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-10T15:26:04Z

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

Re: pg_receivewal documentation

Laurenz Albe <laurenz.albe@cybertec.at> — 2019-07-10T19:12:46Z

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

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-11T04:58:06Z

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

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-16T05:05:55Z

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

Re: pg_receivewal documentation

Laurenz Albe <laurenz.albe@cybertec.at> — 2019-07-16T16:28:57Z

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

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-16T17:03:12Z

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

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-17T01:38:35Z

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

Re: pg_receivewal documentation

Laurenz Albe <laurenz.albe@cybertec.at> — 2019-07-17T05:40:48Z

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

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-17T08:04:24Z

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

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-17T17:59:55Z

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

Re: pg_receivewal documentation

Laurenz Albe <laurenz.albe@cybertec.at> — 2019-07-17T21:21:06Z

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

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-18T05:29:09Z

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

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-18T12:39:48Z

Hi Laurenz,

On 7/17/19 5:21 PM, Laurenz Albe wrote:
> That's factually wrong.  "on" (wait for WAL flush) works fine with
> pg_receivewal, only "remote_apply" doesn't.
> 

Please, try

mkdir /tmp/wal
initdb /tmp/pgsql
pg_ctl -D /tmp/pgsql -l /tmp/logfile start
psql postgres
SELECT pg_create_physical_replication_slot('replica1');
CREATE ROLE repluser WITH LOGIN REPLICATION PASSWORD 'replpass';
\q

synchronous_commit = on
synchronous_standby_names = 'replica1'

pg_ctl -D /tmp/pgsql -l /tmp/logfile restart
pg_receivewal -D /tmp/wal -S replica1 --synchronous -h localhost -p 5432 
-U repluser -W
psql -c 'SELECT * FROM pg_stat_replication;' postgres
psql -c 'SELECT * FROM pg_replication_slots;' postgres
psql -c 'CREATE DATABASE test' postgres

In what scenarios do you see 'on' working ?

Best regards,
  Jesper

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-18T12:40:36Z

Hi,

On 7/18/19 1:29 AM, Michael Paquier wrote:
> 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."

Yeah, better.

Best regards,
  Jesper

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-19T01:09:03Z

On Thu, Jul 18, 2019 at 08:39:48AM -0400, Jesper Pedersen wrote:
> mkdir /tmp/wal
> initdb /tmp/pgsql
> pg_ctl -D /tmp/pgsql -l /tmp/logfile start
> psql postgres
> SELECT pg_create_physical_replication_slot('replica1');
> CREATE ROLE repluser WITH LOGIN REPLICATION PASSWORD 'replpass';
> \q
> 
> synchronous_commit = on
> synchronous_standby_names = 'replica1'
> 
> pg_ctl -D /tmp/pgsql -l /tmp/logfile restart
> pg_receivewal -D /tmp/wal -S replica1 --synchronous -h localhost -p 5432 -U
> repluser -W
> psql -c 'SELECT * FROM pg_stat_replication;' postgres
> psql -c 'SELECT * FROM pg_replication_slots;' postgres
> psql -c 'CREATE DATABASE test' postgres
> 
> In what scenarios do you see 'on' working ?

Because the code says so, "on" is an alias for "remote_flush" (which
is not user-visible by the way):
src/include/access/xact.h:#define SYNCHRONOUS_COMMIT_ON
SYNCHRONOUS_COMMIT_REMOTE_FLUSH

And if you do that it works fine (pg_receivewal --synchronous runs in
the background and I created a dummy table):
=# SELECT application_name, sync_state, flush_lsn, replay_lsn FROM
pg_stat_replication;
 application_name | sync_state | flush_lsn | replay_lsn
------------------+------------+-----------+------------
 pg_receivewal    | sync       | 0/15E1F88 | null
(1 row)
=# set synchronous_commit to on ;
SET
=# insert into aa values (2);
INSERT 0 1

This part however is as expected, just blocking:
=# set synchronous_commit to remote_apply ;
SET
=# insert into aa values (3);
^CCancel request sent
WARNING:  01000: canceling wait for synchronous replication due to
user request
DETAIL:  The transaction has already committed locally, but might not
have been replicated to the standby.
LOCATION:  SyncRepWaitForLSN, syncrep.c:266
INSERT 0 1
--
Michael

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-19T01:27:57Z

On Thu, Jul 18, 2019 at 08:40:36AM -0400, Jesper Pedersen wrote:
> On 7/18/19 1:29 AM, Michael Paquier wrote:
>> 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."
> 
> Yeah, better.

I was looking into committing that, and the part about
synchronous_commit = on is not right.  The location of the warning is
also harder to catch for the reader, so instead let's move it to the
top where we have an extra description for --synchronous.  I am
finishing with the attached that I would be fine to commit and
back-patch as needed.  Does that sound fine?
--
Michael

Re: pg_receivewal documentation

Laurenz Albe <laurenz.albe@cybertec.at> — 2019-07-19T06:50:17Z

On Fri, 2019-07-19 at 10:27 +0900, Michael Paquier wrote:
> On Thu, Jul 18, 2019 at 08:40:36AM -0400, Jesper Pedersen wrote:
> > On 7/18/19 1:29 AM, Michael Paquier wrote:
> > > 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."
> > 
> > Yeah, better.
> 
> I was looking into committing that, and the part about
> synchronous_commit = on is not right.  The location of the warning is
> also harder to catch for the reader, so instead let's move it to the
> top where we have an extra description for --synchronous.  I am
> finishing with the attached that I would be fine to commit and
> back-patch as needed.  Does that sound fine?

It was my first reaction too that this had better be at the top.

I'm happy with the patch as it is.

Yours,
Laurenz Albe

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-19T17:01:27Z

Hi,

On 7/18/19 9:09 PM, Michael Paquier wrote:
>> pg_receivewal -D /tmp/wal -S replica1 --synchronous -h localhost -p 5432 -U
>> repluser -W
>> psql -c 'SELECT * FROM pg_stat_replication;' postgres
>> psql -c 'SELECT * FROM pg_replication_slots;' postgres
>> psql -c 'CREATE DATABASE test' postgres
>>
>> In what scenarios do you see 'on' working ?
> 
> Because the code says so, "on" is an alias for "remote_flush" (which
> is not user-visible by the way):
> src/include/access/xact.h:#define SYNCHRONOUS_COMMIT_ON
> SYNCHRONOUS_COMMIT_REMOTE_FLUSH
> 
> And if you do that it works fine (pg_receivewal --synchronous runs in
> the background and I created a dummy table):
> =# SELECT application_name, sync_state, flush_lsn, replay_lsn FROM
> pg_stat_replication;
>   application_name | sync_state | flush_lsn | replay_lsn
> ------------------+------------+-----------+------------
>   pg_receivewal    | sync       | 0/15E1F88 | null
> (1 row)
> =# set synchronous_commit to on ;
> SET
> =# insert into aa values (2);
> INSERT 0 1
> 

I forgot to use pg_receivewal -d with application_name instead of -h -p -U.

Maybe we should have an explicit option for that, but that is a separate 
thread.

Best regards,
  Jesper

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-19T17:02:21Z

Hi,

On 7/18/19 9:27 PM, Michael Paquier wrote:
> The location of the warning is
> also harder to catch for the reader, so instead let's move it to the
> top where we have an extra description for --synchronous.  I am
> finishing with the attached that I would be fine to commit and
> back-patch as needed.  Does that sound fine?

LGTM.

Best regards,
  Jesper

Re: pg_receivewal documentation

Robert Haas <robertmhaas@gmail.com> — 2019-07-19T18:04:03Z

On Tue, Jul 16, 2019 at 9:38 PM Michael Paquier <michael@paquier.xyz> wrote:
> 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 don't agree with this approach. It seems to me that the original was
too precise already, and making it more precise only exacerbates the
situation.  The point is that synchronous_commit = remote_apply is
*categorically* a bad idea for sessions running pg_receivewal.  The
reason why you're adding all this complexity is to try to distinguish
between the case where it's merely a bad idea and the case where it
will also completely fail to work. But why is it important to describe
the scenarios under which it will altogether fail to work?

You could just say something like:

Since pg_receivewal does not apply WAL, you should not allow it to
become a synchronous standby when synchronous_commit = remote_apply.
If it does, it will appear to be a standby which never catches up,
which may cause commits to block.  To avoid this, you should either
configure an appropriate value for synchronous_standby_names, or
specify an application_name for pg_receivewal that does not match it,
or change the value of synchronous_commit to something other than
remote_apply.

I think that'd be a lot more useful than enumerating the total-failure
scenarios.

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

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-22T01:48:51Z

On Fri, Jul 19, 2019 at 02:04:03PM -0400, Robert Haas wrote:
> You could just say something like:
> 
> Since pg_receivewal does not apply WAL, you should not allow it to
> become a synchronous standby when synchronous_commit = remote_apply.
> If it does, it will appear to be a standby which never catches up,
> which may cause commits to block.  To avoid this, you should either
> configure an appropriate value for synchronous_standby_names, or
> specify an application_name for pg_receivewal that does not match it,
> or change the value of synchronous_commit to something other than
> remote_apply.
> 
> I think that'd be a lot more useful than enumerating the total-failure
> scenarios.

+1.  Thanks for the suggestions!  Your wording looks good to me.
--
Michael

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-22T17:25:41Z

Hi,

On 7/21/19 9:48 PM, Michael Paquier wrote:
>> Since pg_receivewal does not apply WAL, you should not allow it to
>> become a synchronous standby when synchronous_commit = remote_apply.
>> If it does, it will appear to be a standby which never catches up,
>> which may cause commits to block.  To avoid this, you should either
>> configure an appropriate value for synchronous_standby_names, or
>> specify an application_name for pg_receivewal that does not match it,
>> or change the value of synchronous_commit to something other than
>> remote_apply.
>>
>> I think that'd be a lot more useful than enumerating the total-failure
>> scenarios.
> 
> +1.  Thanks for the suggestions!  Your wording looks good to me.

+1

Here is the patch for it, with Robert as the author.

Best regards,
  Jesper

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-23T00:08:20Z

On Mon, Jul 22, 2019 at 01:25:41PM -0400, Jesper Pedersen wrote:
> Hi,
> 
> On 7/21/19 9:48 PM, Michael Paquier wrote:
> > > Since pg_receivewal does not apply WAL, you should not allow it to
> > > become a synchronous standby when synchronous_commit = remote_apply.
> > > If it does, it will appear to be a standby which never catches up,
> > > which may cause commits to block.  To avoid this, you should either
> > > configure an appropriate value for synchronous_standby_names, or
> > > specify an application_name for pg_receivewal that does not match it,
> > > or change the value of synchronous_commit to something other than
> > > remote_apply.
> > > 
> > > I think that'd be a lot more useful than enumerating the total-failure
> > > scenarios.
> > 
> > +1.  Thanks for the suggestions!  Your wording looks good to me.
> 
> +1
> 
> Here is the patch for it, with Robert as the author.

> +   <xref linkend="guc-synchronous-commit"/> to something other than

Looks fine to me.  Just a tiny nit.  For the second reference to
synchronous_commit, I would change the link to a <varname> markup. 
--
Michael

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-23T12:00:41Z

Hi,

On 7/22/19 8:08 PM, Michael Paquier wrote:
>> +   <xref linkend="guc-synchronous-commit"/> to something other than
> 
> Looks fine to me.  Just a tiny nit.  For the second reference to
> synchronous_commit, I would change the link to a <varname> markup.

Sure.

Best regards,
  Jesper

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-24T02:29:28Z

On Tue, Jul 23, 2019 at 08:00:41AM -0400, Jesper Pedersen wrote:
> Sure.

Thanks.  Applied down to 9.6 where remote_apply has been introduced,
with tweaks for 9.6 as the tool is named pg_receivexlog there.
--
Michael

Re: pg_receivewal documentation

Jesper Pedersen <jesper.pedersen@redhat.com> — 2019-07-24T11:55:18Z

Hi,

On 7/23/19 10:29 PM, Michael Paquier wrote:
> Thanks.  Applied down to 9.6 where remote_apply has been introduced,
> with tweaks for 9.6 as the tool is named pg_receivexlog there.

Thanks to everybody involved !

Best regards,
  Jesper

Re: pg_receivewal documentation

Jehan-Guillaume de Rorthais <jgdr@dalibo.com> — 2019-07-24T13:03:04Z

Hi,

On Wed, 24 Jul 2019 11:29:28 +0900
Michael Paquier <michael@paquier.xyz> wrote:

> On Tue, Jul 23, 2019 at 08:00:41AM -0400, Jesper Pedersen wrote:
> > Sure.  
> 
> Thanks.  Applied down to 9.6 where remote_apply has been introduced,
> with tweaks for 9.6 as the tool is named pg_receivexlog there.

Sorry to step in so lately.

Unless I am missing something, another solution might be to use a dedicated
role to pg_receive{xlog|wal} with synchronous_commit lower than
remote_apply.

Not sure we want to add such detail, but if you consider it useful, you'll find
a patch in attachment.

Regards,

Re: pg_receivewal documentation

Michael Paquier <michael@paquier.xyz> — 2019-07-25T07:58:17Z

On Wed, Jul 24, 2019 at 03:03:04PM +0200, Jehan-Guillaume de Rorthais wrote:
> Unless I am missing something, another solution might be to use a dedicated
> role to pg_receive{xlog|wal} with synchronous_commit lower than
> remote_apply.

Aren't you confused by the same thing as I was upthread [1]?
[1]: https://www.postgresql.org/message-id/20190710080423.GG1031@paquier.xyz

remote_apply affects all sessions.  So even if you use a replication
role with synchronous_commit = on and have pg_receivewal use that with
remote_apply set in postgresql.conf, then remote_apply is effective
for all the other sessions so these will still be stuck at commit
waiting for pg_receivewal to apply WAL if it is a synchronous
standby.
--
Michael

Re: pg_receivewal documentation

Jehan-Guillaume de Rorthais <jgdr@dalibo.com> — 2019-07-25T08:29:44Z

On Thu, 25 Jul 2019 16:58:17 +0900
Michael Paquier <michael@paquier.xyz> wrote:

> On Wed, Jul 24, 2019 at 03:03:04PM +0200, Jehan-Guillaume de Rorthais wrote:
> > Unless I am missing something, another solution might be to use a dedicated
> > role to pg_receive{xlog|wal} with synchronous_commit lower than
> > remote_apply.  
> 
> Aren't you confused by the same thing as I was upthread [1]?
> [1]: https://www.postgresql.org/message-id/20190710080423.GG1031@paquier.xyz
> 
> remote_apply affects all sessions.  So even if you use a replication
> role with synchronous_commit = on and have pg_receivewal use that with
> remote_apply set in postgresql.conf, then remote_apply is effective
> for all the other sessions so these will still be stuck at commit
> waiting for pg_receivewal to apply WAL if it is a synchronous
> standby.

Argh!

(Sorry for the noise)