On Tue, Jun 25, 2019 at 7:31 AM Ian Barwick <ian.barw...@2ndquadrant.com> wrote: > > > In particular, in order to consider it unexpected, you have to suppose > >> that the content rules for postgresql.auto.conf are different from those > >> for postgresql.conf (wherein we clearly allow last-one-wins). Can you > >> point to any user-facing documentation that says that? > > > > The backend and frontend tools don't modify postgresql.conf, and we > > don't document how to modify postgresql.auto.conf at *all*, even though > > we clearly now expect tool authors to go modifying it so that they can > > provide the same capabilities that pg_basebackup does and which they > > used to through recovery.conf, so I don't really see that as being > > comparable. > > > > The only thing we used to have to go on was what ALTER SYSTEM did, and > > then pg_basebackup went and did something different, and enough so that > > they ended up conflicting with each other, leading to this discussion. > > Or looking at it from another perspective - previously there was no > particular use-case for appending to .auto.conf, until it (implicitly) > became the only way of doing what recovery.conf used to do, and happened to > expose the issue at hand. > > Leaving aside pg_basebackup and the whole issue of writing replication > configuration, .auto.conf remains a text file which could potentially > include duplicate entries, no matter how much we stipulate it shouldn't. > As-is, ALTER SYSTEM fails to deal with this case, which in my opinion > is a bug and a potential footgun which needs fixing. >
I think there is an agreement that we should change it to remove duplicates and add the new entry at the end. However, we have not reached an agreement on whether we should throw WARNING after removing duplicates. I think it is arguable that it was a bug in the first place in Alter System as there is no way the duplicate lines can be there in postgresql.auto.conf file before this feature or if someone ignores the Warning on top of that file. Having said that, I am in favor of this change for the HEAD, but not sure if we should backpatch the same as well by considering it as a bug-fix. > (Though we'll still need to define/provide a way of writing configuration > while the server is not running, which will be guaranteed to be read in last > when it starts up). > Can you once verify if the current way of writing to postgresql.auto.conf is safe in pg_basebackup? It should ensure that if there are any failures, partial wite problem while writing, then the old file remains intact. It is not clear to me if that is the case with the current code of pg_basebackup, however the same is ensured in Alter System code. Because, if we haven't ensured it then it is a problem for which we definitely need some fix. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
