Thread
-
Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny Voropaev <evgeny.voropaev@tantorlabs.com> — 2025-02-13T07:15:01Z
Hello hackers! The functions, bootstrapping SLRU pages, such as BootStrapMultiXact, BootStrapCLOG, ActivateCommitTs, multixact_redo and others, have a lot of repetitive code. A new proposed function BootStrapSlruPage moves a duplicating code into the single place. Additionally, a new member ZeroFunc is implemented in the SlruCtlData structure. The ZeroFunc keeps a pointer to a function proper for nullifying SLRU pages of a type defined by a corresponding SlruCtlData object. Applying proposed modifications can simplify maintenance and future development of Postgres code in the realm of bootstrapping SLRU. Best regards, Evgeny Voropaev, Tantor Labs LLC.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Aleksander Alekseev <aleksander@timescale.com> — 2025-02-13T12:05:19Z
Hi Evgeny, > The functions, bootstrapping SLRU pages, such as BootStrapMultiXact, > BootStrapCLOG, ActivateCommitTs, multixact_redo and others, have a lot > of repetitive code. > > A new proposed function BootStrapSlruPage moves a duplicating code into > the single place. Additionally, a new member ZeroFunc is implemented in > the SlruCtlData structure. The ZeroFunc keeps a pointer to a function > proper for nullifying SLRU pages of a type defined by a corresponding > SlruCtlData object. > > Applying proposed modifications can simplify maintenance and future > development of Postgres code in the realm of bootstrapping SLRU. Thanks for the patch. ``` --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -102,14 +102,6 @@ TransactionIdToPage(TransactionId xid) */ #define THRESHOLD_SUBTRANS_CLOG_OPT 5 -/* - * Link to shared-memory data structures for CLOG control - */ -static SlruCtlData XactCtlData; - -#define XactCtl (&XactCtlData) - - static int ZeroCLOGPage(int64 pageno, bool writeXlog); static bool CLOGPagePrecedes(int64 page1, int64 page2); static void WriteZeroPageXlogRec(int64 pageno); @@ -130,6 +122,14 @@ static void TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids, XLogRecPtr lsn, int64 pageno); +/* + * Link to shared-memory data structures for CLOG control + */ +static SlruCtlData XactCtlData = { .ZeroPage = ZeroCLOGPage }; + +#define XactCtl (&XactCtlData) + + ``` Since BootStrapSlruPage() is the only caller of ZeroPage() it seems to me that it merely wastes space in SlruCtlData. On top of that I'm not 100% sure if all the supported platforms have C99 compilers with designated initializers support. Wouldn't it be simpler to pass the callback straight to BootStrapSlruPage()? After changing the patch accordingly it shouldn't move XactCtl and others. This is just an irrelevant change. -- Best regards, Aleksander Alekseev -
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-02-13T13:21:33Z
On 2025-Feb-13, Aleksander Alekseev wrote: > Since BootStrapSlruPage() is the only caller of ZeroPage() it seems to > me that it merely wastes space in SlruCtlData. On top of that I'm not > 100% sure if all the supported platforms have C99 compilers with > designated initializers support. They do -- we use them quite extensively nowadays. > Wouldn't it be simpler to pass the callback straight to > BootStrapSlruPage()? Yeah, maybe this would be easier. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Just treat us the way you want to be treated + some extra allowance for ignorance." (Michael Brusser)
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Aleksander Alekseev <aleksander@timescale.com> — 2025-02-13T13:23:52Z
Hi Alvaro, > > Since BootStrapSlruPage() is the only caller of ZeroPage() it seems to > > me that it merely wastes space in SlruCtlData. On top of that I'm not > > 100% sure if all the supported platforms have C99 compilers with > > designated initializers support. > > They do -- we use them quite extensively nowadays. Got it, thanks. I was a bit worried about Visual Studio in particular. -- Best regards, Aleksander Alekseev
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny Voropaev <evgeny.voropaev@tantorlabs.com> — 2025-02-14T06:54:06Z
Hello Aleksander! Hello Álvaro! Thank you for your attention to the patch. > Since BootStrapSlruPage() is the only caller of ZeroPage() it seems to > me that it merely wastes space in SlruCtlData. On top of that I'm not > 100% sure if all the supported platforms have C99 compilers with > designated initializers support. > Wouldn't it be simpler to pass the callback straight to > BootStrapSlruPage()? I agree with your proposals. Moreover, similar solution is also implemented and exploited at Tantor's versions of the xid64 patch (https://www.postgresql.org/message-id/5ca56aed-dc69-4c3e-968f-a822ae0937b5%40gmail.com). The corrected patch version implementing these proposals is attached. Best regards, Evgeny Voropaev, Tantor Labs LLC.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Andrey Borodin <x4mmm@yandex-team.ru> — 2025-02-17T12:18:39Z
> On 14 Feb 2025, at 11:54, Evgeny Voropaev <evgeny.voropaev@tantorlabs.com> wrote: > > <v2-0001-Elimination-of-the-repetitive-code-at-the-SLRU-bo.patch> Hi! Nice patch! BootStrapSlruPage() always calls zerofunc(pageno, false) with second argument false. In case of every possible argument (ZeroCLOGPage, ZeroCommitTsPage, ZeroMultiXactOffsetPage, ZeroMultiXactMemberPage, ZeroSUBTRANSPage) it means just a call to SimpleLruZeroPage(). I think we can safely replace + slotno = (*zerofunc)(pageno, false); with + slotno = SimpleLruZeroPage(pageno); Thus we will not need zerofunc argument at all. Thanks! Best regards, Andrey Borodin.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-02-17T13:00:30Z
On 2025-Feb-17, Andrey Borodin wrote: > BootStrapSlruPage() always calls zerofunc(pageno, false) with second argument false. > In case of every possible argument (ZeroCLOGPage, ZeroCommitTsPage, > ZeroMultiXactOffsetPage, ZeroMultiXactMemberPage, ZeroSUBTRANSPage) it > means just a call to SimpleLruZeroPage(). > I think we can safely replace > > + slotno = (*zerofunc)(pageno, false); > > with > > + slotno = SimpleLruZeroPage(pageno); > > Thus we will not need zerofunc argument at all. Good observation. This also suggests another change: because this new function is used not only for bootstrapping but also during WAL replay, we can call the new function SimpleLruUnloggedZeroPage() and place it immediately after SimpleLruZeroPage, instead of at the end of the file. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "La persona que no quería pecar / estaba obligada a sentarse en duras y empinadas sillas / desprovistas, por cierto de blandos atenuantes" (Patricio Vogel)
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny Voropaev <evgeny.voropaev@tantorlabs.com> — 2025-02-18T09:02:42Z
On 17.02.2025 21:00, Álvaro Herrera wrote: > On 2025-Feb-17, Andrey Borodin wrote: > >> BootStrapSlruPage() always calls zerofunc(pageno, false) with second argument false. >> In case of every possible argument (ZeroCLOGPage, ZeroCommitTsPage, >> ZeroMultiXactOffsetPage, ZeroMultiXactMemberPage, ZeroSUBTRANSPage) it >> means just a call to SimpleLruZeroPage(). >> I think we can safely replace >> >> + slotno = (*zerofunc)(pageno, false); >> >> with >> >> + slotno = SimpleLruZeroPage(pageno); >> >> Thus we will not need zerofunc argument at all. > > Good observation. This also suggests another change: because this new > function is used not only for bootstrapping but also during WAL replay, > we can call the new function SimpleLruUnloggedZeroPage() and place it > immediately after SimpleLruZeroPage, instead of at the end of the file. > Created functions BootStrapSlruPage,SimpleLruZeroAndLogPage, WriteSlruZeroPageXlogRec. Using of these functions allows to delete ZeroXYZPage functions, WriteXYZZeroPageXlogRec functions and eliminate code repetitions. The conception is: /* * BootStrapSlruPage, * SimpleLruZeroAndLogPage, * SimpleLruZeroPage * - functions nullifying SLRU pages. * * BootStrapSlruPage is the most holistic. It performs: * 1. locking, * 2. nullifying, * 3. logging (when writeXlog is true), * 4. writing out, * 5. releasing the lock. * * SimpleLruZeroAndLogPage performs: * 2. nullifying, * 3. logging (when writeXlog is true), * 4. writing out. * * If the writeXlog is true, BootStrapSlruPage and SimpleLruZeroAndLogPage * emit an XLOG record saying we did this. * If the writeXlog is false, the rmid and info parameters are unused. * * SimpleLruZeroPage performs: * 2. nullifying. */
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-02-22T15:51:31Z
On 2025-Feb-18, Evgeny Voropaev wrote: > Created functions BootStrapSlruPage,SimpleLruZeroAndLogPage, > WriteSlruZeroPageXlogRec. Using of these functions allows to delete > ZeroXYZPage functions, WriteXYZZeroPageXlogRec functions and eliminate code > repetitions. I think the overall idea here is good, but I didn't like the new WriteSlruZeroPageXlogRec helper function; it looks too much like a modularity violation (same for the fact that you have to pass the rmid and info from each caller into slru.c). I would not do that in slru.c but instead in every individual caller, and have this helper function in xloginsert.c where it can be caller XLogSimpleInsert or something like that. Also, please do not document a bunch of functions together in a single comment. Instead, have one comment for each function. I mean this here: > diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c > index 9ce628e62a5..cc069da19c6 100644 > --- a/src/backend/access/transam/slru.c > +++ b/src/backend/access/transam/slru.c > @@ -363,6 +364,65 @@ check_slru_buffers(const char *name, int *newval) > return false; > } > > +/* > + * BootStrapSlruPage, > + * SimpleLruZeroAndLogPage, > + * SimpleLruZeroPage > + * - functions nullifying SLRU pages. > + * > + * BootStrapSlruPage is the most holistic. It performs: > + * 1. locking, > + * 2. nullifying, > + * 3. logging (when writeXlog is true), > + * 4. writing out, > + * 5. releasing the lock. > + * > + * SimpleLruZeroAndLogPage performs: > + * 2. nullifying, > + * 3. logging (when writeXlog is true), > + * 4. writing out. > + * > + * If the writeXlog is true, BootStrapSlruPage and SimpleLruZeroAndLogPage > + * emit an XLOG record saying we did this. > + * If the writeXlog is false, the rmid and info parameters are unused. > + * > + * SimpleLruZeroPage performs: > + * 2. nullifying. > + */ > +void > +BootStrapSlruPage(SlruCtl ctl, int64 pageno, > + bool writeXlog, RmgrId rmid, uint8 info) > +{ This is not our style, and I don't think it's very ergonomical. Most people don't read source files from top to bottom normally (heck, sometimes I read source from bottom to top), so somebody looking at SimpleLruZeroAndLogPage (the second function) might just not realize that it's documented a few pagefuls above itself. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ Thou shalt study thy libraries and strive not to reinvent them without cause, that thy code may be short and readable and thy days pleasant and productive. (7th Commandment for C Programmers) -
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny <evorop@gmail.com> — 2025-02-24T05:42:54Z
Hello hackers! > I think the overall idea here is good, but I didn't like the new > WriteSlruZeroPageXlogRec helper function; it looks too much like a > modularity violation (same for the fact that you have to pass the rmid > and info from each caller into slru.c). I would not do that in slru.c > but instead in every individual caller, and have this helper function in > xloginsert.c where it can be caller XLogSimpleInsert or something like > that. Thank you Alvaro! I got your ideas and have tried to implement them. In the v4 patch version: 1) WriteSlruZeroPageXlogRec has been renamed into XLogSimpleInsert and moved to the xloginset.c. 2) With the purpose of excluding passing any resource manager’s information into the slru.c module I propose to pass into the BootStrapSlruPage the XLogInsertFunc parameter pointing to a function accomplishing insertion of an XLog record. Implementations of these functions are enclosed in a corresponding slru-page module (commit_ts.c, multixaxt.c and so on). It preserves original modularity - the BootStrapSlruPage and the slru.c still do not know anything about resource managers. If the XLogInsertFunc parameter equals zero, BootStrapSlruPage will not try to perform XLog recording. 3) In addition, let’s also implement in the BootStrapSlruPage a new parameter writePage. It commands whether to write a page onto a disk or not. As a result, the BootStrapSlruPage become apt to be used in a larger number of places of code. > Also, please do not document a bunch of functions together in a single > comment. Instead, have one comment for each function. > This is not our style, and I don't think it's very ergonomical. I got it. And since 4) The SimpleLruZeroAndLogPage is subject to deletion now. comments are formatted as well. > sometimes I read source from bottom to top Probably, we all should profess this approach! Best regards, Evgeny Voropaev, Tantor Labs LLC.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny <evorop@gmail.com> — 2025-03-04T11:41:47Z
Hello Hackers! Álvaro, Andrey, Aleksander! We have not been discussing anything in the thread for the past ten days. Can we mark this thread as "Ready for Merge" or should I improve something in the patch. If I should, I am ready to do it. Looking forward to your feedback. Evgeny Voropaev.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny <evorop@gmail.com> — 2025-03-11T09:12:10Z
Hello Hackers! On 11 March 2025, the CI job failed (https://cirrus-ci.com/task/5453963400577024). The issue occurred in the test ‘pg_amcheck/t/002_nonesuch.pl . Log: https://api.cirrus-ci.com/v1/artifact/task/5453963400577024/testrun/build/testrun/pg_amcheck/002_nonesuch/log/regress_log_002_nonesuch Log output: -------------------------------------------------------------------------------------- [11:40:20.527](0.000s) not ok 18 - checking with a non-existent user stderr /(?^:role "no_such_user" does not exist)/ [11:40:20.527](0.000s) # Failed test 'checking with a non-existent user stderr /(?^:role "no_such_user" does not exist)/' # at C:/cirrus/src/bin/pg_amcheck/t/002_nonesuch.pl line 86. [11:40:20.527](0.000s) # 'pg_amcheck: error: connection to server on socket "C:/Windows/TEMP/AtJUArHHFu/.s.PGSQL.17536" failed: server closed the connection unexpectedly # This probably means the server terminated abnormally # before or while processing the request. # ' # doesn't match '(?^:role "no_such_user" does not exist)' # Running: pg_amcheck template1 ---------------------------------------------------------------------------------- It appears to be a flickering issue that regularly occurs on the Windows platform and is not related to the patch. For the purpose of restarting the CI job, I have attached the 5th version of the patch. It is identical to v4, except for the version number increment, which helps prevent duplicate attachment names in this thread. Best regards, Evgeny Voropaev.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Andrey Borodin <x4mmm@yandex-team.ru> — 2025-03-12T06:25:47Z
> On 11 Mar 2025, at 14:12, Evgeny <evorop@gmail.com> wrote: > Hi! Some nits: Patch adds whitespace errors .git/rebase-apply/patch:64: trailing whitespace. * Nullify the page (pageno = 0), don't insert an XLog record, .git/rebase-apply/patch:212: trailing whitespace. /* .git/rebase-apply/patch:213: trailing whitespace. * Zero the page; .git/rebase-apply/patch:250: trailing whitespace. .git/rebase-apply/patch:349: trailing whitespace. * Nullify the page (pageno = 0), don't insert an XLog record, warning: squelched 10 whitespace errors warning: 15 lines add whitespace errors. if (writePage != 0) should be if (writePage) XLogSimpleInsert(int64 simpledata, RmgrId rmid, uint8 info) I’d rename function XLogSimpleInsert() to something more descriptive and changed arguments order from generic to specific. Probably, committer has broader view on XLog routines and can decide if this function would better belong to SLRU than common XLog stuff. Besides this patch seems ready to me. Thanks! Best regards, Andrey Borodin.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny Voropaev <evorop.wiki@gmail.com> — 2025-03-12T15:02:07Z
Hello Hackers! Andrey, thank you for your review and remarks. > Patch adds whitespace errors Cleaned. > if (writePage != 0) should be if (writePage) Done. > XLogSimpleInsert(int64 simpledata, RmgrId rmid, uint8 info) > I’d rename function XLogSimpleInsert() to something more descriptive > and changed arguments order from generic to specific. The function has been renamed and the parameters have been reordered. Now we have: XLogInsertInt64(RmgrId rmid, uint8 info, int64 simpledata) > Probably, committer has broader view on XLog routines and can decide > if this function would better belong to SLRU than common XLog stuff. In accordance with Álvaro's proposal, we want to enclose this function in the "xloginsert.c" module. Best regards, Evgeny Voropaev. -
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Andrey Borodin <x4mmm@yandex-team.ru> — 2025-03-12T15:41:00Z
> On 12 Mar 2025, at 20:02, Evgeny Voropaev <evorop.wiki@gmail.com> wrote: > v6 looks good to me. I'll flip the CF entry. Thanks! Best regards, Andrey Borodin.
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-03-13T12:49:20Z
Hello 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. By removing these changes, you can remove the third argument to BootStrapSlruPage (the function pointer), since you don't need it. 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. I don't have faith in the function name XLogInsertInt64(). The datatype has no role there. I liked my proposal better. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ -
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny <evorop@gmail.com> — 2025-03-14T13:43:31Z
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. -
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny <evorop@gmail.com> — 2025-07-02T09:41:05Z
Á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. -
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-07-07T15:01:52Z
On 2025-Jul-02, Evgeny wrote: > Á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? Sure, I pushed your patch now. > Álvaro, should I rename the SimpleLruZeroPageExt function? Well, I didn't like that name -- normally, names ending in Ext represent a version of the routine named without the "Ext" that has some additions to its argument list, so the Ext is an extended version or something like that. That pattern does not fit this case. I used the name "SimpleLruZeroAndWritePage" instead, and rewrote the comment to explain that it's a simple wrapper that does exactly what the name says. Because of this I also removed some comments in the callsites, because those would have been redundant with the new name. So, no you don't need to do anything, because I already did it. I also went back and accepted Andrey's suggestion to have Int64 in the name of the XLogSimpleInsert routine, because it's not totally unthinkable that we'll have some other simple wrapper in the future. I made it return the LSN, because while no current caller needs it, some external caller might want to have that. I also moved each routine to a more natural place, namely just below the function they wrap. The pattern of adding stuff at the end of the file just results in messy code, so don't do that. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Tiene valor aquel que admite que es un cobarde" (Fernandel)
-
Re: Elimination of the repetitive code at the SLRU bootstrap functions.
Evgeny <evorop@gmail.com> — 2025-07-08T08:28:33Z
Álvaro, thank you for pushing, attention and final editing of the patch! I got your recommendations and requirements. I will take them into account at a next patch. Andrey, Alexander thank you for your proposals, attention, advice, and review! Best regards, Evgeny Voropaev, Tantor Labs, LLC.