On Thu, Jun 23, 2016 at 3:50 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Jun 15, 2016 at 4:52 PM, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>> Hello,
>
> Sorry for the late reply, Horiguchi-san. I have finally been able to
> put some mind power into that.
>
>> This is somewhat artificial but the same situation could be made
>> also in the nature. The exact condition for this is replaying a
>> checkpoint record having no buffer modification since the
>> preceding checkpoint in the previous WAL segments.
>
> After thinking about that more, I am seeing your point.
> CreateRestartPoint is clearly missing the shot for the update of
> minRecoveryPoint even when a restart point can be created. Since
> cdd46c7, we assume that a node in recovery will not update it, and
> rely on the fact that it will be updated if some buffers are flushed,
> but that assumption got broken since the introduction of the
> possibility to take base backups from standbys, as in your case there
> is no activity on the standby that would allow minRecoveryPoint to be
> updated and make the backup finish in a clean state. By the way,
> relying on CheckpointStats to decide if it should be updated or not
> looks like a broken concept to me as this information is just stored
> for logging and has no role in any decision-making, and I think that
> it should remain this way. So, instead I think that we had better
> update minRecoveryPoint unconditionally as in the attached. I reworked
> the surrounding comments in consequence. That's a bit more aggressive,
> and that addresses the backup problems.
>
>> Returning to the discussion on which way to choose, we agreed
>> that using replayEndRecPtr instead of minRecoveryPoint in
>> do_pg_stop_backup(), as the patch attached to the following
>> message.
>
> Hm.. If there are no buffers flushed to disk that may increase the
> time it takes to reach a consistent state at recovery for no real
> reasons. So actually it is not that much a good idea...
By the way, do you mind if I add this patch to the next CF? Better not
to lose track of it...
--
Michael