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: pgsql-hackers@postgresql.org
Date: 2019-04-12T16:46:56Z
Lists: pgsql-hackers
On 2019-Apr-11, Antonin Houska wrote:

> While working on the instance encryption I found it annoying to apply
> decyption of XLOG page to three different functions. Attached is a patch that
> tries to merge them all into one function, XLogRead(). The existing
> implementations differ in the way new segment is opened. So I added a pointer
> to callback function as a new argument. This callback handles the specific
> ways to determine segment file name and to open the file.
> 
> I can split the patch into multiple diffs to make detailed review easier, but
> first I'd like to hear if anything is seriously wrong about this
> design. Thanks.

I agree that xlog reading is pretty messy.

I think ifdef'ing the way XLogRead reports errors is not great.  Maybe
we can pass a function pointer that is to be called in case of errors?
Not sure about the walsize; maybe it can be a member in XLogReadPos, and
given to XLogReadInitPos()?  (Maybe rename XLogReadPos as
XLogReadContext or something like that, indicating it's not just the
read position.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Commits

  1. Remove logical_read_local_xlog_page

  2. Refactor WAL file-reading code into WALRead()

  3. Rework WAL-reading supporting structs

  4. Make XLogReaderInvalReadState static

  5. Use pg_pread() and pg_pwrite() for data files and WAL.