On Tue, Aug 30, 2016 at 11:00 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paqu...@gmail.com> writes: >> Does the attached suit better then? > > Thinking about it some more ... what we actually need to prevent, AFAICS, > is standby_mode becoming true in a standalone backend. If you don't set > that, then the process will do PITR recovery, but I'm not aware that there > is any problem with running that standalone, and maybe it's useful to > allow it for debugging purposes. So I'm thinking more like > > /* > * Check for recovery control file, and if so set up state for offline > * recovery > */ > readRecoveryCommandFile(); > > + /* > + * We don't support standby_mode in standalone backends; that > + * requires other processes such as WAL receivers to be alive. > + */ > + if (StandbyModeRequested && !IsUnderPostmaster) > + ereport(FATAL, ...); > + > /* > * Save archive_cleanup_command in shared memory so that other > processes > * can see it. > */ > > or we could put the check near the bottom of readRecoveryCommandFile. > Not sure which placement is clearer.
I have spent some time playing with that and you are right. Only standby_mode = on is able trigger a failure here, and the first one is in WaitForWALToBecomeAvailable(). I'd just put the check at the end of readRecoveryCommandFile(), this will avoid extra thinking should readRecoveryCommandFile() be moved around. That's unlikely to happen but it is a cheap insurance. I have added a test on that using 001_stream_rep.pl, now that's not actually a good idea to push this test as if it passes the execution of postgres would just hang and prevent the test to continue to run and move on. But it makes it easier for anybody looking at this patch to test the pattern prevented here. -- Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index acd95aa..b44fed0 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -5216,6 +5216,14 @@ readRecoveryCommandFile(void) } FreeConfigVariables(head); + + /* + * We don't support standby_mode in standalone backends; that + * requires other processes such as the WAL receiver to be alive. + */ + if (StandbyModeRequested && !IsUnderPostmaster) + ereport(FATAL, + (errmsg("standby mode is not supported for standalone backends"))); } /* diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index fd71095..7b47301 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -3,7 +3,7 @@ use strict; use warnings; use PostgresNode; use TestLib; -use Test::More tests => 4; +use Test::More tests => 5; # Initialize master node my $node_master = get_new_node('master'); @@ -18,6 +18,11 @@ $node_master->backup($backup_name); my $node_standby_1 = get_new_node('standby_1'); $node_standby_1->init_from_backup($node_master, $backup_name, has_streaming => 1); + +# Standby mode not supported for standalone backends +command_fails(['postgres', '--single', '-D', $node_standby_1->data_dir], + 'no standby mode support for standalone backends'); + $node_standby_1->start; # Take backup of standby 1 (not mandatory, but useful to check if
-- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers