On Sun, Nov 27, 2022 at 6:15 PM Ian Lawrence Barwick <barw...@gmail.com> wrote: > 2022年11月22日(火) 5:50 Laurenz Albe <laurenz.a...@cybertec.at>: > > On Mon, 2022-11-21 at 12:11 -0500, Tom Lane wrote: > > > Robert Haas <robertmh...@gmail.com> writes: > > > > The reason that I pushed back -- not as successfully as I would have > > > > liked -- on the changes to pg_stop_backup / pg_start_backup is that I > > > > know there are people using the old method successfully, and it's not > > > > just a 1:1 substitution. Here I don't, and it is. I'm totally open to > > > > the feedback that such people exist and to hearing why adopting one of > > > > the newer methods would be a problem for them, if that's the case. But > > > > if there's no evidence that such people exist or that changing is a > > > > problem for them, I don't think waiting 5 years on principle is good > > > > for the project. > > > > > > We make incompatible changes in every release; see the release notes. > > > Unless somebody can give a plausible use-case where this'd be a > > > difficult change to deal with, I concur that we don't need to > > > deprecate it ahead of time. > > > > Since I am the only one that seems to worry, I'll shut up. You are probably > > right that it the feature won't be missed by many users. > > FWIW, though I prefer to err on the side of caution when removing features > from anything, I am struggling to remember ever having used > "promote_trigger_file" > (or "trigger_file" as it was in Pg11 and earlier); grepping my private notes > brings up a single example from ca. 2012 when I appear to have been > experimenting with replication. > > On a quick web search, a large part of the results are related to its change > to a GUC in Pg 12 and/or commented out entries in sample postgresql.conf > files; > most of the rest seem to be blog articles/tutorials on setting up replication.
Thanks Ian, Laurenz, Robert, Tom for the discussion. Seems like we're all set then. Sorry for delaying over trivialities, but I have a couple more comments about the documentation before committing: "The trigger_file and promote_trigger_file have been removed." was missing some words. I've also added a sentence to say which releases were involved, to make it like nearby paragraphs about other obsolete stuff. The funny thing about that "obsolete" appendix is that it's intended as a URL-preserving way to document the recovery.conf file's demise in r12. It doesn't look like the right place to document ongoing changes to recovery-related GUCs in general. In this particular case it's sort of OK because the old trigger_file GUC was indeed in recovery.conf, so it's not completely irrelevant. So maybe it's OK? Perhaps in future, in a separate commit, we could have a new appendix "Obsolete settings" that has an alphabetical list of the fallen? The main documentation of pg_promote() etc now has "The parameter <varname>promote_trigger_file</varname> has been removed" in the places where the GUC was previously mentioned. Perhaps we should just remove the mentions completely (it's somehow either too much and not enough information), but I'm OK with leaving those in for now until some better place exists for historical notes. So this is the version I will push unless someone sees anything more to tweak about the documentation.
From 15da25b8c46e3e2a57431784d07bb87c6cce5e9f Mon Sep 17 00:00:00 2001 From: Thomas Munro <tmu...@postgresql.org> Date: Mon, 28 Nov 2022 10:50:44 +1300 Subject: [PATCH v12] Remove promote_trigger_file. Previously, an idle startup (recovery) process would wake up every 5 seconds to have a chance to poll for promote_trigger_file, even if that GUC was not configured. That promotion triggering mechanism was effectively superseded by pg_ctl promote and pg_promote() a long time ago. There probably aren't many users left and it's very easy to change to the modern mechanisms, so we agreed to remove the feature. This is part of a campaign to reduce wakeups on idle systems. Author: Simon Riggs <simon.ri...@enterprisedb.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> Reviewed-by: Robert Haas <robertmh...@gmail.com> Reviewed-by: Thomas Munro <thomas.mu...@gmail.com> Discussion: https://postgr.es/m/CANbhV-FsjnzVOQGBpQ589%3DnWuL1Ex0Ykn74Nh1hEjp2usZSR5g%40mail.gmail.com --- .../appendix-obsolete-recovery-config.sgml | 12 +++---- doc/src/sgml/config.sgml | 18 ----------- doc/src/sgml/high-availability.sgml | 24 ++++++-------- src/backend/access/transam/xlogrecovery.c | 32 ++++--------------- src/backend/utils/misc/guc_tables.c | 10 ------ src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/access/xlogrecovery.h | 1 - 7 files changed, 20 insertions(+), 78 deletions(-) diff --git a/doc/src/sgml/appendix-obsolete-recovery-config.sgml b/doc/src/sgml/appendix-obsolete-recovery-config.sgml index 1cf4913114..2414b5e9ec 100644 --- a/doc/src/sgml/appendix-obsolete-recovery-config.sgml +++ b/doc/src/sgml/appendix-obsolete-recovery-config.sgml @@ -34,14 +34,10 @@ </para> <para> - The - <literal>trigger_file</literal> - <indexterm> - <primary>trigger_file</primary> - <see>promote_trigger_file</see> - </indexterm> - setting has been renamed to - <xref linkend="guc-promote-trigger-file"/>. + PostgreSQL 15 and below had a setting + <literal>promote_trigger_file</literal>, and in PostgreSQL 12 it was called <literal>trigger_file</literal>. + Use <command>pg_ctl promote</command> or call + <function>pg_promote()</function> to promote a standby instead. </para> <para> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 24b1624bad..baa9b5e712 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4610,24 +4610,6 @@ ANY <replaceable class="parameter">num_sync</replaceable> ( <replaceable class=" </listitem> </varlistentry> - <varlistentry id="guc-promote-trigger-file" xreflabel="promote_trigger_file"> - <term><varname>promote_trigger_file</varname> (<type>string</type>) - <indexterm> - <primary><varname>promote_trigger_file</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - Specifies a trigger file whose presence ends recovery in the - standby. Even if this value is not set, you can still promote - the standby using <command>pg_ctl promote</command> or calling - <function>pg_promote()</function>. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - </listitem> - </varlistentry> - <varlistentry id="guc-hot-standby" xreflabel="hot_standby"> <term><varname>hot_standby</varname> (<type>boolean</type>) <indexterm> diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index b2b3129397..cac8c71a44 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -653,11 +653,11 @@ protocol to make nodes agree on a serializable transactional order. <para> Standby mode is exited and the server switches to normal operation - when <command>pg_ctl promote</command> is run, - <function>pg_promote()</function> is called, or a trigger file is found - (<varname>promote_trigger_file</varname>). Before failover, - any WAL immediately available in the archive or in <filename>pg_wal</filename> will be - restored, but no attempt is made to connect to the primary. + when <command>pg_ctl promote</command> is run, or + <function>pg_promote()</function> is called. The parameter + <varname>promote_trigger_file</varname> has been removed. Before failover, + any WAL immediately available in the archive or in <filename>pg_wal</filename> + will be restored, but no attempt is made to connect to the primary. </para> </sect2> @@ -1483,15 +1483,11 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)' <para> To trigger failover of a log-shipping standby server, run - <command>pg_ctl promote</command>, call <function>pg_promote()</function>, - or create a trigger file with the file name and path specified by the - <varname>promote_trigger_file</varname>. If you're planning to use - <command>pg_ctl promote</command> or to call - <function>pg_promote()</function> to fail over, - <varname>promote_trigger_file</varname> is not required. If you're - setting up the reporting servers that are only used to offload read-only - queries from the primary, not for high availability purposes, you don't - need to promote it. + <command>pg_ctl promote</command> or call <function>pg_promote()</function>. + The parameter <varname>promote_trigger_file</varname> has been removed. + If you're setting up the reporting servers that are only used to offload + read-only queries from the primary, not for high availability purposes, + you don't need to promote it. </para> </sect1> diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index cb07694aea..b1008c3701 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -95,7 +95,6 @@ int recovery_min_apply_delay = 0; /* options formerly taken from recovery.conf for XLOG streaming */ char *PrimaryConnInfo = NULL; char *PrimarySlotName = NULL; -char *PromoteTriggerFile = NULL; bool wal_receiver_create_temp_slot = false; /* @@ -3840,14 +3839,14 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess, XLogPrefetcherComputeStats(xlogprefetcher); /* - * Wait for more WAL to arrive. Time out after 5 seconds - * to react to a trigger file promptly and to check if the - * WAL receiver is still active. + * Wait for more WAL to arrive, when we will be woken + * immediately by the WAL receiver. Use of trigger file + * via promote_trigger_file is now fully removed. */ (void) WaitLatch(&XLogRecoveryCtl->recoveryWakeupLatch, - WL_LATCH_SET | WL_TIMEOUT | - WL_EXIT_ON_PM_DEATH, - 5000L, WAIT_EVENT_RECOVERY_WAL_STREAM); + WL_LATCH_SET | WL_EXIT_ON_PM_DEATH, + -1L, + WAIT_EVENT_RECOVERY_WAL_STREAM); ResetLatch(&XLogRecoveryCtl->recoveryWakeupLatch); break; } @@ -4300,8 +4299,6 @@ SetPromoteIsTriggered(void) static bool CheckForStandbyTrigger(void) { - struct stat stat_buf; - if (LocalPromoteIsTriggered) return true; @@ -4314,23 +4311,6 @@ CheckForStandbyTrigger(void) return true; } - if (PromoteTriggerFile == NULL || strcmp(PromoteTriggerFile, "") == 0) - return false; - - if (stat(PromoteTriggerFile, &stat_buf) == 0) - { - ereport(LOG, - (errmsg("promote trigger file found: %s", PromoteTriggerFile))); - unlink(PromoteTriggerFile); - SetPromoteIsTriggered(); - return true; - } - else if (errno != ENOENT) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat promote trigger file \"%s\": %m", - PromoteTriggerFile))); - return false; } diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 349dd6a537..1bf14eec66 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -3819,16 +3819,6 @@ struct config_string ConfigureNamesString[] = check_recovery_target_lsn, assign_recovery_target_lsn, NULL }, - { - {"promote_trigger_file", PGC_SIGHUP, REPLICATION_STANDBY, - gettext_noop("Specifies a file name whose presence ends recovery in the standby."), - NULL - }, - &PromoteTriggerFile, - "", - NULL, NULL, NULL - }, - { {"primary_conninfo", PGC_SIGHUP, REPLICATION_STANDBY, gettext_noop("Sets the connection string to be used to connect to the sending server."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 868d21c351..043864597f 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -331,7 +331,6 @@ #primary_conninfo = '' # connection string to sending server #primary_slot_name = '' # replication slot on sending server -#promote_trigger_file = '' # file name whose presence ends recovery #hot_standby = on # "off" disallows queries during recovery # (change requires restart) #max_standby_archive_delay = 30s # max delay before canceling queries diff --git a/src/include/access/xlogrecovery.h b/src/include/access/xlogrecovery.h index 0e3e246bd2..f3398425d8 100644 --- a/src/include/access/xlogrecovery.h +++ b/src/include/access/xlogrecovery.h @@ -65,7 +65,6 @@ extern PGDLLIMPORT TimestampTz recoveryTargetTime; extern PGDLLIMPORT const char *recoveryTargetName; extern PGDLLIMPORT XLogRecPtr recoveryTargetLSN; extern PGDLLIMPORT RecoveryTargetType recoveryTarget; -extern PGDLLIMPORT char *PromoteTriggerFile; extern PGDLLIMPORT bool wal_receiver_create_temp_slot; extern PGDLLIMPORT RecoveryTargetTimeLineGoal recoveryTargetTimeLineGoal; extern PGDLLIMPORT TimeLineID recoveryTargetTLIRequested; -- 2.38.1
