On Friday, July 19, 2013 1:33 AM Alvaro Herrera wrote: > Fujii Masao escribió: > > On Fri, Jul 19, 2013 at 2:45 AM, Josh Berkus <j...@agliodbs.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 this will be removed from patch, once we implement automatic parsing of config/postgresql.auto.conf after parsing postgresql.conf > 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. Okay, I understood your concern about checking during parsing to find error whether directory/file exist. In the next version, I will remove this error. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers