On Fri, Jun 22, 2018 at 02:34:02PM +0900, Kyotaro HORIGUCHI wrote:
> Hello, sorry for the absense and I looked the second patch.
Thanks for the review!
> At Fri, 22 Jun 2018 13:45:21 +0900, Michael Paquier
> <michael@paquier.xyz> wrote in <20180622044521.GC5215@paquier.xyz>
>> long as crash recovery runs. And XLogNeedsFlush() also has a similar
>> problem.
>
> Here, on the other hand, this patch turns off
> updateMinRecoverypoint if minRecoverPoint is invalid when
> RecoveryInProgress() == true. Howerver RecovInProg() == true
> means archive recovery is already started and and
> minRecoveryPoint *should* be updated t for the
> condition. Actually minRecoverypoint is updated just below. If
> this is really right thing, I think that some explanation for the
> reason is required here.
LocalRecoveryInProgress is just a local copy of SharedRecoveryInProgress
so RecoveryInProgress also returns true if crash recovery is running.
But perhaps I am missing what you mean? The point here is that redo can
call XLogNeedsFlush, no?
> In xlog_redo there still be "minRecoverypoint != 0", which ought
> to be described as "!XLogRecPtrIsInvalid(minRecoveryPoint)". (It
> seems the only one. Double negation is a bit uneasy but there are
> many instance of this kind of coding.)
It is possible to use directly a comparison with InvalidXLogRecPtr
instead of a double negation.
> # I'll go all-out next week.
Enjoy your vacations!
--
Michael