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

At Thu, 5 Aug 2021 05:15:04 +0000, "Bossart, Nathan" <bossartn@amazon.com> wrote in 
> On 8/4/21, 9:05 PM, "Kyotaro Horiguchi" <horikyota.ntt@gmail.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