Обсуждение: Why conf.d should be default, and auto.conf and recovery.conf should be in it

Hackers,

ALTER SYSTEM SET has been committed and recovery.conf GUCs are being
reviewed.  I'm going to make a last case for conf.d in relation to these
two patches before 9.4 goes out the door.

In 9.3, we added support for a config directory (conf.d), but have it
disabled by default.  For tool authors, this makes conf.d useless since
you never know, on any given installation, whether it will be
present/enabled or not.  While we don't want to prevent users from
disabling it, conf.d only becomes useful if it's present by default.
There's a simple reason why: if you want to write a tool which manages
postgresql.conf, you don't want the user to have to manually edit
postgresql.conf (and create a directory) in order to enable the tool.

I'm particularly thinking about this in relation to the merger of
recovery.conf and postgresql.conf.  There are several tools already
(RepMgr, OminPITR, HandyRep, pgPool, etc.) which manage recovery.conf
separately from postgresql.conf.  These tools will want to continue
managing recovery.conf as a separate file, even if it's /included in
postgresql.conf now.

If conf.d exists by default, and is enabled in postgresql.conf by
default, this is easy: the tool just drops a recovery.conf file into
conf.d.  Changing file locations and variable names is a fairly simple
exercise in backwards compatibility.

If conf.d does NOT exist by default, things become complicated, and
backwards compatibility for replication management tools becomes much
harder.

Yes, I'm also arguing that postgresql.auto.conf should go into conf.d.
I said I'd bring that up again after ALTER SYSTEM SET was committed, and
here it is.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Josh,

* Josh Berkus (josh@agliodbs.com) wrote:
> In 9.3, we added support for a config directory (conf.d), but have it
> disabled by default.  For tool authors, this makes conf.d useless since
> you never know, on any given installation, whether it will be
> present/enabled or not.  While we don't want to prevent users from
> disabling it, conf.d only becomes useful if it's present by default.
> There's a simple reason why: if you want to write a tool which manages
> postgresql.conf, you don't want the user to have to manually edit
> postgresql.conf (and create a directory) in order to enable the tool.

I don't buy this argument one bit- certainly, on Debian, the defaults
are overridden for a number of variables already and could be done to
enable a conf.d directory as well.  Directory creation would, of
course, also be able to be handled by the Debian package.  Any tool
which is being packaged for Debian would simply have to Depend on the
version of the PG package that enabled the conf.d option by default.

This doesn't deal with upgrade cases or where the user has disabled the
feature and so the tool would need to check for a directory's
existance, but changing our default isn't going to completely address
that issue either.  Indeed, the Debian package would at least be able to
indicate, through the existance or not of the directory, whether a given
cluster was set up by default with the conf.d structure or not.

> I'm particularly thinking about this in relation to the merger of
> recovery.conf and postgresql.conf.  There are several tools already
> (RepMgr, OminPITR, HandyRep, pgPool, etc.) which manage recovery.conf
> separately from postgresql.conf.  These tools will want to continue
> managing recovery.conf as a separate file, even if it's /included in
> postgresql.conf now.

Certainly an interesting thought.

> If conf.d exists by default, and is enabled in postgresql.conf by
> default, this is easy: the tool just drops a recovery.conf file into
> conf.d.  Changing file locations and variable names is a fairly simple
> exercise in backwards compatibility.

This isn't quite that simple on at least Debian, and I believe RH, as
there's going to be multiple directories involved (one for each cluster
which exists on the system).  Also, there are rules which packages need
to follow on Debian regarding conf file changes, which these certainly
would be.

> If conf.d does NOT exist by default, things become complicated, and
> backwards compatibility for replication management tools becomes much
> harder.

My recommendation would be to argue the point with the package
maintainers.  Even if we *had* a default, I'm not convinced that the
package maintainers would keep it that way until and unless they update
their scripts and whatnot to handle it.

> Yes, I'm also arguing that postgresql.auto.conf should go into conf.d.
> I said I'd bring that up again after ALTER SYSTEM SET was committed, and
> here it is.

I disagree with enabling that by default.
Thanks,
    Stephen

On 1/15/14, 1:53 PM, Josh Berkus wrote:
> I'm particularly thinking about this in relation to the merger of
> recovery.conf and postgresql.conf.  There are several tools already
> (RepMgr, OminPITR, HandyRep, pgPool, etc.) which manage recovery.conf
> separately from postgresql.conf.  These tools will want to continue
> managing recovery.conf as a separate file, even if it's /included in
> postgresql.conf now.

That seems like a very general argument.  We'd need to consider exactly
what these tools want to do, and how that affects the recovery.conf
merger.  My personal proposal was to allow postgresql.conf to contain
recovery parameters, but read recovery.conf last anyway.  That would
solve that problem.

> If conf.d exists by default, and is enabled in postgresql.conf by
> default, this is easy: the tool just drops a recovery.conf file into
> conf.d.

That would just give false hope, I think.  My standard operating
procedure is to delete the default postgresql.conf and write a new one
from scratch (or have deployment tools do it).  I don't want to
subscribe to a policy that the only endorsed way to maintain
postgresql.conf is to start with the default one and edit around carefully.

The only way you could achieve your goal is to read the configuration
directory automatically, but that would come with its own set of problems.

> Yes, I'm also arguing that postgresql.auto.conf should go into conf.d.
> I said I'd bring that up again after ALTER SYSTEM SET was committed, and
> here it is.

Independent of the above, I don't agree with that.  postgresql.auto.conf
is a special thing and should have its own special place.  For once
thing, when putting configuration files in a separate directory
structure (/etc/ vs /var), then postgresql.auto.conf should stay in the
data directory.

On 01/15/2014 11:10 AM, Stephen Frost wrote:
> I don't buy this argument one bit- certainly, on Debian, the defaults
> are overridden for a number of variables already and could be done to
> enable a conf.d directory as well.  Directory creation would, of
> course, also be able to be handled by the Debian package.  Any tool
> which is being packaged for Debian would simply have to Depend on the
> version of the PG package that enabled the conf.d option by default.

However, Debian is *never* going to add conf.d to the packages if we
don't recommend it as an upstream project.  And, frankly, I use the
apt.postgresql.org packages anyway, which would certainly include
anything which was decided on this list.

> This doesn't deal with upgrade cases or where the user has disabled the
> feature and so the tool would need to check for a directory's
> existance, but changing our default isn't going to completely address
> that issue either.  

There's a HUGE difference between "This tool depends on conf.d, so
please don't disable it" and "please edit postgresql.conf in the
following way, and create this directory with these permissions ..."

>> I'm particularly thinking about this in relation to the merger of
>> recovery.conf and postgresql.conf.  There are several tools already
>> (RepMgr, OminPITR, HandyRep, pgPool, etc.) which manage recovery.conf
>> separately from postgresql.conf.  These tools will want to continue
>> managing recovery.conf as a separate file, even if it's /included in
>> postgresql.conf now.
> 
> Certainly an interesting thought.

It's more than in interesting thought.  It's the difference between
having 20 lines of backwards compatibility code, and having 150 plus a
bunch of additional user documentation and setup.

>> If conf.d exists by default, and is enabled in postgresql.conf by
>> default, this is easy: the tool just drops a recovery.conf file into
>> conf.d.  Changing file locations and variable names is a fairly simple
>> exercise in backwards compatibility.
> 
> This isn't quite that simple on at least Debian, and I believe RH, as
> there's going to be multiple directories involved (one for each cluster
> which exists on the system).  Also, there are rules which packages need
> to follow on Debian regarding conf file changes, which these certainly
> would be.

Again, from the perspective of the tool (assuming it already supports
Debian), this is just a directory change.  ("if version == 9.4,
directory = directory + "/conf.d/")

> My recommendation would be to argue the point with the package
> maintainers.  Even if we *had* a default, I'm not convinced that the
> package maintainers would keep it that way until and unless they update
> their scripts and whatnot to handle it.

This is disengenuous.  The package maintainers are never, ever going to
add conf.d and enable it by default unless it's the official
recommendation of the PostgreSQL project that they do so.  Exactly how
far would I get with "I couldn't get pgsql-hackers to agree to this, so
I'm asking you"?

On 01/15/2014 11:40 AM, Peter Eisentraut wrote:
> That seems like a very general argument.  We'd need to consider exactly
> what these tools want to do, and how that affects the recovery.conf
> merger.  My personal proposal was to allow postgresql.conf to contain
> recovery parameters, but read recovery.conf last anyway.  That would
> solve that problem.

Three issues:

a) if postgresql is still going to look for a recovery.conf file in the
usual place, but we are changing the names and meaning of some of the
parameters, then aren't we making the upgrade problem much worse?

b) what if the admin *does* want to disable reading recovery.conf in
order to prevent old utilities from mistakenly including one?  How will
they do that?

c) would this be in the configdir, datadir, or both?

I'd thought that part of the idea of the merger was to remove the
"magic" status of recovery.conf.

>> If conf.d exists by default, and is enabled in postgresql.conf by
>> default, this is easy: the tool just drops a recovery.conf file into
>> conf.d.
>
> That would just give false hope, I think.  My standard operating
> procedure is to delete the default postgresql.conf and write a new one
> from scratch (or have deployment tools do it).  I don't want to
> subscribe to a policy that the only endorsed way to maintain
> postgresql.conf is to start with the default one and edit around
carefully.
>
> The only way you could achieve your goal is to read the configuration
> directory automatically, but that would come with its own set of problems.

Frankly, I'm not concerned about people who rewrite their
postgresql.conf from scratch already; it's *easy* to tell those people
to re-add the conf.d reference to that file.  I'm talking about the vast
majority of our users who never edit pg.conf by hand.

> Independent of the above, I don't agree with that.  postgresql.auto.conf
> is a special thing and should have its own special place.  For once
> thing, when putting configuration files in a separate directory
> structure (/etc/ vs /var), then postgresql.auto.conf should stay in the
> data directory.

Ah, I'd forgotten about that line of argument.  Where is auto.conf now?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Josh,

* Josh Berkus (josh@agliodbs.com) wrote:
> However, Debian is *never* going to add conf.d to the packages if we
> don't recommend it as an upstream project.  And, frankly, I use the
> apt.postgresql.org packages anyway, which would certainly include
> anything which was decided on this list.

Those are both categorial false claims.  Debian certainly does not ship
with 'trust' auth, nor do our PGDG packages.  They also move the conf
files out of the data directory.  Last, but not least, they absolutely
enable conf.d directories even when that is not the default upstream.

Additionally, I fully expect and hope that the apt.postgresql.org
packages to follow the Debian/Ubuntu package management- having those
diverge would absolutely be a horrible idea and cause a great deal of
trouble for our users.  Ideally, we can all agree, but this notion that
the PGDG must follow what happens on -hackers is simply wrong, we need
include and coordinate with the OS package maintainers.

> It's more than in interesting thought.  It's the difference between
> having 20 lines of backwards compatibility code, and having 150 plus a
> bunch of additional user documentation and setup.

If I was writing the tool, I'm pretty sure that I'd be writing all that
code either way.
Thanks,
    Stephen

Peter Eisentraut <peter_e@gmx.net> writes:
> On 1/15/14, 1:53 PM, Josh Berkus wrote:
>> Yes, I'm also arguing that postgresql.auto.conf should go into conf.d.
>> I said I'd bring that up again after ALTER SYSTEM SET was committed, and
>> here it is.

> Independent of the above, I don't agree with that.  postgresql.auto.conf
> is a special thing and should have its own special place.  For once
> thing, when putting configuration files in a separate directory
> structure (/etc/ vs /var), then postgresql.auto.conf should stay in the
> data directory.

It seems like we still aren't all on the same page as to whether the
conf.d directory (and contained files) should be expected to be writable
by the postgres server or not.  I think it's hopeless to proceed further
unless there's a strong consensus on that.

My vote would be that the server should *not* be expected to be able to
write these files.  It might physically be able to, in a quick-and-dirty
installation, but in a setup prepared by a packaging system I'd not
expect that the config files would be postgres-owned.

Given that assumption, it's clear that postgresql.auto.conf can't live
in conf.d.  However, if you reject that assumption then maybe it should.
        regards, tom lane

On 1/15/14, 4:35 PM, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> On 1/15/14, 1:53 PM, Josh Berkus wrote:
>>> Yes, I'm also arguing that postgresql.auto.conf should go into conf.d.
>>> I said I'd bring that up again after ALTER SYSTEM SET was committed, and
>>> here it is.
> 
>> Independent of the above, I don't agree with that.  postgresql.auto.conf
>> is a special thing and should have its own special place.  For once
>> thing, when putting configuration files in a separate directory
>> structure (/etc/ vs /var), then postgresql.auto.conf should stay in the
>> data directory.
> 
> It seems like we still aren't all on the same page as to whether the
> conf.d directory (and contained files) should be expected to be writable
> by the postgres server or not.  I think it's hopeless to proceed further
> unless there's a strong consensus on that.

You can turn this around and ask, why should it be writable?  The server
has no need to write anything there.

In my mind, a conf.d directory is an extension of a single-file
configuration, and so it should be handled that way.

On 1/15/14, 3:01 PM, Josh Berkus wrote:
> Three issues:
> 
> a) if postgresql is still going to look for a recovery.conf file in the
> usual place, but we are changing the names and meaning of some of the
> parameters, then aren't we making the upgrade problem much worse?

That assumes that we are changing the names and meanings of some of the
parameters, which I don't see a reason for.

> b) what if the admin *does* want to disable reading recovery.conf in
> order to prevent old utilities from mistakenly including one?  How will
> they do that?

That assumes that there is a reason for doing that, which goes away if
point (a) is addressed.

> c) would this be in the configdir, datadir, or both?

That might depend on the parameter and what a tool wants to do with it.

There is also the consideration of whether some of those tools couldn't
be changed to use ALTER SYSTEM.

> I'd thought that part of the idea of the merger was to remove the
> "magic" status of recovery.conf.

