On 2019-Nov-12, Antonin Houska wrote: > ok, the next version uses explicit lseek(). Maybe the fact that XLOG is mostly > read sequentially (i.e. without frequent seeks) is the reason pread() has't > been adopted so far.
I don't quite understand why you backed off from switching to pread. It seemed a good change to me. Here's a few edits on top of your latest. The new routine WALRead() is not at all the same as the previous XLogRead, so I don't see why we would keep the name. Hence renamed. I see no reason for the openSegment callback to return the FD in an out param instead of straight return value. Changed that way. Having seek/open be a boolean "xlr_seek" seems a bit weird. Changed to an "operation" enum. (Maybe if we go back to pg_pread we can get rid of this.) Accordingly, change WALReadRaiseError and WALDumpReadPage. Change xlr_seg to be a struct rather than pointer to struct. It seems a bit dangerous to me to return a pointer that we don't know is going to be valid at raise-error time. Struct assignment works fine for the purpose. Renamed XLogDumpReadPage to WALDumpReadPage, because what the heck is XLogDump anyway? That doesn't exist. I would only like to switch this back to pg_pread() (from seek/read) and I'd be happy to commit this. What is logical_read_local_xlog_page all about? Seems useless. Let's get rid of it. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services