On Wed, Nov 16, 2022 at 2:34 PM Simon Riggs <simon.ri...@enterprisedb.com> wrote: > > Reposting v6 now so that patch tester doesn't think this has failed > when the patch on other thread gets applied.
Intention of the patch, that is, to get rid of promote_trigger_file GUC sometime in future, looks good to me. However, the timeout change to 60 sec from 5 sec seems far-reaching. While it discourages the use of the GUC, it can impact many existing production servers that still rely on promote_trigger_file as it can increase the failover times impacting SLAs around failover. How about retaining 5 sec as-is and adding a WARNING in promote_trigger_file check/assign and in show GUC, in CheckForStandbyTrigger() whenever PromoteTriggerFile is detected and specifying about the depreciation in GUC's description? + * to react to a trigger file. Direct use of trigger file + * is now deprecated and the promote_trigger_file will be + * removed in a later release. I think, adding 'Direct use of trigger file .....' in a next line that starts with XXX (typically, this represents a TODO item) is good, no? Also, do we need to add a TODO in postgresql wiki (https://wiki.postgresql.org/wiki/Todo), possibly under a new section 'Deprecated Features' or 'Features To-Be-Removed In Near Future' or some other (hm, it seems too vague, but it starts to track such deprecated items), to not miss on removing the promote_trigger_file in future releases? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
