agora inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH 2/2] Simplify XLogReader's open_segment API
23+ messages / 3 participants
[nested] [flat]

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* [PATCH 2/2] Simplify XLogReader's open_segment API
@ 2020-05-08 21:03  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Alvaro Herrera @ 2020-05-08 21:03 UTC (permalink / raw)

Instead of returning the file descriptor, install it directly in
XLogReaderState->seg.ws_file.
---
 src/backend/access/transam/xlogreader.c |  4 ++--
 src/backend/access/transam/xlogutils.c  | 32 ++++++++++++-------------
 src/backend/replication/walsender.c     | 12 ++++------
 src/bin/pg_waldump/pg_waldump.c         |  9 ++++---
 src/include/access/xlogreader.h         | 12 +++++-----
 src/include/access/xlogutils.h          |  2 +-
 6 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f42dee2640..a533241370 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1092,8 +1092,8 @@ WALRead(XLogReaderState *state,
 				state->routine.segment_close(state);
 
 			XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize);
-			state->seg.ws_file = state->routine.segment_open(state, nextSegNo,
-															 &state->segcxt, &tli);
+			state->routine.segment_open(state, nextSegNo, &state->segcxt, &tli);
+			Assert(state->seg.ws_file >= 0);	/* shouldn't happen */
 
 			/* Update the current segment info. */
 			state->seg.ws_tli = tli;
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index fc0bb7d059..1cc2c624a4 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -784,7 +784,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
 }
 
 /* XLogReaderRoutine->segment_open callback for local pg_wal files */
-int
+void
 wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 				 WALSegmentContext *segcxt, TimeLineID *tli_p)
 {
@@ -793,22 +793,20 @@ wal_segment_open(XLogReaderState *state, XLogSegNo nextSegNo,
 	int			fd;
 
 	XLogFilePath(path, tli, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
-
-	if (errno == ENOENT)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("requested WAL segment %s has already been removed",
-						path)));
-	else
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						path)));
-
-	return -1;					/* keep compiler quiet */
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file < 0)
+	{
+		if (errno == ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("requested WAL segment %s has already been removed",
+							path)));
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\": %m",
+							path)));
+	}
 }
 
 /* stock XLogReaderRoutine->segment_close callback */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ed8c08cb6a..b9f029d44f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -248,7 +248,7 @@ static void LagTrackerWrite(XLogRecPtr lsn, TimestampTz local_flush_time);
 static TimeOffset LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now);
 static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
-static int	WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
+static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
 							  WALSegmentContext *segcxt, TimeLineID *tli_p);
 static void UpdateSpillStats(LogicalDecodingContext *ctx);
 
@@ -2445,13 +2445,12 @@ WalSndKill(int code, Datum arg)
 }
 
 /* XLogReaderRoutine->segment_open callback */