Well, clearly, everyone has their own ideas about that.  I have several
non-overlapping ones of my own. ;-)  But my point is that we should look
what actually comes out of that discussion before we start designing
other facilities that interact with it.

* Peter Eisentraut (peter_e@gmx.net) wrote:
> In my mind, a conf.d directory is an extension of a single-file
> configuration, and so it should be handled that way.

I'm apparently out on some funny limb with this thought, but I'll throw
it out there anyway- in my head, the 'postgresql.auto.conf' thing that
essentially ends up included as part of 'postgresql.conf' should be
handled the same way a single 'postgresql.conf' or 'conf.d' directory
is.  Now, I've never particularly agreed with it, but at least on
Debian/Ubuntu, the /etc conf directories are owned by the postgres user
by default.  I dislike the idea of separating the PG config into things
in /etc and things in PGDATA as it will make life more difficult for the
poor sysadmins trying to figure out what's going on.
Thanks,
    Stephen

On Thu, Jan 16, 2014 at 1:31 AM, Josh Berkus <josh@agliodbs.com> wrote:
> On 01/15/2014 11:10 AM, Stephen Frost wrote:
>
>> Independent of the above, I don't agree with that.  postgresql.auto.conf
>> is a special thing and should have its own special place.  For once
>> thing, when putting configuration files in a separate directory
>> structure (/etc/ vs /var), then postgresql.auto.conf should stay in the
>> data directory.
>
> Ah, I'd forgotten about that line of argument.  Where is auto.conf now?
  Data Directory

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 01/16/2014 02:53 AM, Josh Berkus wrote:
> Hackers,
> 
> ALTER SYSTEM SET has been committed and recovery.conf GUCs are being
> reviewed.  I'm going to make a last case for conf.d in relation to these
> two patches before 9.4 goes out the door.

Personally, I'd find conf.d greatly more useful when enabled by default.
The need to hack postgresql.conf to use it eliminates half the point of
having it.

I haven't seen any compelling reason why conf.d should NOT be enabled by
default.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

On 1/15/14, 11:23 PM, Stephen Frost wrote:
> * Peter Eisentraut (peter_e@gmx.net) wrote:
>> In my mind, a conf.d directory is an extension of a single-file
>> configuration, and so it should be handled that way.
> 
> I'm apparently out on some funny limb with this thought, but I'll throw
> it out there anyway- in my head, the 'postgresql.auto.conf' thing that
> essentially ends up included as part of 'postgresql.conf' should be
> handled the same way a single 'postgresql.conf' or 'conf.d' directory
> is.

Then one might as well argue that the pg_db_role_setting table be
relocated to /etc.  It's the same facility, only on a slightly different
level.  The fact that postgresql.auto.conf looks the same as a
plain-text configuration file is an implementation detail.  We could
have chosen some binary format instead.

Peter Eisentraut wrote:

> In my mind, a conf.d directory is an extension of a single-file
> configuration, and so it should be handled that way.

+1 on this.  This means

1. it is to be read automatically by the server without need for an  "include_dir conf.d" option in the main
postgresql.conf.

2. it is to be placed alongside postgresql.conf, not necessarily in  PGDATA.

3. it might or might not be writable by the running server; it's an  operating-system-owned configuration location.

4. there is no point in "disabling" it, and thus we offer no mechanism  to do that.

5. its entries override what is set in postgresql.conf, and are in turn  overridden by what is set in
postgresql.auto.conf.


Taken together, these properties guarantee that it's an useful mechanism
to be used by system-level deployment/configuration tools such as Puppet
et al.


I also think, but this is a secondary point, that initdb should write
its modified settings into a file in conf.d instead of generating a
custom postgresql.conf.

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

Peter,

* Peter Eisentraut (peter_e@gmx.net) wrote:
> Then one might as well argue that the pg_db_role_setting table be
> relocated to /etc.  It's the same facility, only on a slightly different
> level.  The fact that postgresql.auto.conf looks the same as a
> plain-text configuration file is an implementation detail.  We could
> have chosen some binary format instead.

I argued quite a bit that we should be looking at moving more things
into the catalog tables and pulling them out of postgresql.conf,
reducing that down to just what's required to be there, rather than this
nasty hack where SQL commands are modifying text config files.
Thanks,
    Stephen

On 2014-01-16 08:34:23 -0500, Stephen Frost wrote:
> Peter,
> 
> * Peter Eisentraut (peter_e@gmx.net) wrote:
> > Then one might as well argue that the pg_db_role_setting table be
> > relocated to /etc.  It's the same facility, only on a slightly different
> > level.  The fact that postgresql.auto.conf looks the same as a
> > plain-text configuration file is an implementation detail.  We could
> > have chosen some binary format instead.
> 
> I argued quite a bit that we should be looking at moving more things
> into the catalog tables and pulling them out of postgresql.conf,
> reducing that down to just what's required to be there, rather than this
> nasty hack where SQL commands are modifying text config files.

Given that the majority didn't seem to be convinced by this and that the
feature was written differently this isn't a convincing argument for the
location of the file given the current feature, no?

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

Hi Alvaro,

On 16/01/14 10:21, Alvaro Herrera wrote:
> 1. it is to be read automatically by the server without need for an
>    "include_dir conf.d" option in the main postgresql.conf.

+1

> 4. there is no point in "disabling" it, and thus we offer no mechanism
>    to do that.

Not only there is „no point“ in disabling it, it makes this feature
nearly useless. One can't rely on it if the distro may disable
it. There are so many out there, it will never be a reliable feature
if it can be disabled.

> 5. its entries override what is set in postgresql.conf, and are in turn
>    overridden by what is set in postgresql.auto.conf.

+1

Best regards,

-- Christian Kruse               http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> Peter Eisentraut wrote:
> > In my mind, a conf.d directory is an extension of a single-file
> > configuration, and so it should be handled that way.
>
> +1 on this.  This means
>
> 1. it is to be read automatically by the server without need for an
>    "include_dir conf.d" option in the main postgresql.conf.

I am not thrilled with the idea that we're claiming ownership of the
directory which postgresql.conf happens to reside in and you had better
not have any 'conf.d' directory in there unless it's the one for PG.

> 2. it is to be placed alongside postgresql.conf, not necessarily in
>    PGDATA.

I certainly agree with this idea- but I feel it should be configurable.
The path which is passed in should be relative to the location of
postgresql.conf.

> 3. it might or might not be writable by the running server; it's an
>    operating-system-owned configuration location.

Works well enough, I suppose.

> 4. there is no point in "disabling" it, and thus we offer no mechanism
>    to do that.

No.  There should be a way to disable it.

> 5. its entries override what is set in postgresql.conf, and are in turn
>    overridden by what is set in postgresql.auto.conf.

For my part, I'd rather my configurations be deterministic and would
prefer that we error on obvious misconfigurations where values are set
and then reset.

> Taken together, these properties guarantee that it's an useful mechanism
> to be used by system-level deployment/configuration tools such as Puppet
> et al.

