From: Ian Barwick <ian.barwick@2ndquadrant.com>
To: Stephen Frost <sfrost@snowman.net>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
Date: Wed, 19 Jun 2019 13:57:08 +0900
Message-ID: <2e9f82a8-27e8-60cf-7df3-616d6707eede@2ndquadrant.com> (raw)
In-Reply-To: <20190617154133.GF2480@tamriel.snowman.net>
References: <aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com>
<20190614160835.GF2480@tamriel.snowman.net>
<CAA4eK1JW9UfLOtMEJq5b_YcogbWdxzAGUzNi72F2Xx4r1_h6QA@mail.gmail.com>
<20190616171610.GO2480@tamriel.snowman.net>
<27dc9f8d-8d8f-9dd1-0494-5d8bd2f49a45@2ndquadrant.com>
<20190617154133.GF2480@tamriel.snowman.net>
n 6/18/19 12:41 AM, Stephen Frost wrote:
> Greetings,
>
> * Ian Barwick (ian.barwick@2ndquadrant.com) wrote
(...)
>> I suggest explicitly documenting postgresql.auto.conf behaviour (and the circumstances
>> where it's acceptable to modify it outside of ALTER SYSTEM calls) in the documentation
>> (and possibly in the code), so anyone writing utilities which need to
>> append to postgresql.auto.conf knows what the situation is.
>
> Yeah, I would think that, ideally, we'd have some code in the common
> library that other utilities could leverage and which the backend would
> also use.
So maybe something along the lines of creating a stripped-down variant of
AlterSystemSetConfigFile() (from "src/backend/utils/misc/guc.c") which can be
called from front-end code to safely modify .auto.conf while the server is *not*
running.
I'm not terribly familiar with the GUC code, but that would presumably mean making
parts of the GUC parsing/handling code linkable externally (ParseConfigFp() etc.)
as you'd need to parse the file before rewriting it. Something like (minimal
pseudo-code):
void
alter_system_set(char *name, char *value)
{
/*
* check here that the server is *not* running
*/
...
ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail)
...
/*
* some robust portable way of ensuring another process can't
* modify the file(s) until we're done
*/
lock_file(AutoConfFileName);
replace_auto_config_value(&head, &tail, name, value);
write_auto_conf_file(AutoConfTmpFileName, head)
durable_rename(AutoConfTmpFileName, AutoConfFileName, ERROR);
FreeConfigVariables(head);
unlock_file(AutoConfFileName);
}
I'm not sure how feasible it is to validate the provided parameter
without the server running, but if not possible, that's not any worse than the
status quo, i.e. the utility has to be trusted to write the correct parameters
to the file anyway.
The question is though - is this a change which is practical to make at this point
in the release cycle for Pg12?
Regards
Ian Barwick
--
Ian Barwick https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ian.barwick@2ndquadrant.com, sfrost@snowman.net, amit.kapila16@gmail.com
Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
In-Reply-To: <2e9f82a8-27e8-60cf-7df3-616d6707eede@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox