Thread

1. includeifexists in configuration file

   Greg Smith <greg@2ndquadrant.com> — 2011-11-16T05:28:04Z

   By recent popular request in the ongoing discussion saga around merging 
   the recovery.conf, I've added an "includeifexists" directive to the 
   postgresql.conf in the attached patch.  Demo:
   
   $ tail -n 1 $PGDATA/postgresql.conf
   include 'missing.conf'
   $ pg_ctl start -l $PGLOG
   server starting
   $ tail -n 2 $PGLOG
   LOG:  could not open configuration file 
   "/home/gsmith/pgwork/data/include-exists/missing.conf": No such file or 
   directory
   FATAL:  configuration file 
   "/home/gsmith/pgwork/data/include-exists/postgresql.conf" contains errors
   $ vi $PGDATA/postgresql.conf
   $ tail -n 1 $PGDATA/postgresql.conf
   includeifexists 'missing.conf'
   $ pg_ctl start -l $PGLOG
   server starting
   $ tail -n 3 $PGLOG
   LOG:  database system was shut down at 2011-11-16 00:17:36 EST
   LOG:  database system is ready to accept connections
   LOG:  autovacuum launcher started
   
   There might be a cleaner way to write this that eliminates some of the 
   cut and paste duplication between this and the regular include 
   directive.  I'm short on clever but full of brute force tonight.
   
   -- 
   Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
   PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
   
   

2. Re: includeifexists in configuration file

   Euler Taveira de Oliveira <euler@timbira.com> — 2011-11-16T14:16:12Z

   On 16-11-2011 02:28, Greg Smith wrote:
   > By recent popular request in the ongoing discussion saga around merging the
   > recovery.conf, I've added an "includeifexists" directive to the
   > postgresql.conf in the attached patch.
   > 
   I'm not following the merging recovery.conf thread but isn't it worth emitting
   at least an WARNING message when the file does not exist?
   
   Something like
   
   WARNING:  could not open configuration file "/foo/missing.conf", skipping
   
   Let's suppose a DBA is using this new feature to include some general company
   recommendations. If (s)he mistyped the name of the file, the general
   recommendations will not be applied and the DBA won't be even warned. That's
   not what a DBA would expect.
   
   
   -- 
      Euler Taveira de Oliveira - Timbira       http://www.timbira.com.br/
      PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
   
   

3. Re: includeifexists in configuration file

   Tom Lane <tgl@sss.pgh.pa.us> — 2011-11-16T14:54:18Z

   Euler Taveira de Oliveira <euler@timbira.com> writes:
   > On 16-11-2011 02:28, Greg Smith wrote:
   >> By recent popular request in the ongoing discussion saga around merging the
   >> recovery.conf, I've added an "includeifexists" directive to the
   >> postgresql.conf in the attached patch.
   
   > I'm not following the merging recovery.conf thread but isn't it worth emitting
   > at least an WARNING message when the file does not exist?
   
   > Something like
   
   > WARNING:  could not open configuration file "/foo/missing.conf", skipping
   
   Minor note here: people keep thinking that WARNING > LOG with respect to
   messages that can only go to the server log.  This is not correct ...
   LOG would be the right elevel to use.
   
   			regards, tom lane
   
   

4. Re: includeifexists in configuration file

   Robert Haas <robertmhaas@gmail.com> — 2011-11-16T15:19:50Z

   On Wed, Nov 16, 2011 at 12:28 AM, Greg Smith <greg@2ndquadrant.com> wrote:
   > By recent popular request in the ongoing discussion saga around merging the
   > recovery.conf, I've added an "includeifexists" directive to the
   > postgresql.conf in the attached patch.
   
   I haven't read the code yet, but just to get the bikeshedding started,
   I think it might be better to call this include_if_exists rather than
   running it together as one word.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company
   
   

5. Re: includeifexists in configuration file

   Greg Smith <greg@2ndquadrant.com> — 2011-12-12T19:24:53Z

   On 11/16/2011 10:19 AM, Robert Haas wrote:
   > I haven't read the code yet, but just to get the bikeshedding started,
   > I think it might be better to call this include_if_exists rather than
   > running it together as one word.
   >    
   
   What's going on, it's like this bikeshed just disappeared.  I should 
   figure out how that happened so I can replicate it.
   
   This naming style change sounds fine to me, and I just adopted it for 
   the updated configuration directory patch.  That patch now rearranges 
   the documentation this feature modifies.  This is a pretty trivial 
   feature I'm not real concerned about getting a review for.  I'll update 
   this with the name change and appropriate rebased patch once some 
   decision has been made about that one; will just bounce this forward to 
   January if it's still here when the current CF starts closing in earnest.
   
   -- 
   Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
   PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
   
   
   

6. Re: includeifexists in configuration file

   Ross Reedstrom <reedstrm@rice.edu> — 2011-12-12T21:05:04Z

   On Mon, Dec 12, 2011 at 02:24:53PM -0500, Greg Smith wrote:
   > On 11/16/2011 10:19 AM, Robert Haas wrote:
   > >I haven't read the code yet, but just to get the bikeshedding started,
   > >I think it might be better to call this include_if_exists rather than
   > >running it together as one word.
   > 
   > What's going on, it's like this bikeshed just disappeared.  I should
   > figure out how that happened so I can replicate it.
   
   Must be that special "camo" paint.
   
   
   Ross
   Woohoo! Caught up from my beginning of Oct. trip backlog, just in time for Christmas!
   -- 
   Ross Reedstrom, Ph.D.                                 reedstrm@rice.edu
   Systems Engineer & Admin, Research Scientist        phone: 713-348-6166
   Connexions                  http://cnx.org            fax: 713-348-3665
   Rice University MS-375, Houston, TX 77005
   GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E  F888 D3AE 810E 88F0 BEDE
   
   

