Á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.
