Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions

On 8/4/19 1:59 AM, Tom Lane wrote:> Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
>> On Fri, Aug 02, 2019 at 06:08:02PM -0700, Andres Freund wrote:
>>> We're WAY WAY past feature freeze. This isn't the time to rewrite guc.c,
>>> guc-file.l to be suitable for running outside of a backend environment.
>
>> Right. And even if we had the code, it's not quite backpatchable (which
>> we probably should do, considering this is a general ALTER SYSTEM issue,
>> so not pg12-only).
>
>> Not to mention there's no clear consensus this is actually desirable.
>> I'd argue forcing external tools (written in arbitrary language) to use
>> this library (written in C), just to modify a "stupid" text file is a
>> bit overkill. IMO duplicates don't make the file invalid, we should
>> handle that correctly/gracefully, so I don't see why external tools
>> could not simply append to the file. We can deduplicate the file when
>> starting the server, on ALTER SYSTEM, or some other time.
>
> Yup. I'd also point out that even if we had a command-line tool of this
> sort, there would be scenarios where it's not practical or not convenient
> to use. We need not go further than "my tool needs to work with existing
> PG releases" to think of good examples.

I suspect this hasn't been an issue before, simply because until the removal
of recovery.conf AFAIK there hasn't been a general use-case where you'd need
to modify pg.auto.conf while the server is not running. The use-case which now
exists (i.e. for writing replication configuration) is one where the tool will
need to be version-aware anyway (like pg_basebackup is), so I don't see that as
a particular deal-breaker.

But...

> I think we should just accept the facts on the ground, which are that
> some tools modify pg.auto.conf by appending to it

+1. It's just a text file...

> and say that that's supported as long as the file doesn't get unreasonably long.

Albeit with the caveat that the server should not be running.

Not sure how you define "unreasonably long" though.

> I'm not at all on board with inventing a requirement for pg.auto.conf
> to track its modification history. I don't buy that that's a
> widespread need in the first place; if I did buy it, that file
> itself is not where to keep the history; and in any case, it'd be
> a new feature and it's way too late for v12.

Yeah, that's way outside of the scope of this issue.

Regards

Ian Barwick

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