On Thu, Sep 1, 2016 at 4:01 AM, Abhijit Menon-Sen <a...@2ndquadrant.com> wrote: > At 2016-08-31 17:15:59 +0100, si...@2ndquadrant.com wrote: >> >> * Recovery parameters would now be part of the main postgresql.conf >> infrastructure >> Any parameters set in $DATADIR/recovery.conf will be read after the >> main parameter file, similar to the way that postgresql.conf.auto is >> read. >> (Abhijit) >> >> * Parameters >> All of the parameters formerly set in recovery.conf can be set in >> postgresql.conf using RELOAD >> These parameters will have no defaults in postgresql.conf.sample >> Setting them has no effect during normal running, or once recovery ends. >> https://www.postgresql.org/docs/devel/static/archive-recovery-settings.html >> https://www.postgresql.org/docs/devel/static/recovery-target-settings.html >> https://www.postgresql.org/docs/devel/static/standby-settings.html >> (Abhijit) > > I've attached a WIP patch for the above (recovery_guc_v20160831.patch). > This was based on the unite_recoveryconf_postgresqlconf_v3.patch posted > by Fujii Masao. > > Unfortunately, some parts conflict with the patch that Simon just posted > (e.g., his patch removes trigger_file altogether, whereas mine converts > it into a GUC along the lines of the original patch). Rather than trying > to untangle that right now, I'm posting what I have as-is, and I'll post > an updated version tomorrow.
- else if (recoveryTarget == RECOVERY_TARGET_XID) - ereport(LOG, - (errmsg("starting point-in-time recovery to XID %u", - recoveryTargetXid))); User loses information if those logs are removed. + {"recovery_min_apply_delay", PGC_SIGHUP, WAL_RECOVERY_TARGET, + gettext_noop("Sets the minimum delay to apply changes during recovery."), + NULL, + GUC_UNIT_MS + }, What's the point of having them all as SIGHUP? The startup process does not reload GUC parameters with ProcessConfigFile(), it works on recoveryWakeupLatch though. I'd rather let them as PGC_POSTMASTER to begin with as that's the safest approach, and then get a second patch that processes ProcessConfigFile in the startup process and switch some of them to PGC_SIGHUP. Recovery targets should not be SIGHUP, but recovery_min_apply_delay applies definitely applies to that, as well as archive_cleanup_command or restore_command. +static void +assign_recovery_target_name(const char *newval, void *extra) +{ + if (recovery_target_xid != InvalidTransactionId) + recovery_target = RECOVERY_TARGET_XID; + else if (newval[0]) + recovery_target = RECOVERY_TARGET_NAME; + else if (recovery_target_time != 0) + recovery_target = RECOVERY_TARGET_TIME; + else + recovery_target = RECOVERY_TARGET_UNSET; +} That's brittle to put that in the GUC machinery... The recovery target type should be determined only once, if archive recovery is wanted at the beginning of the startup process once and for all, and just be set up within the startup process. If multiple recovery_target_* parameters are set, we should just define the target type in order of priority, instead of the-last-one-wins that is currently present. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers