WIP: raise error when submitting invalid ALTER SYSTEM command

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

WIP: raise error when submitting invalid ALTER SYSTEM command

Jordan Deitch-2
Hi all,

ALTER SYSTEM currently does not raise error upon invalid entry. 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).

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.

Thanks
--
Jordan Deitch
https://id.rsa.pub/

001-alter-system-raise-error-on-invalid-combination.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Tom Lane-2
"Jordan Deitch" <[hidden email]> 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