Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
To: Magnus Hagander <magnus@hagander.net>, Stephen Frost <sfrost@snowman.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Amit Kapila <amit.kapila16@gmail.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: <aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com>
 <20190614160835.GF2480@tamriel.snowman.net>
 <CAA4eK1JW9UfLOtMEJq5b_YcogbWdxzAGUzNi72F2Xx4r1_h6QA@mail.gmail.com>
 <20190616171610.GO2480@tamriel.snowman.net> <5218.1560705699@sss.pgh.pa.us>
 <20190616174332.GR2480@tamriel.snowman.net>
 <CABUevEzejvEMR9BgHvYyv1mXcHbs9+x8LZv-UkUohwTg9o7o6A@mail.gmail.com>
From: Ian Barwick <ian.barwick@2ndquadrant.com>
Message-ID: <952db979-def2-11f4-0bab-780cf5fe688b@2ndquadrant.com>
Date: Mon, 17 Jun 2019 23:51:38 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: 
 <CABUevEzejvEMR9BgHvYyv1mXcHbs9+x8LZv-UkUohwTg9o7o6A@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 6/17/19 2:58 AM, Magnus Hagander wrote:
> On Sun, Jun 16, 2019 at 7:43 PM Stephen Frost <sfrost@snowman.net <mailto:sfrost@snowman.net>> wrote:
> 
> 
>     * Tom Lane (tgl@sss.pgh.pa.us <mailto:tgl@sss.pgh.pa.us>) wrote:
>      > Stephen Frost <sfrost@snowman.net <mailto:sfrost@snowman.net>> writes:
> 
>      > > what we should do is clean them up (and possibly
>      > > throw a WARNING or similar at the user saying "something modified your
>      > > postgresql.auto.conf in an unexpected way").  I'd suggest we do that on
>      > > every ALTER SYSTEM call.
>      >
>      > +1 for having ALTER SYSTEM clean out duplicates.  Not sure whether
>      > a WARNING would seem too in-your-face.
> 
>     I'd hope for a warning from basically every part of the system when it
>     detects, clearly, that a file was changed in a way that it shouldn't
>     have been.  If we don't throw a warning, then we're implying that it's
>     acceptable, but then cleaning up the duplicates, which seems pretty
>     confusing.
> 
> > +1. Silently "fixing" the file by cleaning up duplicates is going to be even
 >  more confusing o uses who had seen them be there before.

Some sort of notification is definitely appropriate here.

However, going back to the original scenario (cascaded standby set up using
"pg_basebackup --write-recovery-conf") there would now be a warning emitted
the first time anyone executes ALTER SYSTEM (about duplicate "primary_conninfo"
entries) which would not have occured in Pg11 and earlier (and which will
no doubt cause consternation along the lines "how did my postgresql.auto.conf
get modified in an unexpected way? OMG? Bug? Was I hacked?").


Regards

Ian Barwick
-- 
  Ian Barwick                   https://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services