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




Reply via email to