Re: Attempt to consolidate reading of XLOG page
Alvaro Herrera <alvherre@2ndquadrant.com>
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Antonin Houska <ah@cybertec.at>
Cc: Thomas Munro <thomas.munro@gmail.com>, Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2019-09-17T22:15:21Z
Lists: pgsql-hackers
Attachments
- xlogopen-rename.patch (text/x-diff) patch
I was confused by the struct name XLogSegment -- the struct is used to represent a WAL segment while it's kept open, rather than just a WAL segment in abstract. Also, now that we've renamed everything to use the term WAL, it seems wrong to use the name XLog for new structs. I propose the name WALOpenSegment for the struct, which solves both problems. (Its initializer function would get the name WALOpenSegmentInit.) Now, the patch introduces a callback for XLogRead, the type of which is called XLogOpenSegment. If we rename it from XLog to WAL, both names end up the same. I propose to rename the function type to WALSegmentOpen, which in a "noun-verb" view of the world, represents the action of opening a WAL segment. I attach a patch for all this renaming, on top of your series. I wonder if each of those WALSegmentOpen callbacks should reset [at least some members of] the struct; they're already in charge of setting ->file, and apparently we're leaving the responsibility of setting the rest of the members to XLogRead. That seems weird. Maybe we should say that the CB should only open the segment and not touch the struct at all and XLogRead is in charge of everything. Perhaps the other way around -- the CB should set everything correctly ... I'm not sure which is best. But having half here and half there seems a recipe for confusion and bugs. Another thing I didn't like much is that everything seems to assume that the only error possible from XLogRead is a read error. Maybe that's okay, because it seems to be the current reality, but it seemed odd. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Remove logical_read_local_xlog_page
- 5d0c2d5eba6b 13.0 landed
-
Refactor WAL file-reading code into WALRead()
- 0dc8ead46363 13.0 landed
-
Rework WAL-reading supporting structs
- 709d003fbd98 13.0 landed
-
Make XLogReaderInvalReadState static
- 25dcc9d35dfe 13.0 landed
-
Use pg_pread() and pg_pwrite() for data files and WAL.
- c24dcd0cfd94 12.0 cited