Re: [HACKERS] archive_timeout ignored directly after promotion

2017-06-21 Thread Michael Paquier
On Thu, Jun 22, 2017 at 2:58 AM, Andres Freund  wrote:
> One easy way to fix that would be to just wakeup the checkpointer from
> the startup process once at the end of recovery, but it'd not be
> pretty.   I think it'd be better to change the
> do_restartpoint = RecoveryInProgress();
>
> /*
>  * The end-of-recovery checkpoint is a real 
> checkpoint that's
>  * performed while we're still in recovery.
>  */
> if (flags & CHECKPOINT_END_OF_RECOVERY)
> do_restartpoint = false;
>
> into having a per-loop 'local_in_recovery' variable, that we turn off
> once a CHECKPOINT_END_OF_RECOVERY checkpoint is requested.
>
> Comments?

By Initializing this flag out of the for(;;) loop once, this could put
more control into the checkpointer logic so as no new restart points
can be generated after the end-of-recovery checkpoint. This way we
could replace the hack looking for DB_IN_ARCHIVE_RECOVERY in
CreateRestartPoint() by an error handling.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] archive_timeout ignored directly after promotion

2017-06-21 Thread Andres Freund
Hi,

When promoting a standby without using the "fast promotion" logic,
e.g. by using recovery targets (which doesn't use fast promotion for
unbeknownst to me reasons), checkpointer doesn't necessarily respect
archive timeout for the first cycle after promotion.  The reason for
that is that it determines the sleep times like
if (XLogArchiveTimeout > 0 && !RecoveryInProgress())
{
elapsed_secs = now - last_xlog_switch_time;
if (elapsed_secs >= XLogArchiveTimeout)
continue;   /* no sleep for us ... 
*/
cur_timeout = Min(cur_timeout, XLogArchiveTimeout - 
elapsed_secs);
}
and RecoveryInProgress() will still return true.  We just fudge that
when creating the end-of-recovery checkpoint:
/*
 * The end-of-recovery checkpoint is a real checkpoint 
that's
 * performed while we're still in recovery.
 */
if (flags & CHECKPOINT_END_OF_RECOVERY)
do_restartpoint = false;

One easy way to fix that would be to just wakeup the checkpointer from
the startup process once at the end of recovery, but it'd not be
pretty.   I think it'd be better to change the
do_restartpoint = RecoveryInProgress();

/*
 * The end-of-recovery checkpoint is a real checkpoint 
that's
 * performed while we're still in recovery.
 */
if (flags & CHECKPOINT_END_OF_RECOVERY)
do_restartpoint = false;

into having a per-loop 'local_in_recovery' variable, that we turn off
once a CHECKPOINT_END_OF_RECOVERY checkpoint is requested.

Comments?

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers