Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])

Fujii Masao escribió:
> On Fri, Jul 19, 2013 at 2:45 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> > On 07/18/2013 04:25 AM, Amit Kapila wrote:
> >> On Thursday, July 18, 2013 12:38 AM Josh Berkus wrote:
> >>> On 07/17/2013 12:01 PM, Alvaro Herrera wrote:
> >>>> Both of these seem like they would make troubleshooters' lives more
> >>>> difficult. I think we should just parse the auto file automatically
> >>>> after parsing postgresql.conf, without requiring the include
> >>> directive
> >>>> to be there.
> >>>
> >>> Wait, I thought the auto file was going into the conf.d directory?
> >>
> >> Auto file is going into config directory, but will that make any difference
> >> if we have to parse it automatically after postgresql.conf.
> >
> > Well, I thought that the whole conf.d directory automatically got parsed
> > after postgresql.conf. No?
>
> No, in the previous patch. We needed to set include_dir to 'config' (though
> that's the default).

I know this has been discussed already, but I want to do a quick summary
of existing proposals, point out their drawbacks, and present a proposal
of my own. Note I ignore the whole file-per-setting vs.
one-file-to-rule-them-all, because the latter has already been shot
down. So live ideas floated here are:

1. have a config/postgresql.auto.conf file, require include_dir or
include in postgresql.conf
This breaks if user removes the config/ directory, or if the user
removes the include_dir directive from postgresql.conf. ALTER SYSTEM
is in the business of doing mkdir() or failing altogether if the dir
is not present. Doesn't seem friendly.

2. have a conf.d/ directory, put the file therein; no include or
include_dir directive is necessary.
I think this is a separate patch and merits its own discussion. This
might be a good idea, but I don't think that this is the
way to implement ALTER SYSTEM. If users don't want to allow conf.d
they can remove it, but that would break ALTER SYSTEM unnecessarily.
Since they are separate features it seems to me that they should
function independently.

I think we should just put the config directives of ALTER SYSTEM into a
file, not dir, alongside postgresql.conf; I would suggest
postgresql.auto.conf. This file is parsed automatically after
postgresql.conf, without requiring an "include" directive in
postgresql.conf. If the user does not want that file, they can remove
it; but a future ALTER SYSTEM will create the file again. No need to
parse stuff to find out whether the directory exists, or the file
exists, or such nonsense.

I think the only drawback of this is that there's no way to disable
ALTER SYSTEM (i.e. there's no directory you can remove to prevent the
thing from working). But is this a desirable property? I mean, if we
want to disallow ALTER SYSTEM I think we should provide a direct way to
do that, perhaps allow_alter_system = off in postgresql.conf; but I
don't think we need to provide this, really, at least not in the first
implementation.

Note that the Debian packaging uses postgres-writable directories for
its configuration files, so the daemon is always able to create the file
in the config dir.

This seems the simplest way; config tools such as Puppet know they
always need to consider the postgresql.auto.conf file.

I think the whole business of parsing the file, and then verifying
whether the auto file has been parsed, is nonsensical and should be
removed from the patch.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services