It will be a useful mechanism for puppet even with the ability to
disable it in postgresql.conf (you're probably managing that with puppet
also, or keeping it at the system default which will likely include the
include_dir option by default- but, yes, you'd need to check that) and
without the "override previous definition" option.

> I also think, but this is a secondary point, that initdb should write
> its modified settings into a file in conf.d instead of generating a
> custom postgresql.conf.

This makes sense.
Thanks,
    Stephen

* Andres Freund (andres@2ndquadrant.com) wrote:
> Given that the majority didn't seem to be convinced by this and that the
> feature was written differently this isn't a convincing argument for the
> location of the file given the current feature, no?

I'll start by pointing out that the only reason this ALTER SYSTEM thing
exists is because there are options that we can't set in the catalogs.

Now, I can't claim to conjecture on what the majority opinion is when it
disagreed with my own.  I remain of the opinion that having settings
which admins are familiar with being overridden by some file in the data
directory is going to cause confusion and problems.  Therefore, I feel
that the 'auto' file should be where postgresql.conf lives because
admins will at least have some hope of finding it there (and then
realizing they need to disable that functionality and lock down the
postgresql.conf permissions to be root-owned...).

Having the log files be sufficiently detailed to help an admin is better
than nothing, but it certainly won't be as good as having the config
files in the same directory when the admin is trying to figure out why
the cluster won't start any more.
Thanks,
    Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
>> 1. it is to be read automatically by the server without need for an
>> "include_dir conf.d" option in the main postgresql.conf.

> I am not thrilled with the idea that we're claiming ownership of the
> directory which postgresql.conf happens to reside in and you had better
> not have any 'conf.d' directory in there unless it's the one for PG.

If we were to do this, I'd argue that the location of this hard-wired
config directory ought to be selected by a configure option.   And
in fact, I'd argue that the default behavior with no such option be
that there's no such hard-wired directory.  That puts it entirely
on the packager to decide what makes sense as a location.  In the end
it's going to be the packager's decision anyway --- it's just a matter
of how painful we make it for him to configure it to his distro's
conventions.

On the whole though, I find the argument that we can't configure this
in postgresql.conf to be exceedingly weak.  In particular, the idea
that you can puppet-ify a configuration without any knowledge of the
distro you're targeting is ridiculous on its face, as is the idea
that we/postgres can dictate configuration file location conventions
to packagers who have distro rules to follow.  There isn't going to
be a "one location to rule them all", and that means the argument
that a location determined by postgresql.conf is too unstable does
not really hold water.

Another point here is that hard-wiring a config directory location
into the executables completely breaks many scenarios for running
multiple clusters with the same executables.  Yeah, maybe you'd
like to share most of the same config across all your clusters.
But then again, maybe you wouldn't.  The proposed behavior would
make it practically impossible for a test cluster to not pick up
random subsets of the system primary cluster's configuration.
        regards, tom lane

Stephen Frost wrote:
> * Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> > Peter Eisentraut wrote:
> > > In my mind, a conf.d directory is an extension of a single-file
> > > configuration, and so it should be handled that way.
> > 
> > +1 on this.  This means
> > 
> > 1. it is to be read automatically by the server without need for an
> >    "include_dir conf.d" option in the main postgresql.conf.
> 
> I am not thrilled with the idea that we're claiming ownership of the
> directory which postgresql.conf happens to reside in and you had better
> not have any 'conf.d' directory in there unless it's the one for PG.

Please explain.  I don't see a reason not to.  Are you saying you expect
to put, say, Apache configuration files in the same directory as
PostgreSQL's?  That doesn't sound really tenable to me, and it doesn't
sounds like what any sane admin would do.

In /etc you have /etc/postfix/ where you have Postfix files, and
/etc/exim4 where you have Exim files, and /etc/apache2 or whatever.  And
if each of those software packages uses a "conf.d" or "conf-enabled"
directory, they use a hardcoded name for a place inside their own
subdirectory, not one directly in /etc.

What you don't have is /etc/exim4.conf and /etc/postfix.conf and
/etc/apache.conf, where each of those files specifies that I would
please like to have extra files loaded from /etc/exim.conf.d and
/etc/apache.conf.d, and that packages please figure out by parsing my
config file in whatever random format I have invented to figure out
where snippets are to be stored.

> > 2. it is to be placed alongside postgresql.conf, not necessarily in
> >    PGDATA.
> 
> I certainly agree with this idea- but I feel it should be configurable.
> The path which is passed in should be relative to the location of
> postgresql.conf.

What would be the point of having it be configurable?

> > 4. there is no point in "disabling" it, and thus we offer no mechanism
> >    to do that.
> 
> No.  There should be a way to disable it.

What's you rationale for this?

> > 5. its entries override what is set in postgresql.conf, and are in turn
> >    overridden by what is set in postgresql.auto.conf.
> 
> For my part, I'd rather my configurations be deterministic and would
> prefer that we error on obvious misconfigurations where values are set
> and then reset.

Any convention we establish will make configurations deterministic.
Random variations such as making the conf.d directory be configurable
would make them non-deterministic.  ISTM you're contradicting yourself
here.

> > Taken together, these properties guarantee that it's an useful mechanism
> > to be used by system-level deployment/configuration tools such as Puppet
> > et al.
> 
> It will be a useful mechanism for puppet even with the ability to
> disable it in postgresql.conf (you're probably managing that with puppet
> also, or keeping it at the system default which will likely include the
> include_dir option by default- but, yes, you'd need to check that) and
> without the "override previous definition" option.

Meh.  External packages (say, postgis) will be able to add their own
files into conf.d without having to check what its actual location is,
and without having to check whether the thing is enabled in the first
place.  If we make it disable-able, packages will not be able to do
that; or rather, they will be able to add files, but they will have no
effect.

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

On 2014-01-16 11:12:55 -0500, Tom Lane wrote:
> If we were to do this, I'd argue that the location of this hard-wired
> config directory ought to be selected by a configure option.
> in fact, I'd argue that the default behavior with no such option be
> that there's no such hard-wired directory.  That puts it entirely
> on the packager to decide what makes sense as a location.  In the end
> it's going to be the packager's decision anyway --- it's just a matter
> of how painful we make it for him to configure it to his distro's
> conventions.

I don't think a configure option is going to help much to reach that
goal - unless the distribution only allows a single cluster and a single
version to be installed anything but a path relative to $PGDATA won't
work well if configured statically.

So, if we want to support antything but a) relative to pgdata b)
relative to postgresql.conf it needs to be configurable at startup
time. Possibly with an added initdb switch to set the location.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

Avlaro,

* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> Please explain.  I don't see a reason not to.  Are you saying you expect
> to put, say, Apache configuration files in the same directory as
> PostgreSQL's?  That doesn't sound really tenable to me, and it doesn't
> sounds like what any sane admin would do.

That doesn't mean we get to dictate it.

> In /etc you have /etc/postfix/ where you have Postfix files, and
> /etc/exim4 where you have Exim files, and /etc/apache2 or whatever.  And
> if each of those software packages uses a "conf.d" or "conf-enabled"
> directory, they use a hardcoded name for a place inside their own
> subdirectory, not one directly in /etc.

I missed the part of this where you point out that Apache on Debian has
some kind of problem because it's possible for an admin to remove the
'IncludeDir sites-enabled' line from apache2.conf.

