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

Hi,

On 2019-08-02 20:57:20 -0400, Stephen Frost wrote:
> No, I’m saying that we already *have* a library and we can add a few
> functions to it and if people want to leverage those functions then they
> can write glue code to do so, just like was done with libpq. The argument
> that “we shouldn’t put code into the common library because only tools
> written in C can use the common library” is what I was specifically taking
> exception with and your response doesn’t change my opinion of that argument
> one bit.

Wait, which library is this? And which code is suitable for being put in
a library right now?

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.

> Apparently I don’t have the experiences that you do as I’ve not seen a lot
> of systems which are constantly rewriting the conf file to the point where
> keeping the versions would be likely to add up to anything interesting.

Shrug. I've e.g. seen people continuously (every few minutes or so)
change autovacuum settings depending on load and observed response
times. Which isn't even a crazy thing to do.

> Designing the system around “well, we don’t think you’ll modify the file
> very much from an external tool, so we just won’t worry about it, but if
> you use alter system then we will clean things up” certainly doesn’t strike
> me as terribly principled.

Well. You shouldn't change postgresql.conf.auto while the server is
running, for fairly obvious reasons. Therefore external tools not using
ALTER SYSTEM only make sense when the server is not running. And I don't
think it's a crazy to assume that PG servers where you'd regularly
change the config are running most of the time.

And again, we're talking about v12 here. I don't think anybody is
arguing that we shouldn't provide library/commandline tools to make make
changes to postgresql.auto.conf conveniently possible without
duplicating lines. BUT not for v12, especially not because as the person
arguing for this, you've not provided a patch providing such a library.

> > Personally, I don’t buy the “run out of disk space” argument but if we are
> > > going to go there then we should apply it appropriately.
> > >
> > > Having the history of changes to auto.conf would actually be quite
> > useful,
> > > imv, and worth a bit of disk space (heck, it’s not exactly uncommon for
> > > people to keep their config files in git repos..). I’d suggest we also
> > > include the date/time of when the modification was made.
> >
> > That just seems like an entirely different project. It seems blindlingly
> > obvious that we can't keep the entire history in the file that we're
> > going to be parsing on a regular basis. Having some form of config
> > history tracking might be interesting, but I think it's utterly and
> > completely independent from what we need to fix for v12.

> We don’t parse the file on anything like a “regular” basis.

Well, everytime somebody does pg_reload_conf(), which for systems that
do frequent ALTER SYSTEMs, is kinda frequent too...

Greetings,

Andres Freund