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.