> What you don't have is /etc/exim4.conf and /etc/postfix.conf and
> /etc/apache.conf, where each of those files specifies that I would
> please like to have extra files loaded from /etc/exim.conf.d and
> /etc/apache.conf.d, and that packages please figure out by parsing my
> config file in whatever random format I have invented to figure out
> where snippets are to be stored.

Right, there's absolutely zero cases of /etc/*.conf files these days.
Nor are there any /etc/*.conf.d directories.

</sarcasm>

Sure, maybe we all feel that there should be an /etc/pam directory
instead of /etc/pam.conf and /etc/pam.d, but that doesn't make it so,
and we're only talking about a subset (perhaps a majority, but it's
certainly not a totality) of PG users- not everyone is using Debian or
RH or even *Linux* or GNU tools to run PG with.

> > > 2. it is to be placed alongside postgresql.conf, not necessarily in
> > >    PGDATA.
> >
> > I certainly agree with this idea- but I feel it should be configurable.
> > The path which is passed in should be relative to the location of
> > postgresql.conf.
>
> What would be the point of having it be configurable?

To allow admins to configure it?  Perhaps their policy is that it must
be postgresql.conf.d, or that they want it served off of an NFS mount
instead of being pulled from /etc, or maybe they only back up /srv but
they don't want to change where the package manager dumps the
postgresql.conf that's used for starting up the cluster?

> > > 4. there is no point in "disabling" it, and thus we offer no mechanism
> > >    to do that.
> >
> > No.  There should be a way to disable it.
>
> What's you rationale for this?

I don't want PG randomly guessing at directories to go poking around in
for config files.  It's also documentation.

> > > 5. its entries override what is set in postgresql.conf, and are in turn
> > >    overridden by what is set in postgresql.auto.conf.
> >
> > For my part, I'd rather my configurations be deterministic and would
> > prefer that we error on obvious misconfigurations where values are set
> > and then reset.
>
> Any convention we establish will make configurations deterministic.
> Random variations such as making the conf.d directory be configurable
> would make them non-deterministic.  ISTM you're contradicting yourself
> here.

Perhaps you see it that way, but I certainly don't.  Having conf.d be
configurable doesn't mean that we have one file with "work_mem = 1GB"
and another with "work_mem = 5GB" where it's not at all clear which one
is actually what gets used.

> > It will be a useful mechanism for puppet even with the ability to
> > disable it in postgresql.conf (you're probably managing that with puppet
> > also, or keeping it at the system default which will likely include the
> > include_dir option by default- but, yes, you'd need to check that) and
> > without the "override previous definition" option.
>
> Meh.  External packages (say, postgis) will be able to add their own
> files into conf.d without having to check what its actual location is,
> and without having to check whether the thing is enabled in the first
> place.  If we make it disable-able, packages will not be able to do
> that; or rather, they will be able to add files, but they will have no
> effect.

Which cluster directory is the packager of PostGIS going to put his
config snippets into, exactly?  Even if you force the hard-code conf.d
idea, the packager will still need to figure out what clusters exist.

Although, if you could make the conf.d directory configurable, and have
more than one, then we actually could have a "system-wide" directory and
then a per-cluster directory, which *would* make things easier for
packagers.  But, oh wait, don't forget that the "system-wide" one would
still need to be major-version-specific...
Thanks,
    Stephen

* Andres Freund (andres@2ndquadrant.com) wrote:
> So, if we want to support antything but a) relative to pgdata b)
> relative to postgresql.conf it needs to be configurable at startup
> time. Possibly with an added initdb switch to set the location.

How would an initdb switch be better than an option in postgresql.conf?
That sounds very backwards to me.  If you meant a *postmaster*/pg_ctl
option, that would make some sense to me, for folks who would like to
avoid having a single postgresql.conf at all...  Don't know if that
would work today or not tho.
Thanks,
    Stephen

On 2014-01-16 11:35:00 -0500, Stephen Frost wrote:
> * Andres Freund (andres@2ndquadrant.com) wrote:
> > So, if we want to support antything but a) relative to pgdata b)
> > relative to postgresql.conf it needs to be configurable at startup
> > time. Possibly with an added initdb switch to set the location.
> 
> How would an initdb switch be better than an option in postgresql.conf?
> That sounds very backwards to me.  If you meant a *postmaster*/pg_ctl
> option, that would make some sense to me, for folks who would like to
> avoid having a single postgresql.conf at all...  Don't know if that
> would work today or not tho.

Oh, I was thinking of initdb option being complementary to a
postgresql.conf option, just setting the path in the generated
postgresql.conf. That way distributions wouldn't have to muck around in
the generated config anymore.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

* Andres Freund (andres@2ndquadrant.com) wrote:
> On 2014-01-16 11:35:00 -0500, Stephen Frost wrote:
> > * Andres Freund (andres@2ndquadrant.com) wrote:
> > > So, if we want to support antything but a) relative to pgdata b)
> > > relative to postgresql.conf it needs to be configurable at startup
> > > time. Possibly with an added initdb switch to set the location.
> >
> > How would an initdb switch be better than an option in postgresql.conf?
> > That sounds very backwards to me.  If you meant a *postmaster*/pg_ctl
> > option, that would make some sense to me, for folks who would like to
> > avoid having a single postgresql.conf at all...  Don't know if that
> > would work today or not tho.
>
> Oh, I was thinking of initdb option being complementary to a
> postgresql.conf option, just setting the path in the generated
> postgresql.conf. That way distributions wouldn't have to muck around in
> the generated config anymore.

Oh, sure, I can see that being useful.
Thanks,
    Stephen

Tom Lane wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> >> 1. it is to be read automatically by the server without need for an
> >> "include_dir conf.d" option in the main postgresql.conf.
> 
> > I am not thrilled with the idea that we're claiming ownership of the
> > directory which postgresql.conf happens to reside in and you had better
> > not have any 'conf.d' directory in there unless it's the one for PG.
> 
> If we were to do this, I'd argue that the location of this hard-wired
> config directory ought to be selected by a configure option.   And
> in fact, I'd argue that the default behavior with no such option be
> that there's no such hard-wired directory.  That puts it entirely
> on the packager to decide what makes sense as a location.

I fail to see the sense in this.  Surely the only useful option is to
use a relative path: an absolute path is completely useless because then
there's no way to have multiple clusters using the same executables, as
you say.  Since it has to be relative to *something*, there aren't that
many options: we could make it relative to the directory where the
executables reside (which would be absolutely pointless), or relative to
the data directory (which we already ruled out for reasons well
understood), or relative to the directory containing the other
configuration files.

> On the whole though, I find the argument that we can't configure this
> in postgresql.conf to be exceedingly weak.  In particular, the idea
> that you can puppet-ify a configuration without any knowledge of the
> distro you're targeting is ridiculous on its face,

Surely puppetification is not the only use case for conf.d.  For
example, suppose for a minute that a module providing an extension wants
to offer a program to initialize its configuration file into some
configuration directory (say, an hypothetical PostGIS package drops a
"postgis.conf" into conf.d with initial values for all the config
options it provides).  If you're not administering postgresql.conf with
puppet, you'd like to be able to do this to the clusters you have
without having to figure out *where* to put the files (that is, without
having to parse and possibly modify postgresql.conf).  Just dump it in
the cluster's conf.d.

