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

Greetings,

On Mon, Aug 5, 2019 at 14:11 Andres Freund <andres(at)anarazel(dot)de> wrote:

> On 2019-08-05 13:34:39 -0400, Stephen Frost wrote:
> > * Andres Freund (andres(at)anarazel(dot)de) wrote:
> > > On 2019-08-05 12:43:24 -0400, Tom Lane wrote:
> > > > On the other hand, people have also opined that they want full error
> > > > checking on the inserted values, and that seems out of reach with
> less
> > > > than a full running system (mumble extensions mumble).
> > >
> > > I think the error checking ought to be about as complete as the one we
> > > do during a normal postmaster startup. Afaict that requires loading
> > > shared_preload_library extensions, but does *not* require booting up
> far
> > > enough to run GUC checks in a context with database access.
> >
> > I'm not following this thread of the discussion.
>
> It's about the future, not v12.

I’m happy to chat about post-v12, certainly. As I said up thread, I get
that we are in this unfortunate situation for v12 and we can do what needs
doing here (where I agree with what Tom said, “a doc patch”- and with fixes
for ALTER SYSTEM to be in line with that doc patch, along with
pg_basebackup, should any changes be needed, of course).

> Where are you thinking this error checking would be happening?
>
> A hypothethical post v12 tool that can set config values with as much
> checking as feasible. The IMO most realistic tool to do so is postmaster
> itself, similar to it's already existing -C. Boot it up until
> shared_preload_libraries have been processed, run check hook(s) for the
> new value(s), change postgresql.auto.conf, shutdown.

That’s a nice idea but I don’t think it’s really necessary and I’m not sure
how useful this level of error checking would end up being as part of
pg_basebackup.

I can certainly see value in a tool that could be run to verify a
postgresql.conf+auto.conf is valid to the extent that we are able to do so,
since that could, ideally, be run by the init script system prior to a
restart to let the user know that their restart is likely to fail. Having
that be some option to the postmaster could work, as long as it is assured
to not do anything that would upset a running PG instance (like, say, try
to allocate shared buffers).

> You're not suggesting that pg_basebackup perform this error checking
> > after it modifies the file, are you..?
>
> Not at the moment, at least.

Since pg_basebackup runs, typically, on a server other than the one that PG
is running on, it certainly would have to have a way to at least disable
anything that caused it to try and load libraries on the destination side,
or do anything else that required something external in order to validate-
but frankly I don’t think it should ever be loading libraries that it has
no business with, not even if it means that the error checking of the
postgresql.conf would be wonderful.

Thanks,

Stephen

>