On Fri, Jun 14, 2019 at 9:38 PM Stephen Frost <sfr...@snowman.net> wrote: > * Ian Barwick (ian.barw...@2ndquadrant.com) wrote: > > > > Note this issue is not specific to pg_basebackup, primary_conninfo (or any > > other settings > > formerly in recovery.conf), it has just manifested itself as the built-in > > toolset as of now > > provides a handy way of getting into this situation without too much effort > > (and any > > utilities which clone standbys and append the replication configuration to > > "postgresql.auto.conf" in lieu of creating "recovery.conf" will be prone to > > running into > > the same situation). > > This is absolutely the fault of the system for putting in multiple > entries into the auto.conf, which it wasn't ever written to handle. >
Right. I think if possible, it should use existing infrastructure to write to postgresql.auto.conf rather than inventing a new way to change it. Apart from this issue, if we support multiple ways to edit postgresql.auto.conf, we might end up with more problems like this in the future where one system is not aware of the way file being edited by another system. > > I had previously always assumed that ALTER SYSTEM would change the *last* > > occurrence for > > the parameter in "postgresql.auto.conf", in the same way you'd need to be > > sure to change > > the last occurrence in the normal configuration files, however this > > actually not the case - > > as per replace_auto_config_value() ( src/backend/utils/misc/guc.c ): > > > > /* Search the list for an existing match (we assume there's only one) */ > > > > the *first* match is replaced. > > > > Attached patch attempts to rectify this situation by having > > replace_auto_config_value() > > deleting any duplicate entries first, before making any changes to the last > > entry. > > While this might be a good belt-and-suspenders kind of change to > include, > Another possibility to do something on these lines is to extend Alter System Reset <config_param> to remove all the duplicate entries. Then the user has a way to remove all duplicate entries if any and set the new value. I think handling duplicate entries in *.auto.conf files is an enhancement of the existing system and there could be multiple things we can do there, so we shouldn't try to do that as a bug-fix. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
