On Fri, Jul 14, 2023 at 8:46 PM Andres Freund <and...@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
v2-0001-New-WAL-record-for-checkpoint-redo-location.patch
Description: Binary data