> as is the idea that we/postgres can dictate configuration file
> location conventions to packagers who have distro rules to follow.

I don't see that this is ridiculous at all.  Configuration files always
go together; a distro rule that some config files go in some place and
other configuration files go somewhere else, sounds more ridiculous to
me, actually.

> There isn't going to be a "one location to rule them all", and that
> means the argument that a location determined by postgresql.conf is
> too unstable does not really hold water.

postgresql.conf's location already rules them all.  Remember, my
proposal upthread was to have conf.d be located in the same directory
that holds postgresql.conf.


> Another point here is that hard-wiring a config directory location
> into the executables completely breaks many scenarios for running
> multiple clusters with the same executables.

Therefore the proposal is not to hardwire the location in the
executables.

A server with multiple clusters using the same executables needs a way
to specify different ports for each (at the very least); therefore they
already have separate postgresql.conf.  Each directory containing such
file can contain a conf.d subdirectory with extra stuff perfectly well.


Stephen Frost wrote:

> * Alvaro Herrera (alvherre@2ndquadrant.com) wrote:

> > In /etc you have /etc/postfix/ where you have Postfix files, and
> > /etc/exim4 where you have Exim files, and /etc/apache2 or whatever.  And
> > if each of those software packages uses a "conf.d" or "conf-enabled"
> > directory, they use a hardcoded name for a place inside their own
> > subdirectory, not one directly in /etc.
> 
> I missed the part of this where you point out that Apache on Debian has
> some kind of problem because it's possible for an admin to remove the
> 'IncludeDir sites-enabled' line from apache2.conf.

I don't see that this is parallel.  For one thing, I don't think you
would normally have multiple apache2.conf files.  In the other hand,
even if you had more than one, surely the one that's managed by the
packaging system, if there is one, is not going to want to have that
line changed, because that would immediately break all modules installed
by packages.


