Re: WIP: raise error when submitting invalid ALTER SYSTEM command

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Jordan Deitch" <jd(at)rsa(dot)pub>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: WIP: raise error when submitting invalid ALTER SYSTEM command
Date: 2019-10-09 05:31:13
Message-ID: 31791.1570599073@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Jordan Deitch" <jd(at)rsa(dot)pub> writes:
> ALTER SYSTEM currently does not raise error upon invalid entry.

You mean on invalid combinations of entries.

Take for example:
> alter system set superuser_reserved_connections = 10;
> ALTER SYSTEM
> alter system set max_connections = 5;
> ALTER SYSTEM

> The database will now fail to restart without manual intervention by way of editing the autoconf file (which says "# Do not edit this file manually!" on its first line).

Yeah. That's unfortunate, but ...

> The attached WIP patch is intended to raise an error on invalid ALTER SYSTEM commands before being written out to the filesystem. Hopefully this behavior is more intuitive.

There's no chance that you can make this work. We've had unpleasant
experiences with previous attempts to implement cross-checks between
GUC variables; in general, they created more problems than they fixed.

A specific issue with what you've got here is that it only checks values
that are proposed to be put into postgresql.auto.conf, without regard to
other value sources such as postgresql.conf or built-in defaults. You
also managed to break the system's defenses against invalid combinations
that arise from such other sources --- taking out those error checks in
PostmasterMain is completely unsafe.

Also, even if you believe that O(N^2) behavior isn't a problem, this
programming approach doesn't scale to cases where more than two variables
contribute to an issue. Somewhere around O(N^3) or O(N^4) there is
definitely going to be a threshold of pain. This aspect doesn't seem
that hard to fix ... but it's just an efficiency issue, and doesn't
speak at all to the fundamental problem that you don't have enough
visibility into what the next postmaster run will be seeing.

Also, from a code maintenance standpoint, having code in
AlterSystemSetConfigFile that tries to know all about not only specific
GUCs, but every possible combination of specific GUCs, is just not going
to be maintainable. (The real underlying problem there is that those
checks in PostmasterMain are merely the tip of the iceberg of error
conditions that might cause a postmaster startup failure.)

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Moon, Insung 2019-10-09 05:34:24 Re: Transparent Data Encryption (TDE) and encrypted files
Previous Message Tom Lane 2019-10-09 05:01:01 Re: pg_dump compatibility level / use create view instead of create table/rule