Turning recovery.conf into GUCs

From: Jaime Casanova <jaime(at)2ndquadrant(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Turning recovery.conf into GUCs
Date: 2013-10-16 16:48:25
Message-ID: CAJKUy5id1eyweK0W4+yyCM6+-qYs9erLidUmb=1a-QYBgTW4Qw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

Hi everyone,

Seems that the latest patch in this series is:

= Applying =

Reading the threads it seems there is consensus in this happening, so
let's move in that direction.
The patch applies almost cleanly except for a minor change in xlog.c

= Building =

In pg_basebackup we have now 2 unused functions:
escapeConnectionParameter and escape_quotes. the only use of these
functions was when that tool created the recovery.conf file so they
aren't needed anymore.

= Functionality =

I have been testing functionality and it looks good so far, i still
need to test a few things but i don't expect anything bad.

I have 2 complaints though:

1) the file to trigger recovery is now called standby.enabled. I know
this is because we use the file to also make the node a standby.
Now, reality is that the file doesn't make the node a standby but the
combination of this file (which starts recovery) + standby_mode=on.
so, i agree with the suggestion of naming the file: recovery.trigger

2) This patch removes the dual existence of recovery.conf: as a state
file and as a configuration file

- the former is accomplished by changing the name of the file that
triggers recovery (from recovery.conf to standby.enabled)
- the latter is done by moving all parameters to postgresql.conf and
*not reading* recovery.conf anymore

so, after applying this patch postgres don't use recovery.conf for
anything... except for complaining.
it's completely fair to say we are no longer using that file and
ignoring its existence, but why we should care if users want to use a
file with that name for inclusion in postgresql.conf or where they put
that hypotetic file?

after this patch, recovery.conf will have no special meaning anymore
so let's the user put it whatever files he wants, wherever he wants.
if we really want to warn the user, use WARNING instead of FATAL

= Code & functionality =

why did you changed the context in which we can set archive_command?
please, let it as a SIGHUP parameter

- {"archive_command", PGC_SIGHUP, WAL_ARCHIVING,
+ {"archive_command", PGC_POSTMASTER, WAL_ARCHIVING,

All parameters moved from recovery.conf has been marked as
PGC_POSTMASTER, but most of them are clearly PGC_SIGHUP candidates

+ {"archive_cleanup_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+ {"recovery_end_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+ {"recovery_target_xid", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+ {"recovery_target_name", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+ {"recovery_target_time", PGC_POSTMASTER, WAL_RECOVERY_TARGET,

Not sure about these ones

+ {"recovery_target_timeline", PGC_POSTMASTER, WAL_RECOVERY_TARGET,

This is the only one i agree, should be PGC_POSTMASTER only


Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitaciĆ³n
Phone: +593 4 5107566 Cell: +593 987171157


Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2013-10-16 16:52:59 Re: removing old ports and architectures
Previous Message Robert Haas 2013-10-16 16:26:28 Re: removing old ports and architectures