Re: archive status ".ready" files may be created too early

At Thu, 5 Aug 2021 05:15:04 +0000, "Bossart, Nathan" <bossartn(at)amazon(dot)com> wrote in
> On 8/4/21, 9:05 PM, "Kyotaro Horiguchi" <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > By the way about the v3 patch,
> >
> > +#define InvalidXLogSegNo ((XLogSegNo) 0xFFFFFFFFFFFFFFFF)
> >
> > Like InvalidXLogRecPtr, the first valid segment number is 1 so we can
> > use 0 as InvalidXLogSegNo.
>
> It's been a while since I wrote this, but if I remember correctly, the
> issue with using 0 is that we could end up initializing
> lastNotifiedSeg to InvalidXLogSegNo in XLogWrite(). Eventually, we'd
> initialize it to 1, but we will have skipped creating the .ready file
> for the first segment.

Maybe this?

+ SetLastNotifiedSegmentIfInvalid(openLogSegNo - 1);

Hmm. Theoretically 0 is invalid as segment number. So we'd better not
using 0 as a valid value of lastNotifiedSeg.

Honestly I don't like having this initialization in XLogWrite. We
should and I think can initialize it earlier. It seems to me the most
appropriate timing to initialize the variable is just before running
the end-of-recovery checkpoint). Since StartupXLOG knows the first
segment to write . If it were set to 0, that doesn't matter at all.
We can get rid of the new symbol by doing this.

Maybe something like this:

> {
> /*
> * There is no partial block to copy. Just set InitializedUpTo, and
> * let the first attempt to insert a log record to initialize the next
> * buffer.
> */
> XLogCtl->InitializedUpTo = EndOfLog;
> }
>
+ /*
+ * EndOfLog resides on the next segment of the last finished one. Set the
+ * last finished segment as lastNotifiedSeg now. In the case where the
+ * last crash has left the last several segments not being marked as
+ * .ready, the checkpoint just after does that for all finished segments.
+ * There's a corner case where the checkpoint advances segment, but that
+ * ends up at most with a duplicate archive notification.
+ */
+ XLByteToSeg(EndOfLog, EndOfLogSeg, wal_segment_size);
+ Assert(EndOfLogSeg > 0);
+ SetLastNotifiedSegment(EndOfLogSeg - 1);
+
> LogwrtResult.Write = LogwrtResult.Flush = EndOfLog;

Does this makes sense?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center