7. Re: includeifexists in configuration file

   Andrew Dunstan <andrew@dunslane.net> — 2011-12-12T21:47:08Z

   
   On 12/12/2011 02:24 PM, Greg Smith wrote:
   > On 11/16/2011 10:19 AM, Robert Haas wrote:
   >> I haven't read the code yet, but just to get the bikeshedding started,
   >> I think it might be better to call this include_if_exists rather than
   >> running it together as one word.
   >
   > What's going on, it's like this bikeshed just disappeared.  I should 
   > figure out how that happened so I can replicate it.
   >
   > This naming style change sounds fine to me, and I just adopted it for 
   > the updated configuration directory patch.  That patch now rearranges 
   > the documentation this feature modifies.  This is a pretty trivial 
   > feature I'm not real concerned about getting a review for.  I'll 
   > update this with the name change and appropriate rebased patch once 
   > some decision has been made about that one; will just bounce this 
   > forward to January if it's still here when the current CF starts 
   > closing in earnest.
   >
   
   
   I have briefly looked at the code (but not tried to apply or build it), 
   and modulo the naming issue it looks OK to me.
   
   Unless there is some other issue let's just get it applied. It looks 
   like almost a no-brainer to me.
   
   cheers
   
   andrew
   
   

8. Re: includeifexists in configuration file

   Greg Smith <greg@2ndquadrant.com> — 2011-12-15T11:54:57Z

   On 12/12/2011 04:47 PM, Andrew Dunstan wrote:
   > I have briefly looked at the code (but not tried to apply or build 
   > it), and modulo the naming issue it looks OK to me.
   > Unless there is some other issue let's just get it applied. It looks 
   > like almost a no-brainer to me.
   
   It isn't very fancy, but is does something people that can fit into a 
   couple of use-cases.  Attached update has two changes to address the 
   suggestions I got, which closes everything I knew about with this one:
   
   -It's now include_if_exists
   -Files that are skipped are logged now
   
   So current behavior:
   
   $ tail -n 1 postgresql.conf
   include 'missing.conf'
   $ start
   server starting
   $ tail $PGLOG
   LOG:  could not open configuration file 
   "/home/gsmith/pgwork/data/include-exists/missing.conf": No such file or 
   directory
   FATAL:  configuration file 
   "/home/gsmith/pgwork/data/include-exists/postgresql.conf" contains errors
   
   And new behavior:
   
   $ vi $PGDATA/postgresql.conf
   $ tail -n 1 postgresql.conf
   include_if_exists 'missing.conf'
   $ start
   server starting
   $ tail $PGLOG
   LOG:  skipping missing configuration file 
   "/home/gsmith/pgwork/data/include-exists/missing.conf"
   LOG:  database system was shut down at 2011-12-15 06:48:46 EST
   LOG:  database system is ready to accept connections
   
   -- 
   Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
   PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
   
   

9. Re: includeifexists in configuration file

   Andrew Dunstan <andrew@dunslane.net> — 2011-12-16T01:16:22Z

   
   On 12/15/2011 06:54 AM, Greg Smith wrote:
   > On 12/12/2011 04:47 PM, Andrew Dunstan wrote:
   >> I have briefly looked at the code (but not tried to apply or build 
   >> it), and modulo the naming issue it looks OK to me.
   >> Unless there is some other issue let's just get it applied. It looks 
   >> like almost a no-brainer to me.
   >
   > It isn't very fancy, but is does something people that can fit into a 
   > couple of use-cases.  Attached update has two changes to address the 
   > suggestions I got, which closes everything I knew about with this one:
   >
   > -It's now include_if_exists
   > -Files that are skipped are logged now
   >
   > So current behavior:
   >
   > $ tail -n 1 postgresql.conf
   > include 'missing.conf'
   > $ start
   > server starting
   > $ tail $PGLOG
   > LOG:  could not open configuration file 
   > "/home/gsmith/pgwork/data/include-exists/missing.conf": No such file 
   > or directory
   > FATAL:  configuration file 
   > "/home/gsmith/pgwork/data/include-exists/postgresql.conf" contains errors
   >
   > And new behavior:
   >
   > $ vi $PGDATA/postgresql.conf
   > $ tail -n 1 postgresql.conf
   > include_if_exists 'missing.conf'
   > $ start
   > server starting
   > $ tail $PGLOG
   > LOG:  skipping missing configuration file 
   > "/home/gsmith/pgwork/data/include-exists/missing.conf"
   > LOG:  database system was shut down at 2011-12-15 06:48:46 EST
   > LOG:  database system is ready to accept connections
   
   Committed. I changed the elog() call to use ereport(): you're not 
   supposed to use elog() for things we expect might well happen and cause 
   log entries - see bottom of 
   <http://www.postgresql.org/docs/current/static/error-message-reporting.html>. 
   I've probably been guilty of this in the past, it's a bit too easy to 
   forget.
   
   cheers
   
   andrew
   
   

10. Re: includeifexists in configuration file

    Greg Smith <greg@2ndquadrant.com> — 2011-12-16T06:02:06Z

    On 12/15/2011 08:16 PM, Andrew Dunstan wrote:
    >  I changed the elog() call to use ereport(): you're not supposed to 
    > use elog() for things we expect might well happen and cause log 
    > entries - see bottom of 
    > <http://www.postgresql.org/docs/current/static/error-message-reporting.html>. 
    > I've probably been guilty of this in the past, it's a bit too easy to 
    > forget.
    
    Quite, I both knew this once and forgot it last night.  There was some 
    nagging in the back of my head that I was doing something wrong, but I 
    couldn't place what.  Happy this is committed, given that I've suggested 
    relying upon it in the recovery.conf thread.
    
    -- 
    Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
    PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us