Re: [BUG] pg_basebackup from disconnected standby fails

Hi,

At Mon, 24 Oct 2016 15:55:58 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in
<CAB7nPqTDnE62F9RVXwvOw4ihCy9iVMgEYJeozjDQYV5pF7YvFg@mail.gmail.com>
> On Mon, Oct 24, 2016 at 1:39 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Mon, Oct 24, 2016 at 9:18 AM, Kyotaro HORIGUCHI
> > <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> >> Thank you for looking and retelling this.
> 
> +1.
> 
> >> At Fri, 21 Oct 2016 13:02:21 -0400, Robert Haas <robertmhaas@gmail.com> wrote in
<CA+TgmoaRdyfCqcg9x-E99XCxdC4LW0zygP5qLsPhFsb6BYDKow@mail.gmail.com>
> >>> I can think of two solutions that would be "tighter":
> >>>
> >>> 1. When performing a restartpoint, update the minimum recovery point
> >>> to just beyond the checkpoint record.  I think this can't hurt anyone
> >>> who is actually restarting recovery on the same machine, because
> >>> that's exactly the point where they are going to start recovery.  A
> >>> minimum recovery point which precedes the point at which they will
> >>> start recovery is no better than one which is equal to the point at
> >>> which they will start recovery.
> >>
> >> This is a candidate that I thought of. But I avoided to change
> >> the behavior of minRecoveryPoint that (seems to me that) it
> >> describes only buffer state. So checkpoint with no update doesn't
> >> advances minRecoveryPoint as my understanding.
> >
> > I think what you are saying is not completely right, because we do
> > update minRecoveryPoint when we don't perform a new restart point.
> > When we perform restart point, then it assumes that flushing the
> > buffers will take care of updating minRecoveryPoint.

Thank you pointing that. I've forgot it. But as I looked there
again, I found that I have mistaken for a long time the code as
updating minRecoveryPoint, but it actually does invalidation.  If
the "update" were in the block of shutdown checkpoint, it would
look consistent but it is before the block, so any skipped
non-shutdown checkpoint record invalidates minRecoveryPoint but
leave the state as DB_IN_ARCHIVE_RECOVERY, aren't this two in a
contradiction? (Though I don't see something bad will happen with
it..)

> Yep, minRecoveryPoint still gets updated when the last checkpoint
> record is the last restart point to avoid a hot standby to allow
> read-only connections at a LSN-point earlier than the last shutdown.

The described behavior seems corect, but the code seems to be
doing somewhat different.

> Anyway, we can clearly reject 1. in the light of
> https://www.postgresql.org/message-id/CAA4eK1KmjtsXqF0cav7Cs4d4vwv2H_pc8d8q1BUCqDzAF+7EzQ@mail.gmail.com
> when playing with different stop locations at recovery.

| * If the last checkpoint record we've replayed is already our last
| * restartpoint, we can't perform a new restart point. We still update
| * minRecoveryPoint in that case, so that if this is a shutdown restart
| * point, we won't start up earlier than before. 
...
| * We don't explicitly advance minRecoveryPoint when we do create a
| * restartpoint. It's assumed that flushing the buffers will do that as a
| * side-effect.

The second sentense seems to me as "we *expect* minRecoveryPoint
to be updated anyway even if we don't do that here". Though a bit
different in reality..

skipped checkpoints - advance minRecvoeryPoint to the checkpoint

I'm failing to make a consistent model for the code around here
in my mind..

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center