On Fri, Jun 22, 2018 at 9:28 AM, Michael Paquier <mich...@paquier.xyz> wrote:
> > > This is not really a complicated patch, and it took a lot of energy from > me the last couple of days per the nature of the many scenarios to think > about... Thanks for the efforts. It wasn't an easy bug to chase to begin with. So I am not surprised there were additional problems that I missed. > So an extra pair of eyes from another committer would be > welcome. I am letting that cool down for a couple of days now. > I am not a committer, so don't know if my pair of eyes count, but FWIW the patch looks good to me except couple of minor points. +/* + * Local copies of equivalent fields in the control file. When running + * crash recovery, minRecoveryPoint is set to InvalidXLogRecPtr as we + * expect to replay all the WAL available, and updateMinRecoveryPoint is + * switched to false to prevent any updates while replaying records. + * Those values are kept consistent as long as crash recovery runs. + */ static XLogRecPtr minRecoveryPoint; /* local copy of * ControlFile->minRecoveryPoint */ The inline comment looks unnecessary now that we have comment at the top. @@ -4266,6 +4276,12 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode, minRecoveryPoint = ControlFile->minRecoveryPoint; minRecoveryPointTLI = ControlFile->minRecoveryPointTLI; + /* + * The startup process can update its local copy of + * minRecoveryPoint from that point. + */ s/that/this Thanks, Pavan -- Pavan Deolasee http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
