On 12/12/14 16:34, Alex Shulgin wrote:
Alex Shulgin <a...@commandprompt.com> writes:
Alex Shulgin <a...@commandprompt.com> writes:
Here's an attempt to revive this patch.
Here's the patch rebased against current HEAD, that is including the
recently committed action_at_recovery_target option.
The default for the new GUC is 'pause', as in HEAD, and
pause_at_recovery_target is removed completely in favor of it.
I've also taken the liberty to remove that part that errors out when
finding $PGDATA/recovery.conf. Now get your rotten tomatoes ready. ;-)
This was rather short-sighted, so I've restored that part.
Also, rebased on current HEAD, following the rename of
action_at_recovery_target to recovery_target_action.
Hi,
I had a quick look, the patch does not apply cleanly anymore but it's
just release notes so nothing too bad.
I did not do any testing yet, but I have few comments about the code:
- the patch mixes functionality change with the lowercasing of the
config variables, I wonder if those could be separated into 2 separate
diffs - it would make review somewhat easier, but I can live with it as
it is if it's too much work do separate into 2 patches
- the StandbyModeRequested and StandbyMode should be lowercased like the
rest of the GUCs
- I am wondering if StandbyMode and hot_standby should be merged into
one GUC if we are breaking backwards compatibility anyway
- Could you explain the reasoning behind:
+ if ((*newval)[0] == 0)
+ xid = InvalidTransactionId;
+ else
in check_recovery_target_xid() (and same check in
check_recovery_target_timeline()), isn't the strtoul which is called
later enough?
- The whole CopyErrorData and memory context logic is not needed in
check_recovery_target_time() as the FlushErrorState() is not called there
- The new code in StartupXLOG() like
+ if (recovery_target_xid_string != NULL)
+ InitRecoveryTarget(RECOVERY_TARGET_XID);
+
+ if (recovery_target_time_string != NULL)
+ InitRecoveryTarget(RECOVERY_TARGET_TIME);
+
+ if (recovery_target_name != NULL)
+ InitRecoveryTarget(RECOVERY_TARGET_NAME);
+
+ if (recovery_target_string != NULL)
+ InitRecoveryTarget(RECOVERY_TARGET_IMMEDIATE);
would probably be better in separate function that you then call
StartupXLOG() as StartupXLOG() is crazy long already so I think it's
preferable to not make it worse.
- I wonder if we should rename trigger_file to standby_tigger_file
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
