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 > <mich...@paquier.xyz> wrote in <20180622044521.gc5...@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
signature.asc
Description: PGP signature