> > What you don't have is /etc/exim4.conf and /etc/postfix.conf and
> > /etc/apache.conf, where each of those files specifies that I would
> > please like to have extra files loaded from /etc/exim.conf.d and
> > /etc/apache.conf.d, and that packages please figure out by parsing my
> > config file in whatever random format I have invented to figure out
> > where snippets are to be stored.
> 
> Right, there's absolutely zero cases of /etc/*.conf files these days.
> Nor are there any /etc/*.conf.d directories.
> 
> </sarcasm>

I just checked my system, and it seems to me that most cases that have
.conf files and something.conf.d directly in /etc are more because of
historical reasons than because they have made a conscious, contemporary
decision to do that.  Do we want to replicate choices made in the
1990's?

> Sure, maybe we all feel that there should be an /etc/pam directory
> instead of /etc/pam.conf and /etc/pam.d, but that doesn't make it so,
> and we're only talking about a subset (perhaps a majority, but it's
> certainly not a totality) of PG users- not everyone is using Debian or
> RH or even *Linux* or GNU tools to run PG with.

Yeah, so instead of making a rational choice for people using all
platforms, mirroring the rational choice made by Linux/Debian/GNU, lets
make an irrational choice so that everybody will be constantly bitten by
it, regardless of platform.

</sarcasm>

> > > > 2. it is to be placed alongside postgresql.conf, not necessarily in
> > > >    PGDATA.
> > > 
> > > I certainly agree with this idea- but I feel it should be configurable.
> > > The path which is passed in should be relative to the location of
> > > postgresql.conf.
> > 
> > What would be the point of having it be configurable?
> 
> To allow admins to configure it?  Perhaps their policy is that it must
> be postgresql.conf.d

This example sounds rather weak.  I mean, if they *must* have
/etc/postgresql/9.2/main/postgresql.conf.d instead of
/etc/postgresql/9.2/main/conf.d, why aren't they forced to have 

Now, if we want to hardcode the name postgresql.conf.d instead of just
conf.d, that's fine too.  Just have *one* name and stick with it, not a
configurable choice which subsequently becomes a PITA.

> or that they want it served off of an NFS mount instead of being
> pulled from /etc,

So conf.d would be in an NFS mount but postgresql.conf would not?

> or maybe they only back up /srv but they don't want to change where
> the package manager dumps the postgresql.conf that's used for starting
> up the cluster?


> > > > 4. there is no point in "disabling" it, and thus we offer no mechanism
> > > >    to do that.
> > > 
> > > No.  There should be a way to disable it.
> > 
> > What's you rationale for this?
> 
> I don't want PG randomly guessing at directories to go poking around in
> for config files.  It's also documentation.

Documentation is fine.  We can have a comment at the top of
postgresql.conf saying "also see files in postgresql.conf.d/" or some
such.  I can hardly thing that opening and reading a subdirectory named
conf.d or postgresql.conf.d, *in the same directory as postgresql.conf*
is "randomly poking".


> > > > 5. its entries override what is set in postgresql.conf, and are in turn
> > > >    overridden by what is set in postgresql.auto.conf.
> > > 
> > > For my part, I'd rather my configurations be deterministic and would
> > > prefer that we error on obvious misconfigurations where values are set
> > > and then reset.
> > 
> > Any convention we establish will make configurations deterministic.
> > Random variations such as making the conf.d directory be configurable
> > would make them non-deterministic.  ISTM you're contradicting yourself
> > here.
> 
> Perhaps you see it that way, but I certainly don't.  Having conf.d be
> configurable doesn't mean that we have one file with "work_mem = 1GB"
> and another with "work_mem = 5GB" where it's not at all clear which one
> is actually what gets used.

There must be a way to jump over the chasm of logic like you did in the
above paragraph, but I don't see it.

If you want to argue that multiple lines setting the same variable
should raise an error instead of "last definition wins", please do
propose that separately.


> > > It will be a useful mechanism for puppet even with the ability to
> > > disable it in postgresql.conf (you're probably managing that with puppet
> > > also, or keeping it at the system default which will likely include the
> > > include_dir option by default- but, yes, you'd need to check that) and
> > > without the "override previous definition" option.
> > 
> > Meh.  External packages (say, postgis) will be able to add their own
> > files into conf.d without having to check what its actual location is,
> > and without having to check whether the thing is enabled in the first
> > place.  If we make it disable-able, packages will not be able to do
> > that; or rather, they will be able to add files, but they will have no
> > effect.
> 
> Which cluster directory is the packager of PostGIS going to put his
> config snippets into, exactly?  Even if you force the hard-code conf.d
> idea, the packager will still need to figure out what clusters exist.

This was a thinko on my part, I admit, which doesn't invalidate the rest
of my argumentation.  I think the way to think about this would be
something like having mode in pg_ctlcluster (for example) that enables
postgis for a certain cluster, which means it drops a certain file in
the cluster's conf.d directory, as I described above.

> Although, if you could make the conf.d directory configurable, and have
> more than one, then we actually could have a "system-wide" directory and
> then a per-cluster directory, which *would* make things easier for
> packagers.  But, oh wait, don't forget that the "system-wide" one would
> still need to be major-version-specific...

Hmm.  I guess you could have /etc/postgresql/9.2/conf.d which is used
for all clusters, yeah.  The need for this is not as clear as a
per-cluster conf.d though, and IMO would be a separate proposal.

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

* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> Stephen Frost wrote:
> > I missed the part of this where you point out that Apache on Debian has
> > some kind of problem because it's possible for an admin to remove the
> > 'IncludeDir sites-enabled' line from apache2.conf.
>
> I don't see that this is parallel.  For one thing, I don't think you
> would normally have multiple apache2.conf files.  In the other hand,
> even if you had more than one, surely the one that's managed by the
> packaging system, if there is one, is not going to want to have that
> line changed, because that would immediately break all modules installed
> by packages.

My point above was that having it configurable does *not* make it a
burden on packagers or tool authors.  If it did, the Debian folks would
be complaining to ASF to have the option removed and instead required.
Indeed, the fact that it's configurable, in my view, is how we ended up
with sites-available and sites-enabled in the first place.  Perhaps we'd
have gotten there without it, but it sure feels like having the option
was a help and not a hindrence.

> I just checked my system, and it seems to me that most cases that have
> .conf files and something.conf.d directly in /etc are more because of
> historical reasons than because they have made a conscious, contemporary
> decision to do that.  Do we want to replicate choices made in the
> 1990's?

There are people who are still running OS's from the 1990's and forcing
what we feel is the 'right' answer on users is helping ourselves more
than helping our users, imv.

> Yeah, so instead of making a rational choice for people using all
> platforms, mirroring the rational choice made by Linux/Debian/GNU, lets
> make an irrational choice so that everybody will be constantly bitten by
> it, regardless of platform.

This argument that it only works if it's hard-coded simply doesn't fly
when you look at the existing examples in Debian.

> > To allow admins to configure it?  Perhaps their policy is that it must
> > be postgresql.conf.d
>
> This example sounds rather weak.  I mean, if they *must* have
> /etc/postgresql/9.2/main/postgresql.conf.d instead of
> /etc/postgresql/9.2/main/conf.d, why aren't they forced to have

You're being completely Debian-centric here.  It's possible they've got
their own /etc directory structure which doesn't match your
expectations- and that's *fine* until we start forcing the issue on
them.  Perhaps they prefer to have /etc/postgresql.conf and
/etc/postgresql.conf.d, why should we be preventing them from doing so
just because the OS's and distro's that we run don't work that way?

> > or that they want it served off of an NFS mount instead of being
> > pulled from /etc,
>
> So conf.d would be in an NFS mount but postgresql.conf would not?

That's correct- and it wouldn't be mounted at
/etc/postgresql/9.2/main/conf.d, and please don't start in with the
symlinks argument- *that* would be a mess.

> > I don't want PG randomly guessing at directories to go poking around in
> > for config files.  It's also documentation.
>
> Documentation is fine.  We can have a comment at the top of
> postgresql.conf saying "also see files in postgresql.conf.d/" or some
> such.  I can hardly thing that opening and reading a subdirectory named
> conf.d or postgresql.conf.d, *in the same directory as postgresql.conf*
> is "randomly poking".

It's completely random when we have no idea where the postgesql.conf is
nor what other files or directories exist there today.  Perhaps it's
100% true that, no where on this planet, a postgresql.conf file and a
conf.d directory/file/socket/whateve share a directory, but I wouldn't
bet on it.  Hell, I'm half tempted to go make sure just to be sure to
prove anyone wrong who argues that case. ;)

Remember- these would not just be new installs, someone simply running
pg_upgrade to move to 9.4 would suddenly find their database server
trying to open a non-existant directory wherever their postgresql.conf
file resides.

Another issue is how does this scale up to directories for pg_hba.conf
or pg_ident.conf or friends- are we going to hard-code directories for
those too?

> If you want to argue that multiple lines setting the same variable
> should raise an error instead of "last definition wins", please do
> propose that separately.

I've brought it up a number of times before- having the lines be in
different files just makes it that much worse.

> > Which cluster directory is the packager of PostGIS going to put his
> > config snippets into, exactly?  Even if you force the hard-code conf.d
> > idea, the packager will still need to figure out what clusters exist.
>
> This was a thinko on my part, I admit, which doesn't invalidate the rest
> of my argumentation.  I think the way to think about this would be
> something like having mode in pg_ctlcluster (for example) that enables
> postgis for a certain cluster, which means it drops a certain file in
> the cluster's conf.d directory, as I described above.

Boy does that ever sound hokey.

> > Although, if you could make the conf.d directory configurable, and have
> > more than one, then we actually could have a "system-wide" directory and
> > then a per-cluster directory, which *would* make things easier for
> > packagers.  But, oh wait, don't forget that the "system-wide" one would
> > still need to be major-version-specific...
>
> Hmm.  I guess you could have /etc/postgresql/9.2/conf.d which is used
> for all clusters, yeah.  The need for this is not as clear as a
> per-cluster conf.d though, and IMO would be a separate proposal.

Uh, ok, so now we're going to have an 'include_dir' option, but it can't
be used to change the default 'conf.d' and instead can only specify some
other directory to *also* check?
Thanks,
    Stephen

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> Another point here is that hard-wiring a config directory location
>> into the executables completely breaks many scenarios for running
>> multiple clusters with the same executables.

> Therefore the proposal is not to hardwire the location in the
> executables.

At least some people seem to be advocating that, even if I misunderstood
whether you were.

I'm fine if the proposal is that postgresql.conf include "include_dir
conf.d" by default (where that's read as relative to postgresql.conf's own
directory).  Even better if it's not terribly difficult for a packager to
change that, because I think some will want to.  We could possibly reduce
the need for packagers to change it if we made it be
"include_dir postgresql.d", because conf.d is a damn generic name for
something that might be in the same /etc directory as configs for other
packages.
        regards, tom lane

PS: off topic, but isn't ParseConfigDirectory leaking the result
of AbsoluteConfigLocation?  In both normal and error paths?

On 01/16/2014 10:46 AM, Tom Lane wrote:
> I'm fine if the proposal is that postgresql.conf include "include_dir
> conf.d" by default (where that's read as relative to postgresql.conf's own
> directory).  Even better if it's not terribly difficult for a packager to
> change that, because I think some will want to.  We could possibly reduce
> the need for packagers to change it if we made it be
> "include_dir postgresql.d", because conf.d is a damn generic name for
> something that might be in the same /etc directory as configs for other
> packages.

FWIW, this is what I was proposing.  We have an "include_dir
postgresql.conf.d" currently in the stock postgresql.conf, but it's
disabled (commented out) by default.  I'd just like it enabled by
default, and to pass a suggestion to the packagers that they pick an
appropriate directory and enable it by default.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

* Josh Berkus (josh@agliodbs.com) wrote:
> On 01/16/2014 10:46 AM, Tom Lane wrote:
> > I'm fine if the proposal is that postgresql.conf include "include_dir
> > conf.d" by default (where that's read as relative to postgresql.conf's own
> > directory).  Even better if it's not terribly difficult for a packager to
> > change that, because I think some will want to.  We could possibly reduce
> > the need for packagers to change it if we made it be
> > "include_dir postgresql.d", because conf.d is a damn generic name for
> > something that might be in the same /etc directory as configs for other
> > packages.
>
> FWIW, this is what I was proposing.  We have an "include_dir
> postgresql.conf.d" currently in the stock postgresql.conf, but it's
> disabled (commented out) by default.  I'd just like it enabled by
> default, and to pass a suggestion to the packagers that they pick an
> appropriate directory and enable it by default.

To this point- I've asked the Debian packager Martin Pitt to review and
comment on this thread when he gets a chance.
Thanks,
    Stephen

On 01/16/2014 07:32 AM, Christian Kruse wrote:
> Hi Alvaro,
>
> On 16/01/14 10:21, Alvaro Herrera wrote:
>> 1. it is to be read automatically by the server without need for an
>>    "include_dir conf.d" option in the main postgresql.conf.
>
> +1
>
>> 4. there is no point in "disabling" it, and thus we offer no mechanism
>>    to do that.
>
> Not only there is „no point“ in disabling it, it makes this feature
> nearly useless. One can't rely on it if the distro may disable
> it. There are so many out there, it will never be a reliable feature
> if it can be disabled.

It would make *my* life vastly easier if we could mandate things like
the presence and relative directory of a conf.d.  However, if Apache
can't do it, we certainly can't.  Ultimately, we cannot impose things on
distributions which they are unwilling to support; Debian, for one, will
happily fork PostgreSQL rather than accept directory assignments which
don't meet their standards.

Also, enough people install PostgreSQL from source or using custom
packages to make for a high degree of variation anyway.

That's why I was just advocating changing the *defaults*, not mandating
anything.  Actual directory locations and usage should be configurable
by distros, packagers and users.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Fri, Jan 17, 2014 at 12:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> PS: off topic, but isn't ParseConfigDirectory leaking the result
> of AbsoluteConfigLocation?  In both normal and error paths?

   Yes, I also think it leaks in both cases and similar leak is
   present in ParseConfigFile(). I have tried to fix both of these
   leaks with attached patch.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Fri, Jan 17, 2014 at 12:07 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, Jan 17, 2014 at 12:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> PS: off topic, but isn't ParseConfigDirectory leaking the result
>> of AbsoluteConfigLocation?  In both normal and error paths?
>
>    Yes, I also think it leaks in both cases and similar leak is
>    present in ParseConfigFile(). I have tried to fix both of these
>    leaks with attached patch.

Committed and back-patched to 9.3.  While reviewing, I noted that the
"skipping missing configuration file" message in ParseConfigFile()
uses an elevel of LOG, while the other messages in the same file use
"elevel".  I'm thinking that's a bug.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jan 21, 2014 at 8:25 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Jan 17, 2014 at 12:07 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Fri, Jan 17, 2014 at 12:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> PS: off topic, but isn't ParseConfigDirectory leaking the result
>>> of AbsoluteConfigLocation?  In both normal and error paths?
>>
>>    Yes, I also think it leaks in both cases and similar leak is
>>    present in ParseConfigFile(). I have tried to fix both of these
>>    leaks with attached patch.
>
> Committed and back-patched to 9.3.  Thanks.

> While reviewing, I noted that the
> "skipping missing configuration file" message in ParseConfigFile()
> uses an elevel of LOG, while the other messages in the same file use
> "elevel".  I'm thinking that's a bug.

It might not be right for all cases, but I think this is saving us in few cases
when the caller (directly or indirectly) has sent elevel as ERROR or FATAL,
in such cases we just want to LOG it, if strict is not true. Now it might not
be appropriate if the caller has sent DEBUG2 and we use LOG, but to
fix it (if this sounds an issue), some more analysis is required, so let's
keep it as it is for now.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Amit Kapila <amit.kapila16@gmail.com> writes:
> On Tue, Jan 21, 2014 at 8:25 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> While reviewing, I noted that the
>> "skipping missing configuration file" message in ParseConfigFile()
>> uses an elevel of LOG, while the other messages in the same file use
>> "elevel".  I'm thinking that's a bug.

> It might not be right for all cases, but I think this is saving us in few cases
> when the caller (directly or indirectly) has sent elevel as ERROR or FATAL,
> in such cases we just want to LOG it, if strict is not true. Now it might not
> be appropriate if the caller has sent DEBUG2 and we use LOG, but to
> fix it (if this sounds an issue), some more analysis is required, so let's
> keep it as it is for now.

The problem with this coding is that at SIGHUP, *all* the postgres
processes will bleat about a missing file.

To avoid that kind of log spam, the usual practice is to code the
elog level as something like "IsUnderPostmaster ? DEBUG2 : LOG";
see the elevel setting in ProcessConfigFile() for precedent.
In fact, I'm pretty sure that the elevel passed to ParseConfigFile
should always have come from that logic.

In any case, your argument is bogus because we could already have
thrown an error at the given elevel further up in ParseConfigFile,
or later in ParseConfigFp.  None of this code is meant to receive
an ERROR-or-higher elevel.

So yes, this is broken and needs to be changed as Robert says.
        regards, tom lane

On Wed, Jan 22, 2014 at 8:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Kapila <amit.kapila16@gmail.com> writes:
>> On Tue, Jan 21, 2014 at 8:25 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> While reviewing, I noted that the
>>> "skipping missing configuration file" message in ParseConfigFile()
>>> uses an elevel of LOG, while the other messages in the same file use
>>> "elevel".  I'm thinking that's a bug.
>
>> It might not be right for all cases, but I think this is saving us in few cases
>> when the caller (directly or indirectly) has sent elevel as ERROR or FATAL,
>> in such cases we just want to LOG it, if strict is not true. Now it might not
>> be appropriate if the caller has sent DEBUG2 and we use LOG, but to
>> fix it (if this sounds an issue), some more analysis is required, so let's
>> keep it as it is for now.
>
> The problem with this coding is that at SIGHUP, *all* the postgres
> processes will bleat about a missing file.
>
> To avoid that kind of log spam, the usual practice is to code the
> elog level as something like "IsUnderPostmaster ? DEBUG2 : LOG";
> see the elevel setting in ProcessConfigFile() for precedent.
> In fact, I'm pretty sure that the elevel passed to ParseConfigFile
> should always have come from that logic.
  The reason why I have mentioned that it can receive different elevel  than what ProcessConfigFile() sends is that
ParseConfigFile()gets  called from ParseConfigFp() which gets called from other paths  (recovery, parse_extension.. )
aswell which sends elevel as ERROR  or higher. However ParseConfigFp() when called from those paths  will not call
ParseConfigFile()as none of them will contain any  include directive, but code as written doesn't ensure that it will
alwayscome elevel set similar to ProcessConfigFile().
 

> In any case, your argument is bogus because we could already have
> thrown an error at the given elevel further up in ParseConfigFile,
> or later in ParseConfigFp.
 True, but not for this kind of case (ignore if file not present).



With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com