Thread: Separate HEAP WAL replay logic into its own file
Hi PostgreSQL hackers, For most access methods in PostgreSQL, the implementation of the access method itself and the implementation of its WAL replaylogic are organized in separate source files. However, the HEAP access method is an exception. Both the access methodand the WAL replay logic are collocated in the same heapam.c. To follow the pattern established by other access methodsand to improve maintainability, I made the enclosed patch to separate HEAP’s replay logic into its own file. Thechanges are straightforward. Move the replay related functions into the new heapam_xlog.c file, push the common heap_execute_freeze_tuple()helper function into the heapam.h header, and adjust the build files. I hope people find this straightforward refactoring helpful. Yong
Attachment
On Mon, Jun 17, 2024 at 2:20 AM Li, Yong <yoli@ebay.com> wrote: > > Hi PostgreSQL hackers, > > For most access methods in PostgreSQL, the implementation of the access method itself and the implementation of its WALreplay logic are organized in separate source files. However, the HEAP access method is an exception. Both the accessmethod and the WAL replay logic are collocated in the same heapam.c. To follow the pattern established by other accessmethods and to improve maintainability, I made the enclosed patch to separate HEAP’s replay logic into its own file. The changes are straightforward. Move the replay related functions into the new heapam_xlog.c file, push the commonheap_execute_freeze_tuple() helper function into the heapam.h header, and adjust the build files. I'm not against this change, but I am curious at what inspired this. Were you looking at Postgres code and simply noticed that there isn't a heapam_xlog.c (like there is a nbtxlog.c etc) and thought that you wanted to change that? Or is there some specific reason this would help you as a Postgres developer, user, or ecosystem member? - Melanie
> On Jun 17, 2024, at 23:01, Melanie Plageman <melanieplageman@gmail.com> wrote: > > External Email > > On Mon, Jun 17, 2024 at 2:20 AM Li, Yong <yoli@ebay.com> wrote: >> >> Hi PostgreSQL hackers, >> >> For most access methods in PostgreSQL, the implementation of the access method itself and the implementation of its WALreplay logic are organized in separate source files. However, the HEAP access method is an exception. Both the accessmethod and the WAL replay logic are collocated in the same heapam.c. To follow the pattern established by other accessmethods and to improve maintainability, I made the enclosed patch to separate HEAP’s replay logic into its own file. The changes are straightforward. Move the replay related functions into the new heapam_xlog.c file, push the commonheap_execute_freeze_tuple() helper function into the heapam.h header, and adjust the build files. > > I'm not against this change, but I am curious at what inspired this. > Were you looking at Postgres code and simply noticed that there isn't > a heapam_xlog.c (like there is a nbtxlog.c etc) and thought that you > wanted to change that? Or is there some specific reason this would > help you as a Postgres developer, user, or ecosystem member? > > - Melanie As a newcomer, when I was walking through the code looking for WAL replay related code, it was relatively easy for me tofind them for the B-Tree access method because of the “xlog” hint in the file names. It took me a while to find the samefor the heap access method. When I finally found them (via text search), it was a small surprise. Having different fileorganizations for different access methods gives me this urge to make everything consistent. I think it will make iteasier for newcomers, and it will reduce the mental load for everyone to remember that heap replay is inside the heapam.cnot some “???xlog.c”. Yong
On Mon, Jun 17, 2024 at 9:12 PM Li, Yong <yoli@ebay.com> wrote: > > As a newcomer, when I was walking through the code looking for WAL replay related code, it was relatively easy for me tofind them for the B-Tree access method because of the “xlog” hint in the file names. It took me a while to find the samefor the heap access method. When I finally found them (via text search), it was a small surprise. Having different fileorganizations for different access methods gives me this urge to make everything consistent. I think it will make iteasier for newcomers, and it will reduce the mental load for everyone to remember that heap replay is inside the heapam.cnot some “???xlog.c”. That makes sense. The branch for PG18 has not been cut yet, so I recommend registering this patch for the July commitfest [1] so it doesn't get lost. - Melanie [1] https://commitfest.postgresql.org/48/
> On Jun 18, 2024, at 20:42, Melanie Plageman <melanieplageman@gmail.com> wrote: > > External Email > > On Mon, Jun 17, 2024 at 9:12 PM Li, Yong <yoli@ebay.com> wrote: >> >> As a newcomer, when I was walking through the code looking for WAL replay related code, it was relatively easy for meto find them for the B-Tree access method because of the “xlog” hint in the file names. It took me a while to find thesame for the heap access method. When I finally found them (via text search), it was a small surprise. Having differentfile organizations for different access methods gives me this urge to make everything consistent. I think it willmake it easier for newcomers, and it will reduce the mental load for everyone to remember that heap replay is insidethe heapam.c not some “???xlog.c”. > > That makes sense. The branch for PG18 has not been cut yet, so I > recommend registering this patch for the July commitfest [1] so it > doesn't get lost. > > - Melanie > Thanks for the positive feedback. I’ve added the patch to the July CF. Yong