Re: PITR potentially broken in 9.2

On 2012-12-05 18:35:47 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2012-12-05 16:15:38 -0500, Tom Lane wrote:
> >> That's fine, but the immediate question is what are we doing to fix
> >> the back branches. I think everyone is clear that we should be testing
> >> LocalHotStandbyActive rather than precursor conditions to see if a pause
> >> is allowed, but are we going to do anything more than that?
>
> > I'd like to have inclusive/non-inclusive stops some resemblance of
> > sanity.
> > Raw patch including your earlier LocalHotStandbyActive one attached.
>
> I looked at this but couldn't get excited about using it. There were
> some obvious bugs ("if (!recoveryStopsHere)"?) but the real problem is
> that I think we're going to end up reworking the interaction between
> recoveryPausesHere and the recoveryStopsHere stanza quite a bit.
> In particular, we should expect that we're going to need to respond
> to a changed recovery target after any pause. So placing a call of
> recoveryPausesHere down at the loop bottom where the action is already
> predetermined seems like the wrong thing. I'm not entirely sure what
> a clean design would look like, but that's not it. I'll leave it to
> Simon to think about what we want to do there next.

Yes, the patch obviously wasn't ready for anything. Just seemed easier
that way than trying to describe what I think is sensible.

What I dislike with what you committed is that the state you're
investigating during the pause isn't the one youre going to end up
recoveryApply == true. That seems dangerous to me, even if its going to
be reworked in HEAD.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services