Re: Elimination of the repetitive code at the SLRU bootstrap functions.

Álvaro, Andrey, Alexander, hello!

Since the master branch became the PG19dev and PG18 is now stable, I 
have directed this patch into PG19. Could we continue with it now?

Álvaro, should I rename the SimpleLruZeroPageExt function?

Best regards,
Evgeny Voropaev.


On 14.03.2025 21:43, Evgeny wrote:
> Hello Hackers!
> 
>  > I think this is going too far.  The new function BootStrapSlruPage() now
>  > is being used for things other than bootstrapping, and that doesn't seem
>  > appropriate to me.  I think we should leave the things that aren't
>  > bootstrap out of this patch.  For instance, ExtendCLOG was more
>  > understandable before this patch than after.  Same with
>  > ExtendMultiXact{Offset,Member}, ExtendSUBTRANS, etc.
> I have excluded the uses of BootStrapSlruPage in Extend* functions. The
> code of these functions is remaining still modified a bit on account of
> Zero* functions having been deleted. It now includes such fragments:
> 
> /* Zero the page and make an XLOG entry about it */
> SimpleLruZeroPage(MultiXactMemberCtl, pageno);
> XLogSimpleInsert(RM_MULTIXACT_ID, XLOG_MULTIXACT_ZERO_MEM_PAGE, pageno);
> 
> as a substitution of:
> /* Zero the page and make an XLOG entry about it */
> ZeroMultiXactOffsetPage(pageno, true);
> 
> But, probably, it even makes a code more readable showing both actions
> clearly.
> 
> 
>  > By removing these changes, you can remove the third argument to
>  > BootStrapSlruPage (the function pointer), since you don't need it.
> 
> Indeed, it has helped to remove an input parameter from the
> BootStrapSlruPage function. Honestly, even two arguments are removed,
> since code has not opted whether to write a page on a disk or not. It
> saves a page every time.
> 
>  > I'm also very suspicious of the use of the new function in the redo
>  > routines also, because those are not bootstrapping anything.  I'd rather
>  > have another routine, say SimpleLruRedoZeroPage() which only handles the
>  > zeroing of a page from a WAL record.  It would be very similar to
>  > BootstrapSlruPage, and maybe they can share an internal "workhorse"
>  > function for the bits that are common.
> Now the logic zeroing page on the "do" side is fully equal to one on the
> "redo" side. As a result, creation of a new distinct function for "redo"
> is not needed. In order for the function to conform to the both sides
> ("do" and "redo") I have renamed it into SimpleLruZeroPageExt. If this
> name looks not appropriate, we can change it, please, propose new one.
> 
> 
>  > I don't have faith in the function name XLogInsertInt64().  The datatype
>  > has no role there.  I liked my proposal better.
> 
> The name has been reverted to one Álvaro has proposed at first
> (XLogSimpleInsert).
> 
> Patch looks even better - it reduce code by 180 lines.
> 
> Best regards,
> Evgeny Voropaev.