-static int
+static void
 WalSndSegmentOpen(XLogReaderState *state,
 				  XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				  TimeLineID *tli_p)
 {
 	char		path[MAXPGPATH];
-	int			fd;
 
 	/*-------
 	 * When reading from a historic timeline, and there is a timeline switch
@@ -2488,9 +2487,9 @@ WalSndSegmentOpen(XLogReaderState *state,
 	}
 
 	XLogFilePath(path, *tli_p, nextSegNo, segcxt->ws_segsize);
-	fd = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-	if (fd >= 0)
-		return fd;
+	state->seg.ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
+	if (state->seg.ws_file >= 0)
+		return;
 
 	/*
 	 * If the file is not found, assume it's because the standby asked for a
@@ -2513,7 +2512,6 @@ WalSndSegmentOpen(XLogReaderState *state,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
 						path)));
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 46734914b7..1a5c5a157c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -280,7 +280,7 @@ identify_target_directory(char *directory, char *fname)
 }
 
 /* pg_waldump's XLogReaderRoutine->segment_open callback */
-static int
+static void
 WALDumpOpenSegment(XLogReaderState *state,
 				   XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 				   TimeLineID *tli_p)
@@ -300,9 +300,9 @@ WALDumpOpenSegment(XLogReaderState *state,
 	 */
 	for (tries = 0; tries < 10; tries++)
 	{
-		fd = open_file_in_directory(segcxt->ws_dir, fname);
-		if (fd >= 0)
-			return fd;
+		state->seg.ws_file = open_file_in_directory(segcxt->ws_dir, fname);
+		if (state->seg.ws_file >= 0)
+			return;
 		if (errno == ENOENT)
 		{
 			int			save_errno = errno;
@@ -318,7 +318,6 @@ WALDumpOpenSegment(XLogReaderState *state,
 	}
 
 	fatal_error("could not find file \"%s\": %m", fname);
-	return -1;					/* keep compiler quiet */
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e77f478d68..b73df02218 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -63,10 +63,10 @@ typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
 							   int reqLen,
 							   XLogRecPtr targetRecPtr,
 							   char *readBuf);
-typedef int (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
-								 XLogSegNo nextSegNo,
-								 WALSegmentContext *segcxt,
-								 TimeLineID *tli_p);
+typedef void (*WALSegmentOpenCB) (XLogReaderState *xlogreader,
+								  XLogSegNo nextSegNo,
+								  WALSegmentContext *segcxt,
+								  TimeLineID *tli_p);
 typedef void (*WALSegmentCloseCB) (XLogReaderState *xlogreader);
 
 typedef struct XLogReaderRoutine
@@ -94,8 +94,8 @@ typedef struct XLogReaderRoutine
 	XLogPageReadCB page_read;
 
 	/*
-	 * Callback to open the specified WAL segment for reading.  The file
-	 * descriptor of the opened segment shall be returned.  In case of
+	 * Callback to open the specified WAL segment for reading.  ->seg.ws_file
+	 * shall be set to the file descriptor of the opened segment.  In case of
 	 * failure, an error shall be raised by the callback and it shall not
 	 * return.
 	 *
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 68ce815476..b7bdc5db34 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -50,7 +50,7 @@ extern void FreeFakeRelcacheEntry(Relation fakerel);
 extern int	read_local_xlog_page(XLogReaderState *state,
 								 XLogRecPtr targetPagePtr, int reqLen,
 								 XLogRecPtr targetRecPtr, char *cur_page);
-extern int	wal_segment_open(XLogReaderState *state,
+extern void wal_segment_open(XLogReaderState *state,
 							 XLogSegNo nextSegNo,
 							 WALSegmentContext *segcxt,
 							 TimeLineID *tli_p);
-- 
2.20.1


--/04w6evG8XlLl3ft--





^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Consistently use the function name CreateCheckPoint instead of CreateCheckpoint in code comments
@ 2022-01-14 03:25  Bharath Rupireddy <[email protected]>
  0 siblings, 1 reply; 23+ messages in thread

From: Bharath Rupireddy @ 2022-01-14 03:25 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

Hi,

The function CreateCheckPoint is specified as CreateCheckpoint in some
of the code comments whereas in other places it is correctly
mentioned. Attaching a tiny patch to use CreateCheckPoint consistently
across code comments.

Regards,
Bharath Rupireddy.


Attachments:

  [application/octet-stream] v1-0001-Consistently-use-the-function-name-CreateCheckPoi.patch (1.9K, ../../CALj2ACVZmKsvDjtd45+9oTcnjUJtC4LF2BYK8TpWT1f=NjJX3w@mail.gmail.com/2-v1-0001-Consistently-use-the-function-name-CreateCheckPoi.patch)
  download | inline diff:
From 77e6252a2ceb3312710c68b21a1c83a5bda30150 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Fri, 14 Jan 2022 02:32:49 +0000
Subject: [PATCH v1] Consistently use the function name CreateCheckPoint
 instead of CreateCheckpoint in code comments

---
 src/backend/storage/buffer/bufmgr.c | 2 +-
 src/backend/storage/ipc/standby.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 4b72274989..a2512e750c 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3907,7 +3907,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
 			 * never gets written, so crash recovery will fix.
 			 *
 			 * It's possible we may enter here without an xid, so it is
-			 * essential that CreateCheckpoint waits for virtual transactions
+			 * essential that CreateCheckPoint waits for virtual transactions
 			 * rather than full transactionids.
 			 */
 			MyProc->delayChkpt = delayChkpt = true;
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index d70f2e839d..87ac0f74b2 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -1182,7 +1182,7 @@ standby_redo(XLogReaderState *record)
  * starting to accumulate changes at a point just prior to when we derive
  * the snapshot on the primary, then ignore duplicates when we later apply
  * the snapshot from the running xacts record. This is implemented during
- * CreateCheckpoint() where we use the logical checkpoint location as
+ * CreateCheckPoint() where we use the logical checkpoint location as
  * our starting point and then write the running xacts record immediately
  * before writing the main checkpoint WAL record. Since we always start
  * up from a checkpoint and are immediately at our starting point, we
-- 
2.25.1



^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Re: Consistently use the function name CreateCheckPoint instead of CreateCheckpoint in code comments
@ 2022-01-14 14:35  Amit Kapila <[email protected]>
  parent: Bharath Rupireddy <[email protected]>
  0 siblings, 1 reply; 23+ messages in thread

From: Amit Kapila @ 2022-01-14 14:35 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Fri, Jan 14, 2022 at 8:55 AM Bharath Rupireddy
<[email protected]> wrote:
>
> The function CreateCheckPoint is specified as CreateCheckpoint in some
> of the code comments whereas in other places it is correctly
> mentioned. Attaching a tiny patch to use CreateCheckPoint consistently
> across code comments.
>

LGTM. I'll take care of this unless someone thinks otherwise.

-- 
With Regards,
Amit Kapila.






^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Re: Consistently use the function name CreateCheckPoint instead of CreateCheckpoint in code comments
@ 2022-01-17 03:15  Amit Kapila <[email protected]>
  parent: Amit Kapila <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Amit Kapila @ 2022-01-17 03:15 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Fri, Jan 14, 2022 at 8:05 PM Amit Kapila <[email protected]> wrote:
>
> On Fri, Jan 14, 2022 at 8:55 AM Bharath Rupireddy
> <[email protected]> wrote:
> >
> > The function CreateCheckPoint is specified as CreateCheckpoint in some
> > of the code comments whereas in other places it is correctly
> > mentioned. Attaching a tiny patch to use CreateCheckPoint consistently
> > across code comments.
> >
>
> LGTM. I'll take care of this unless someone thinks otherwise.
>

Pushed.

-- 
With Regards,
Amit Kapila.






^ permalink  raw  reply  [nested|flat] 23+ messages in thread

end of thread, other threads:[~2022-01-17 03:15 UTC | newest]

Thread overview: 23+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2020-05-08 21:03 [PATCH 2/2] Simplify XLogReader's open_segment API Alvaro Herrera <[email protected]>
2022-01-14 03:25 Consistently use the function name CreateCheckPoint instead of CreateCheckpoint in code comments Bharath Rupireddy <[email protected]>
2022-01-14 14:35 ` Re: Consistently use the function name CreateCheckPoint instead of CreateCheckpoint in code comments Amit Kapila <[email protected]>
2022-01-17 03:15   ` Re: Consistently use the function name CreateCheckPoint instead of CreateCheckpoint in code comments Amit Kapila <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox