On Wed, Aug 3, 2016 at 9:35 PM, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > Updated patch set for pg_ctl promote -w, incorporating AFAICT all of the > previous reviews, in particular Michael Paquier's changes to the > StartupXLOG() ordering, and rebasing on top of > src/common/controldata_utils.c.
Glad to see a new set of patches here: 1) From 0001 - ok($result, "@$cmd exit code 0"); - is($stderr, '', "@$cmd no stderr"); + ok($result, "$test_name: exit code 0"); + is($stderr, '', "$test_name: no stderr"); Okay with that, there is anyway a log mentioning the command anyway. Perhaps you'd want to backpatch that? 2) From 0002: +command_fails_like([ 'pg_ctl', 'promote', '-D', "$tempdir/nonexistent" ], Any code using src/port/getopt_long.c (Windows!) is going to fail on that. The argument that is not preceded by an option name needs to be put at the end of the command. So for example this command needs to be changed to that: [ 'pg_ctl', '-D', "$tempdir/nonexistent", 'promote' ] +$node_standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()'); + +is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'), + 'f', 'promoted standby is not in recovery'); We could just check that poll_query_until returns 1 here. This would save running one query. 3) From 0003 In do_stop, this patches makes the wait happen for a maximum of wait_seconds * 2, once when getting the control file information, and once when waiting for the server to shut down. This is not a good idea, and the idea of putting a wait argument in get_controlfile does not seem a good interface to me. I'd rather see get_controlfile be extended with a flag saying no_error_on_failure and keep the wait logic within pg_ctl. The flag would do the following when enabled: - for frontends, do not issue a WARNING message and return NULL instead. - for backends, do not issue an ERROR and return NULL instead. 4) Patch 0004, no real comments :) I am glad you picked up this idea. 5) Patch 0005: Looks good to me. I just noticed that similar to 0002 regarding the ordering of those arguments: +command_ok([ 'pg_ctl', 'promote', '-w', '-D', $node_standby->data_dir ], + 'pg_ctl promote -w of standby runs'); Thanks, -- Michael -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers