Re: New WAL record to detect the checkpoint redo location

On Fri, Jul 14, 2023 at 8:46 PM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> As I think I mentioned before, I like this idea. However, I don't like the
> implementation too much.

Thanks for looking into it.


> This might work right now, but doesn't really seem maintainable. Nor do I like
> adding branches into this code a whole lot.

Okay, Now I have moved the XlogInsert for the special record outside
the WalInsertLock so we don't need this special handling here.

> > @@ -6597,6 +6612,32 @@ CreateCheckPoint(int flags)
>
> I think the commentary above this function would need a fair bit of
> revising...
>
> >        */
> >       RedoRecPtr = XLogCtl->Insert.RedoRecPtr = checkPoint.redo;
> >
> > +     /*
> > +      * Insert a special purpose CHECKPOINT_REDO record as the first record at
> > +      * checkpoint redo lsn.  Although we have the checkpoint record that
> > +      * contains the exact redo lsn, there might have been some other records
> > +      * those got inserted between the redo lsn and the actual checkpoint
> > +      * record.  So when processing the wal, we cannot rely on the checkpoint
> > +      * record if we want to stop at the checkpoint-redo LSN.
> > +      *
> > +      * This special record, however, is not required when we doing a shutdown
> > +      * checkpoint, as there will be no concurrent wal insertions during that
> > +      * time.  So, the shutdown checkpoint LSN will be the same as
> > +      * checkpoint-redo LSN.
> > +      *
> > +      * This record is guaranteed to be the first record at checkpoint redo lsn
> > +      * because we are inserting this while holding the exclusive wal insertion
> > +      * lock.
> > +      */
> > +     if (!shutdown)
> > +     {
> > +             int                     dummy = 0;
> > +
> > +             XLogBeginInsert();
> > +             XLogRegisterData((char *) &dummy, sizeof(dummy));
> > +             recptr = XLogInsert(RM_XLOG_ID, XLOG_CHECKPOINT_REDO);
> > +     }
>
> It seems to me that we should be able to do better than this.
>
> I suspect we might be able to get rid of the need for exclusive inserts
> here. If we rid of that, we could determine the redoa location based on the
> LSN determined by the XLogInsert().

Yeah, good idea, actually we can do this insert outside of the
exclusive insert lock and set the LSN of this insert as the
checkpoint. redo location.  So now we do not need to compute the
checkpoint. redo based on the current insertion point we can directly
use the LSN of this record.  I have modified this and I have also
moved some other code that is not required to be inside the WAL
insertion lock.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com