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

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

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

Peter Eisentraut-3
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.

Discussion: https://www.postgresql.org/message-id/flat/e445616d-023e-a268-8aa1-67b8b335340c%40pgmasters.net

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/e04a53a6071d13ef4a13a41c6419d8e14c30b95c

Modified Files
--------------
doc/src/sgml/config.sgml | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)

Reply | Threaded
Open this post in threaded view
|

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

Fujii Masao-2
On Mon, Sep 30, 2019 at 6:10 AM Peter Eisentraut <[hidden email]> 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

standby_parameters_document_v1.patch (788 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Fujii Masao-2
On Mon, Sep 30, 2019 at 10:14 PM Fujii Masao <[hidden email]> wrote:

>
> On Mon, Sep 30, 2019 at 6:10 AM Peter Eisentraut <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

Alvaro Herrera-9
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


Reply | Threaded
Open this post in threaded view
|

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

Fujii Masao-2
On Tue, Nov 5, 2019 at 10:28 PM Alvaro Herrera <[hidden email]> 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

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

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

Peter Eisentraut-6
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


Reply | Threaded
Open this post in threaded view
|

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

Fujii Masao-2
On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
<[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

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

> On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
> <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

Fujii Masao-2
On Fri, Nov 8, 2019 at 12:23 AM Peter Eisentraut
<[hidden email]> wrote:

>
> On 2019-11-06 10:49, Fujii Masao wrote:
> > On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
> > <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

David Steele
In reply to this post by Fujii Masao-2
On 11/6/19 4:49 AM, Fujii Masao wrote:

> On Wed, Nov 6, 2019 at 4:29 PM Peter Eisentraut
> <[hidden email]> 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
[hidden email]


Reply | Threaded
Open this post in threaded view
|

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

Peter Eisentraut-6
In reply to this post by Fujii Masao-2
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