Re: Attempt to consolidate reading of XLOG page - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Attempt to consolidate reading of XLOG page
Date
Msg-id 20190412042746.GJ2144@paquier.xyz
Whole thread Raw
In response to Re: Attempt to consolidate reading of XLOG page  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Responses Re: Attempt to consolidate reading of XLOG page
List pgsql-hackers
On Fri, Apr 12, 2019 at 12:27:11PM +0900, Kyotaro HORIGUCHI wrote:
> This patch changes XLogRead to allow using other than
> BasicOpenFile to open a segment, and use XLogReaderState.private
> to hold a new struct XLogReadPos for the segment reader. The new
> struct is heavily duplicated with XLogReaderState and I'm not
> sure the rason why the XLogReadPos is needed.
> Any other opinions, or thoughts?

The focus is on the stability of v12 for the next couple of months, so
please make sure to register it to the next CF if you want feedback.

Here are some basic thoughts after a very quick lookup.

+/*
+ * Position in XLOG file while reading it.
+ */
+typedef struct XLogReadPos
+{
+   int segFile;                /* segment file descriptor */
+   XLogSegNo   segNo;          /* segment number */
+   uint32 segOff;              /* offset in the segment */
+   TimeLineID tli;     /* timeline ID of the currently open file */
+
+   char    *dir;               /* directory (only needed by
frontends) */
+} XLogReadPos;
Not sure if there is any point to split that with the XLOG reader
status.

+static void fatal_error(const char *fmt,...) pg_attribute_printf(1, 2);
+
+static void
+fatal_error(const char *fmt,...)
In this more confusion accumulates with something which has enough
warts on HEAD when it comes to declare locally equivalents to the
elog() for src/common/.

+/*
+ * This is a front-end counterpart of XLogFileNameP.
+ */
+static char *
+XLogFileNameFE(TimeLineID tli, XLogSegNo segno)
+{
+   char       *result = palloc(MAXFNAMELEN);
+
+   XLogFileName(result, tli, segno, WalSegSz);
+   return result;
+}
We could use a pointer to an allocated area.  Or even better, just a
static variable as this just gets used for error messages to store
temporarily the segment name in a routine part of perhaps
xlogreader.c.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Experimenting with hash join prefetch
Next
From: Michael Paquier
Date:
Subject: Re: How to include the header files effectively