Re: PANIC during crash recovery of a recently promoted standby

On Fri, Jun 22, 2018 at 9:28 AM, Michael Paquier <michael@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