Thread: includeifexists in configuration file
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
Attachment
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, Suporte24x7 e Treinamento
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
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
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
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
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
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
Attachment
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
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