Re: Continue work on changes to recovery.conf API

Hello

thank you for review!

>>  - if present both standby.signal and recovery.signal - we use standby mode
>>  (this design from previous thread)
>
> Didn't find the discussion on that and don't understand the reasons, but
> seems OK and easy to change if we don't like it.
I meant this was implemented in previous proposed patch and i do not changed this. I didn't find the discussion on that
too...

>>  - recovery_target (immediate), recovery_target_name, recovery_target_time, recovery_target_xid, recovery_target_lsn
arereplaced to recovery_target_type and recovery_target_value (was discussed and changed in previous thread)
 
>
> I think this was the major point of contention. I reread the old
> thread, and it's still not clear why we need to change this. _type and
> _value look like an EAV system to me. GUC variables should be
> verifiable independent of another variable. The other idea of using
> only one variable seems better, but using two variables seems like a
> poor compromise between one and five.

> No, they MUST be independently verifiable. The interactions between the check_xxx functions in this patch are utterly
unsafe.

Understood, thank you.
I will implement set of current five recovery_target* settings and would like to leave reorganization for futher
pathes.

Not sure about one thing. All recovery_target settings change one internal recoveryTarget variable. GUC infrastructure
willguarantee behavior if user erroneously set multiple different recovery_target*? I mean check_* and assign_* will be
calledin order and so latest config line wins?
 

>>  - trigger_file was renamed to promote_signal_file for clarify (my change, in prev thread was removed)
>
> OK to add the "promote" prefix, but I'd prefer to keep the "trigger"
> name. There is a lot of discussion and knowledge around that. Seems
> unnecessary to change.
Ok, will change to promote_trigger_file

>>  - pg_basebackup with -R (--write-recovery-conf) option will create standby.signal file and append primary_conninfo
andprimary_slot_name (if was used) to postgresql.auto.conf instead of writing new recovery.conf
 
>
> I wonder if that would cause any problems. The settings in
> postgresql.auto.conf are normally not PGC_POSTMASTER, otherwise you
> couldn't use ALTER SYSTEM to put them there. Maybe it's OK.
Actually we can use ALTER SYSTEM to put PGC_POSTMASTER settings. I tried now "alter system set max_connections to
300;",postgresql.auto.conf was changed and affect max_connections during database start.
 
Of course, we can not reload PGC_POSTMASTER settings, but during start we call regular ParseConfigFile and can override
PGC_POSTMASTER.

regards, Sergei