pgsql: doc: Further clarify how recovery target parameters are applied

On Mon, Sep 30, 2019 at 6:10 AM Peter Eisentraut <peter@eisentraut.org> wrote:

doc: Further clarify how recovery target parameters are applied

Recovery target parameters are all applied even in standby mode. The
previous documentation mostly wished they were not but this was never
the case.

Thanks for improving the documentation.

+     In this mode, the parameters from both this section and <xref
+     linkend="runtime-config-wal-recovery-target"/> will be used.  Parameters
+     from <xref linkend="runtime-config-replication-standby"/> will not be
+     used.

The latter description seems not true. hot_standby and
max_standby_archive_delay in "runtime-config-replication-standby"
are used during archive recovery. So something like the following
change would need to be applied, I think. Patch attached.

      In this mode, the parameters from both this section and <xref
      linkend="runtime-config-wal-recovery-target"/> will be used.  Parameters
      from <xref linkend="runtime-config-replication-standby"/> will not be
-     used.
+     used (<varname>hot_standby</varname> and
+     <varname>max_standby_archive_delay</varname>).
     </para>

Regards,

--
Fujii Masao

On Mon, Sep 30, 2019 at 10:14 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Sep 30, 2019 at 6:10 AM Peter Eisentraut <peter@eisentraut.org> wrote:

doc: Further clarify how recovery target parameters are applied

Recovery target parameters are all applied even in standby mode. The
previous documentation mostly wished they were not but this was never
the case.

Thanks for improving the documentation.

+     In this mode, the parameters from both this section and <xref
+     linkend="runtime-config-wal-recovery-target"/> will be used.  Parameters
+     from <xref linkend="runtime-config-replication-standby"/> will not be
+     used.

The latter description seems not true. hot_standby and
max_standby_archive_delay in "runtime-config-replication-standby"
are used during archive recovery. So something like the following
change would need to be applied, I think. Patch attached.

In this mode, the parameters from both this section and <xref
linkend="runtime-config-wal-recovery-target"/> will be used.  Parameters
from <xref linkend="runtime-config-replication-standby"/> will not be
-     used.
+     used (<varname>hot_standby</varname> and
+     <varname>max_standby_archive_delay</varname>).
</para>

In the above change, I forgot to add "except" before "<varname>hot_standby"...

Now, not only hot_standby and max_standby_archive_delay but also
recovery_min_apply_delay from
<xref linkend="runtime-config-replication-standby"/> can be used in
archive recovery, thanks to commit ec1259e880. So I'm just inclined to
remove the description "Parameters from
<xref linkend="runtime-config-replication-standby"/> will not be used."
rather than to list all "exceptive" parameters.

Regards,

--
Fujii Masao

On 2019-Oct-18, Fujii Masao wrote:

Thanks for improving the documentation.

+     In this mode, the parameters from both this section and <xref
+     linkend="runtime-config-wal-recovery-target"/> will be used.  Parameters
+     from <xref linkend="runtime-config-replication-standby"/> will not be
+     used.

The latter description seems not true. hot_standby and
max_standby_archive_delay in "runtime-config-replication-standby"
are used during archive recovery. So something like the following
change would need to be applied, I think. Patch attached.

This has not been applied, right? Can you send a complete patch?

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

On Tue, Nov 5, 2019 at 10:28 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Oct-18, Fujii Masao wrote:

Thanks for improving the documentation.

+     In this mode, the parameters from both this section and <xref
+     linkend="runtime-config-wal-recovery-target"/> will be used.  Parameters
+     from <xref linkend="runtime-config-replication-standby"/> will not be
+     used.

The latter description seems not true. hot_standby and
max_standby_archive_delay in "runtime-config-replication-standby"
are used during archive recovery. So something like the following
change would need to be applied, I think. Patch attached.

This has not been applied, right?

Yes.

Can you send a complete patch?

Thanks for the ping!
Patch attached. As I argued upthread, IMO it's better to remove
the latter description from the doc and the patch does that.
Also the patch adds "mainly" into the former description.

Regards,

--
Fujii Masao

On 2019-11-06 05:48, Fujii Masao wrote:

Patch attached. As I argued upthread, IMO it's better to remove
the latter description from the doc and the patch does that.
Also the patch adds "mainly" into the former description.

I think we should list explicitly what is applied and what is not. This
is the reference documentation after all.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-11-06 05:48, Fujii Masao wrote:

Patch attached. As I argued upthread, IMO it's better to remove
the latter description from the doc and the patch does that.
Also the patch adds "mainly" into the former description.

I think we should list explicitly what is applied and what is not. This
is the reference documentation after all.

That's possible, but may make the document more complicated and confusing,
I'm afraid. Because the parameters used for archive recovery and standby mode
are not categorized cleanly. For example,

- Parameters from Section 19.6.1. "Sending Servers" will be used except
track_commit_timestamp (in standby mode) and except track_commit_timestamp
and wal_init_zero (in archive recovery)
- Parameters from Section 19.6.3. "Standby Servers" will be used in standby
mode, but will not used except hot_standby, recovery_min_apply_delay, etc
in archive recovery.
- Parameters from Section 19.5.3. "Archiving" will be used in both standby mode
and archive recovery, but except archive_timeout.

So we would need to list such several parameters like the above
and which might make the document more complicated. Also which would
make the maintenance of the document harder.

Or it might be better to add something like "This setting has no effect if
in standby mode / archive recovery" into the description of each parameter,
instead.

Regards,

--
Fujii Masao

On 2019-11-06 10:49, Fujii Masao wrote:

On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-11-06 05:48, Fujii Masao wrote:

Patch attached. As I argued upthread, IMO it's better to remove
the latter description from the doc and the patch does that.
Also the patch adds "mainly" into the former description.

I think we should list explicitly what is applied and what is not. This
is the reference documentation after all.

That's possible, but may make the document more complicated and confusing,
I'm afraid. Because the parameters used for archive recovery and standby mode
are not categorized cleanly.

After reading this again and again, I think the best fix to get PG12
into "correct" shape is to just remove the offending sentence:

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 46bc31de4c..fdebe9bf4b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3217,9 +3217,7 @@ <title>Archive Recovery</title>
       takes precedence.  Targeted recovery mode ends when the archived 
WAL is
       fully replayed, or when <varname>recovery_target</varname> is 
reached.
       In this mode, the parameters from both this section and <xref
-     linkend="runtime-config-wal-recovery-target"/> will be used. 
Parameters
-     from <xref linkend="runtime-config-replication-standby"/> will not be
-     used.
+     linkend="runtime-config-wal-recovery-target"/> will be used.
      </para>

<variablelist>

As we are discussing in other threads, there is further opportunity to
reorganize how these settings interact, but that's a different project.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Nov 8, 2019 at 12:23 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-11-06 10:49, Fujii Masao wrote:

On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-11-06 05:48, Fujii Masao wrote:

Patch attached. As I argued upthread, IMO it's better to remove
the latter description from the doc and the patch does that.
Also the patch adds "mainly" into the former description.

I think we should list explicitly what is applied and what is not. This
is the reference documentation after all.

That's possible, but may make the document more complicated and confusing,
I'm afraid. Because the parameters used for archive recovery and standby mode
are not categorized cleanly.

After reading this again and again, I think the best fix to get PG12
into "correct" shape is to just remove the offending sentence:

+1

As we are discussing in other threads, there is further opportunity to
reorganize how these settings interact, but that's a different project.

Yes.

Regards,

--
Fujii Masao

On 11/6/19 4:49 AM, Fujii Masao wrote:

On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-11-06 05:48, Fujii Masao wrote:

Patch attached. As I argued upthread, IMO it's better to remove
the latter description from the doc and the patch does that.
Also the patch adds "mainly" into the former description.

I think we should list explicitly what is applied and what is not. This
is the reference documentation after all.

Or it might be better to add something like "This setting has no effect if
in standby mode / archive recovery" into the description of each parameter,
instead.

+1. This will probably be easier to maintain and less confusing overall.

--
-David
david@pgmasters.net

On 2019-11-07 18:10, Fujii Masao wrote:

After reading this again and again, I think the best fix to get PG12
into "correct" shape is to just remove the offending sentence:

+1

done

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services