Обсуждение: pgsql: doc: Further clarify how recovery target parameters are applied
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(-)
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