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

From Robert Haas
Subject Re: Attempt to consolidate reading of XLOG page
Date
Msg-id CA+TgmobTi1mVe_H9sig=Y3R91wZ-5FT5dL-gB8roifr3xboecg@mail.gmail.com
Whole thread Raw
In response to Re: Attempt to consolidate reading of XLOG page  (Antonin Houska <ah@cybertec.at>)
Responses Re: Attempt to consolidate reading of XLOG page  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
On Thu, May 2, 2019 at 12:18 PM Antonin Houska <ah@cybertec.at> wrote:
> The next version of the patch is attached.

I don't think any of this looks acceptable:

+#ifndef FRONTEND
+/*
+ * Backend should have wal_segment_size variable initialized, segsize is not
+ * used.
+ */
+#define XLogFileNameCommon(tli, num, segsize) XLogFileNameP((tli), (num))
+#define xlr_error(...) ereport(ERROR, (errcode_for_file_access(),
errmsg(__VA_ARGS__)))
+#else
+static char xlr_error_msg[MAXFNAMELEN];
+#define XLogFileNameCommon(tli, num, segsize)
(XLogFileName(xlr_error_msg, (tli), (num), (segsize)),\
+    xlr_error_msg)
+#include "fe_utils/logging.h"
+/*
+ * Frontend application (currently only pg_waldump.c) cannot catch and further
+ * process errors, so they simply treat them as fatal.
+ */
+#define xlr_error(...) do {pg_log_fatal(__VA_ARGS__);
exit(EXIT_FAILURE); } while(0)
+#endif

The backend part doesn't look OK because depending on the value of a
global variable instead of getting the information via parameters
seems like a step backward.  The frontend part doesn't look OK because
it locks every application that uses the xlogreader stuff into using
pg_log_fatal when an error occurs, which may not be what everybody
wants to do.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Fixing order of resowner cleanup in 12, for Windows
Next
From: Robert Haas
Date:
Subject: Re: Fixing order of resowner cleanup in 12, for Windows