Date: Sat, 3 Aug 2019 01:00:11 +0200
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Stephen Frost <sfrost@snowman.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Amit Kapila <amit.kapila16@gmail.com>,
	Ian Barwick <ian.barwick@2ndquadrant.com>,
	Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
Message-ID: <20190802230011.dqokdhjptykopxxc@development>
References: <21353.1561136112@sss.pgh.pa.us>
 <CA+TgmoZVx-vco-39EN9MxO9BaG-oVY+NuDeZPGdS=_bX5VH70w@mail.gmail.com>
 <20190624185647.GW2480@tamriel.snowman.net>
 <15032.1561403529@sss.pgh.pa.us>
 <20190624192014.GB2480@tamriel.snowman.net>
 <56aff4f3-dec8-f462-7feb-0938411b04a6@2ndquadrant.com>
 <CAA4eK1+JMm=tvx720y6jhD8pdzt=HdS1-TwQwkARib2pwK8EvQ@mail.gmail.com>
 <20190802222254.qsvqjtkwte464lu3@development>
 <31697.1564784856@sss.pgh.pa.us>
 <CAOuzzgqNyQM_z0biEEBVBK9WcaLmbCyWrd5xTdf-Oxb_9pntLA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAOuzzgqNyQM_z0biEEBVBK9WcaLmbCyWrd5xTdf-Oxb_9pntLA@mail.gmail.com>
User-Agent: NeoMutt/20180716-1444-295967
Precedence: bulk

On Fri, Aug 02, 2019 at 06:38:46PM -0400, Stephen Frost wrote:
>Greetings,
>
>On Fri, Aug 2, 2019 at 18:27 Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>> > There seems to be a consensus that this this not a pg_basebackup issue
>> > (i.e. duplicate values don't make the file invalid), and it should be
>> > handled in ALTER SYSTEM.
>>
>> Yeah.  I doubt pg_basebackup is the only actor that can create such
>> situations.
>>
>> > The proposal seems to be to run through the .auto.conf file, remove any
>> > duplicates, and append the new entry at the end. That seems reasonable.
>>
>> +1
>
>
>I disagree that this should only be addressed in alter system, as I’ve said
>before and as others have agreed with.  Having one set of code that can be
>used to update parameters in the auto.conf and then have that be used by
>pg_basebackup, alter system, and external tools, is the right approach.
>
>The idea that alter system should be the only thing that doesn’t just
>append changes to the file is just going to lead to confusion and bugs down
>the road.
>

I don't remember any suggestions ALTER SYSTEM should be the only thing
that can rewrite the config file, but maybe it's buried somewhere in the
thread history. The current proposal certainly does not prohibit any
external tool from doing so, it just says we should expect duplicates.

>As I said before, an alternative could be to make alter system simply
>always append and declare that to be the way to update parameters in the
>auto.conf.
>

That just seems strange, TBH.

>> There was a discussion whether to print warnings about the duplicates. I
>> > personally see not much point in doing that - if we consider duplicates
>> > to be expected, and if ALTER SYSTEM has the license to rework the config
>> > file any way it wants, why warn about it?
>>
>> Personally I agree that warnings are unnecessary.
>
>
>And at least Magnus and I disagree with that, as I recall from this
>thread.  Let’s have a clean and clear way to modify the auto.conf and have
>everything that touches the file update it in a consistent way.
>

Well, I personally don't feel very strongly about it. I think the
warnings will be a nuisance bothering people with expeced stuff, but I'm
not willing to fight against it.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services