SIGHUP during recovery
Hi,
Currently, the startup process ignores SIGHUP.
The attached patch allows the startup process to re-read config file:
when SIGHUP arrives, the startup process also receives the signal
from postmaster and reload the settings in main redo apply loop.
Obviously, this is useful to change the parameters which the startup
process may use (e.g. log_line_prefix, log_checkpoints).
Any comments welcome.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
sighup_startup_0303.patchapplication/octet-stream; name=sighup_startup_0303.patchDownload
? GNUmakefile
? config.log
? config.status
? contrib/pg_standby/pg_standby
? contrib/pgbench/pgbench
? src/Makefile.global
? src/backend/postgres
? src/backend/catalog/postgres.bki
? src/backend/catalog/postgres.description
? src/backend/catalog/postgres.shdescription
? src/backend/snowball/snowball_create.sql
? src/backend/utils/probes.h
? src/backend/utils/mb/conversion_procs/conversion_create.sql
? src/bin/initdb/initdb
? src/bin/pg_config/pg_config
? src/bin/pg_controldata/pg_controldata
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_dump/pg_restore
? src/bin/pg_resetxlog/pg_resetxlog
? src/bin/psql/psql
? src/bin/scripts/clusterdb
? src/bin/scripts/createdb
? src/bin/scripts/createlang
? src/bin/scripts/createuser
? src/bin/scripts/dropdb
? src/bin/scripts/droplang
? src/bin/scripts/dropuser
? src/bin/scripts/reindexdb
? src/bin/scripts/vacuumdb
? src/include/pg_config.h
? src/include/stamp-h
? src/interfaces/ecpg/compatlib/exports.list
? src/interfaces/ecpg/compatlib/libecpg_compat.so.3.1
? src/interfaces/ecpg/ecpglib/exports.list
? src/interfaces/ecpg/ecpglib/libecpg.so.6.1
? src/interfaces/ecpg/include/ecpg_config.h
? src/interfaces/ecpg/include/stamp-h
? src/interfaces/ecpg/pgtypeslib/exports.list
? src/interfaces/ecpg/pgtypeslib/libpgtypes.so.3.1
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpq/exports.list
? src/interfaces/libpq/libpq.so.5.2
? src/port/pg_config_paths.h
? src/test/regress/log
? src/test/regress/pg_regress
? src/test/regress/results
? src/test/regress/testtablespace
? src/test/regress/tmp_check
? src/test/regress/expected/constraints.out
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/largeobject.out
? src/test/regress/expected/largeobject_1.out
? src/test/regress/expected/misc.out
? src/test/regress/expected/tablespace.out
? src/test/regress/sql/constraints.sql
? src/test/regress/sql/copy.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/largeobject.sql
? src/test/regress/sql/misc.sql
? src/test/regress/sql/tablespace.sql
? src/timezone/zic
Index: src/backend/access/transam/xlog.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.332
diff -c -r1.332 xlog.c
*** src/backend/access/transam/xlog.c 23 Feb 2009 09:28:49 -0000 1.332
--- src/backend/access/transam/xlog.c 3 Mar 2009 06:38:29 -0000
***************
*** 429,434 ****
--- 429,435 ----
/*
* Flag set by interrupt handlers for later service in the redo loop.
*/
+ static volatile sig_atomic_t got_SIGHUP = false;
static volatile sig_atomic_t shutdown_requested = false;
/*
* Flag set when executing a restore command, to tell SIGTERM signal handler
***************
*** 5363,5368 ****
--- 5364,5378 ----
#endif
/*
+ * Check if we were requested to re-read config file.
+ */
+ if (got_SIGHUP)
+ {
+ got_SIGHUP = false;
+ ProcessConfigFile(PGC_SIGHUP);
+ }
+
+ /*
* Check if we were requested to exit without finishing
* recovery.
*/
***************
*** 7641,7646 ****
--- 7651,7663 ----
}
+ /* SIGHUP: set flag to re-read config file at next convenient time */
+ static void
+ StartupProcSigHupHandler(SIGNAL_ARGS)
+ {
+ got_SIGHUP = true;
+ }
+
/* SIGTERM: set flag to abort redo and exit */
static void
StartupProcShutdownHandler(SIGNAL_ARGS)
***************
*** 7667,7673 ****
/*
* Properly accept or ignore signals the postmaster might send us
*/
! pqsignal(SIGHUP, SIG_IGN); /* ignore config file updates */
pqsignal(SIGINT, SIG_IGN); /* ignore query cancel */
pqsignal(SIGTERM, StartupProcShutdownHandler); /* request shutdown */
pqsignal(SIGQUIT, startupproc_quickdie); /* hard crash time */
--- 7684,7690 ----
/*
* Properly accept or ignore signals the postmaster might send us
*/
! pqsignal(SIGHUP, StartupProcSigHupHandler); /* ignore config file updates */
pqsignal(SIGINT, SIG_IGN); /* ignore query cancel */
pqsignal(SIGTERM, StartupProcShutdownHandler); /* request shutdown */
pqsignal(SIGQUIT, startupproc_quickdie); /* hard crash time */
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.574
diff -c -r1.574 postmaster.c
*** src/backend/postmaster/postmaster.c 25 Feb 2009 11:07:43 -0000 1.574
--- src/backend/postmaster/postmaster.c 3 Mar 2009 06:38:38 -0000
***************
*** 1949,1954 ****
--- 1949,1956 ----
(errmsg("received SIGHUP, reloading configuration files")));
ProcessConfigFile(PGC_SIGHUP);
SignalChildren(SIGHUP);
+ if (StartupPID != 0)
+ signal_child(StartupPID, SIGHUP);
if (BgWriterPID != 0)
signal_child(BgWriterPID, SIGHUP);
if (WalWriterPID != 0)
Fujii Masao wrote:
Currently, the startup process ignores SIGHUP.
The attached patch allows the startup process to re-read config file:
when SIGHUP arrives, the startup process also receives the signal
from postmaster and reload the settings in main redo apply loop.
Obviously, this is useful to change the parameters which the startup
process may use (e.g. log_line_prefix, log_checkpoints).
Thanks, committed.
The fact that bgwriter can run simultaneously with the startup process
makes this more important than before. Otherwise if you change something
like log_line_prefix, bgwriter will use the new setting but startup
process will not.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
On Wed, 2009-03-04 at 15:58 +0200, Heikki Linnakangas wrote:
Fujii Masao wrote:
Currently, the startup process ignores SIGHUP.
The attached patch allows the startup process to re-read config file:
when SIGHUP arrives, the startup process also receives the signal
from postmaster and reload the settings in main redo apply loop.
Obviously, this is useful to change the parameters which the startup
process may use (e.g. log_line_prefix, log_checkpoints).Thanks, committed.
The fact that bgwriter can run simultaneously with the startup process
makes this more important than before. Otherwise if you change something
like log_line_prefix, bgwriter will use the new setting but startup
process will not.
Should we reload recovery.conf also?
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
Hi,
On Thu, Mar 5, 2009 at 6:28 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Should we reload recovery.conf also?
I think no for now. At least the parameters which specify the recovery target
should not be changed during recovery. On the other hand, restore_command
maybe can be set safely, but I'm not sure if it's really useful at this time.
Or, upcoming HS needs such capability?
BTW, I found that backup.sgml still had the description of log_restartpoints.
Here is the patch to remove it.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
remove_log_restartpoints_doc.patchapplication/octet-stream; name=remove_log_restartpoints_doc.patchDownload
Index: doc/src/sgml/backup.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/backup.sgml,v
retrieving revision 2.122
diff -c -r2.122 backup.sgml
*** doc/src/sgml/backup.sgml 13 Jan 2009 00:54:11 -0000 2.122
--- doc/src/sgml/backup.sgml 5 Mar 2009 10:41:03 -0000
***************
*** 1198,1217 ****
</listitem>
</varlistentry>
- <varlistentry id="log-restartpoints"
- xreflabel="log_restartpoints">
- <term><varname>log_restartpoints</varname>
- (<type>boolean</type>)
- </term>
- <listitem>
- <para>
- Specifies whether to log each restart point as it occurs. This
- can be helpful to track the progress of a long recovery.
- Default is <literal>false</>.
- </para>
- </listitem>
- </varlistentry>
-
</variablelist>
</sect3>
--- 1198,1203 ----
***************
*** 1865,1871 ****
backup. You can do this by running <application>pg_controldata</>
on the standby server to inspect the control file and determine the
current checkpoint WAL location, or by using the
! <varname>log_restartpoints</> option to print values to the server log.
</para>
</sect2>
</sect1>
--- 1851,1857 ----
backup. You can do this by running <application>pg_controldata</>
on the standby server to inspect the control file and determine the
current checkpoint WAL location, or by using the
! <varname>log_checkpoints</> option to print values to the server log.
</para>
</sect2>
</sect1>
On Thu, 2009-03-05 at 19:52 +0900, Fujii Masao wrote:
Hi,
On Thu, Mar 5, 2009 at 6:28 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Should we reload recovery.conf also?
I think no for now. At least the parameters which specify the recovery target
should not be changed during recovery.
Why not?
On the other hand, restore_command
maybe can be set safely, but I'm not sure if it's really useful at this time.
Not sure changing those parameters would be a bad thing. Other
parameters can be changed, why not those?
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
Simon Riggs wrote:
On Thu, 2009-03-05 at 19:52 +0900, Fujii Masao wrote:
Hi,
On Thu, Mar 5, 2009 at 6:28 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Should we reload recovery.conf also?
I think no for now. At least the parameters which specify the recovery target
should not be changed during recovery.Why not?
I don't see either why it wouldn't work. As long as the new value is
greater than the current point in recovery.
BTW, we now have some extra safeguards (minRecoveryPoint) in place to
prevent you from corrupting your database by moving recovery target
backwards to a point earlier than an already recovery xlog record.
That's not really that relevant to reloading recovery.conf on the fly,
but it's worth noting. The documentation currently says about the
settings in recovery.conf that "They cannot be changed once recovery has
begun." That wording should probably be relaxed.
On the other hand, restore_command
maybe can be set safely, but I'm not sure if it's really useful at this time.Not sure changing those parameters would be a bad thing. Other
parameters can be changed, why not those?
It seems safe to me. It doesn't seem very useful, though. The only
options left in recovery.conf are restore_command, and all the
target-related options. If you want to change those, you can always stop
the server, change the settings and restart; without hot standby there
isn't any other backends active yet that would get upset about the shutdown.
The main reason why reloading the main config file on SIGHUP is a good
idea is that otherwise you get an inconsistency between the background
writer and the startup process.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Fujii Masao wrote:
BTW, I found that backup.sgml still had the description of log_restartpoints.
Here is the patch to remove it.
Thanks, I had put that in the Open Items list so that we remember to do
that before release.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com