Re: pg_ctl promote wait

On Wed, Aug 3, 2016 at 9:35 PM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)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