agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
[PATCH 5/5] Remove the old implemenations of XLogRead().
28+ messages / 7 participants
[nested] [flat]

* [PATCH 5/5] Remove the old implemenations of XLogRead().
@ 2019-07-09 09:54 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-07-09 09:54 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the XLogOpenSegment callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 125 ----------------------
 src/backend/replication/walsender.c    | 186 ---------------------------------
 src/bin/pg_waldump/pg_waldump.c        | 123 ----------------------
 3 files changed, 434 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 899bf1b551..c35852d7a1 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -17,14 +17,11 @@
  */
 #include "postgres.h"
 
-#include <unistd.h>
-
 #include "access/timeline.h"
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogutils.h"
 #include "miscadmin.h"
-#include "pgstat.h"
 #include "storage/smgr.h"
 #include "utils/guc.h"
 #include "utils/hsearch.h"
@@ -639,128 +636,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index c64a4bf9f8..68715aed9e 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -249,7 +249,6 @@ static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
 static void WalSndOpenSegment(XLogSegNo nextSegNo, TimeLineID *tli,
 							  XLogSegment *seg);
-static void XLogReadOld(char *buf, XLogRecPtr startptr, Size count);
 
 
 /* Initialize walsender process before entering the main command loop */
@@ -2351,191 +2350,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, wal_segment_size);
-
-		if (sendSeg->file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->num, sendSeg->size))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->file >= 0)
-				close(sendSeg->file);
-
-			XLByteToSeg(recptr, sendSeg->num, sendSeg->size);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, wal_segment_size);
-				if (sendSeg->num == endSegNo)
-					sendSeg->tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->tli, sendSeg->num, wal_segment_size);
-
-			sendSeg->file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->tli, sendSeg->num))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->off != startoff)
-		{
-			if (lseek(sendSeg->file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->tli, sendSeg->num),
-								startoff)));
-			sendSeg->off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (wal_segment_size - startoff))
-			segbytes = wal_segment_size - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->tli, sendSeg->num),
-							sendSeg->off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->tli, sendSeg->num),
-							sendSeg->off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, wal_segment_size);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->file >= 0)
-		{
-			close(sendSeg->file);
-			sendSeg->file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 3e09519f88..cd44090d36 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -14,7 +14,6 @@
 
 #include <dirent.h>
 #include <sys/stat.h>
-#include <unistd.h>
 
 #include "access/xlogreader.h"
 #include "access/xlogrecord.h"
@@ -335,128 +334,6 @@ XLogDumpOpenSegment(XLogSegNo nextSegNo, TimeLineID *tli, XLogSegment *seg)
 	seg->tli = *tli;
 }
 
-/*
- * Read count bytes from a segment file in the specified directory, for the
- * given timeline, containing the specified record pointer; store the data in
- * the passed buffer.
- */
-static void
-XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
-				 XLogRecPtr startptr, char *buf, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static uint32 sendOff = 0;
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, WalSegSz);
-
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, WalSegSz))
-		{
-			char		fname[MAXFNAMELEN];
-			int			tries;
-
-			/* Switch to another logfile segment */
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, WalSegSz);
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-			/*
-			 * In follow mode there is a short period of time after the server
-			 * has written the end of the previous file before the new file is
-			 * available. So we loop for 5 seconds looking for the file to
-			 * appear before giving up.
-			 */
-			for (tries = 0; tries < 10; tries++)
-			{
-				sendFile = open_file_in_directory(directory, fname);
-				if (sendFile >= 0)
-					break;
-				if (errno == ENOENT)
-				{
-					int			save_errno = errno;
-
-					/* File not there yet, try again */
-					pg_usleep(500 * 1000);
-
-					errno = save_errno;
-					continue;
-				}
-				/* Any other error, fall through and fail */
-				break;
-			}
-
-			if (sendFile < 0)
-				fatal_error("could not find file \"%s\": %s",
-							fname, strerror(errno));
-			sendOff = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				int			err = errno;
-				char		fname[MAXPGPATH];
-
-				XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-				fatal_error("could not seek in log file %s to offset %u: %s",
-							fname, startoff, strerror(err));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (WalSegSz - startoff))
-			segbytes = WalSegSz - startoff;
-		else
-			segbytes = nbytes;
-
-		readbytes = read(sendFile, p, segbytes);
-		if (readbytes <= 0)
-		{
-			int			err = errno;
-			char		fname[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-			errno = save_errno;
-
-			if (readbytes < 0)
-				fatal_error("could not read from log file %s, offset %u, length %d: %s",
-							fname, sendOff, segbytes, strerror(err));
-			else if (readbytes == 0)
-				fatal_error("could not read from log file %s, offset %u: read %d of %zu",
-							fname, sendOff, readbytes, (Size) segbytes);
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * XLogReader read_page callback
  */
-- 
2.16.4


--=-=-=--





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

* [PATCH 4/4] Remove the old implemenations of XLogRead().
@ 2019-09-09 09:53 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-09-09 09:53 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the XLogOpenSegment callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 125 -----------------
 src/backend/replication/walsender.c    | 186 -------------------------
 src/bin/pg_waldump/pg_waldump.c        | 123 ----------------
 3 files changed, 434 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 83b014e04c..5b0ba39f17 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -17,14 +17,11 @@
  */
 #include "postgres.h"
 
-#include <unistd.h>
-
 #include "access/timeline.h"
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogutils.h"
 #include "miscadmin.h"
-#include "pgstat.h"
 #include "storage/smgr.h"
 #include "utils/guc.h"
 #include "utils/hsearch.h"
@@ -639,128 +636,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 0685c320b4..072d46d853 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -249,7 +249,6 @@ static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
 
 static void WalSndOpenSegment(XLogSegNo nextSegNo, TimeLineID *tli,
 							  XLogSegment *seg);
-static void XLogReadOld(char *buf, XLogRecPtr startptr, Size count);
 
 
 /* Initialize walsender process before entering the main command loop */
@@ -2351,191 +2350,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, wal_segment_size);
-
-		if (sendSeg->file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->num, sendSeg->size))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->file >= 0)
-				close(sendSeg->file);
-
-			XLByteToSeg(recptr, sendSeg->num, sendSeg->size);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, wal_segment_size);
-				if (sendSeg->num == endSegNo)
-					sendSeg->tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->tli, sendSeg->num, wal_segment_size);
-
-			sendSeg->file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->tli, sendSeg->num))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->off != startoff)
-		{
-			if (lseek(sendSeg->file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->tli, sendSeg->num),
-								startoff)));
-			sendSeg->off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (wal_segment_size - startoff))
-			segbytes = wal_segment_size - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->tli, sendSeg->num),
-							sendSeg->off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->tli, sendSeg->num),
-							sendSeg->off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, wal_segment_size);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->file >= 0)
-		{
-			close(sendSeg->file);
-			sendSeg->file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 3e09519f88..cd44090d36 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -14,7 +14,6 @@
 
 #include <dirent.h>
 #include <sys/stat.h>
-#include <unistd.h>
 
 #include "access/xlogreader.h"
 #include "access/xlogrecord.h"
@@ -335,128 +334,6 @@ XLogDumpOpenSegment(XLogSegNo nextSegNo, TimeLineID *tli, XLogSegment *seg)
 	seg->tli = *tli;
 }
 
-/*
- * Read count bytes from a segment file in the specified directory, for the
- * given timeline, containing the specified record pointer; store the data in
- * the passed buffer.
- */
-static void
-XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
-				 XLogRecPtr startptr, char *buf, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static uint32 sendOff = 0;
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, WalSegSz);
-
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, WalSegSz))
-		{
-			char		fname[MAXFNAMELEN];
-			int			tries;
-
-			/* Switch to another logfile segment */
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, WalSegSz);
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-			/*
-			 * In follow mode there is a short period of time after the server
-			 * has written the end of the previous file before the new file is
-			 * available. So we loop for 5 seconds looking for the file to
-			 * appear before giving up.
-			 */
-			for (tries = 0; tries < 10; tries++)
-			{
-				sendFile = open_file_in_directory(directory, fname);
-				if (sendFile >= 0)
-					break;
-				if (errno == ENOENT)
-				{
-					int			save_errno = errno;
-
-					/* File not there yet, try again */
-					pg_usleep(500 * 1000);
-
-					errno = save_errno;
-					continue;
-				}
-				/* Any other error, fall through and fail */
-				break;
-			}
-
-			if (sendFile < 0)
-				fatal_error("could not find file \"%s\": %s",
-							fname, strerror(errno));
-			sendOff = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				int			err = errno;
-				char		fname[MAXPGPATH];
-
-				XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-				fatal_error("could not seek in log file %s to offset %u: %s",
-							fname, startoff, strerror(err));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (WalSegSz - startoff))
-			segbytes = WalSegSz - startoff;
-		else
-			segbytes = nbytes;
-
-		readbytes = read(sendFile, p, segbytes);
-		if (readbytes <= 0)
-		{
-			int			err = errno;
-			char		fname[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-			errno = save_errno;
-
-			if (readbytes < 0)
-				fatal_error("could not read from log file %s, offset %u, length %d: %s",
-							fname, sendOff, segbytes, strerror(err));
-			else if (readbytes == 0)
-				fatal_error("could not read from log file %s, offset %u: read %d of %zu",
-							fname, sendOff, readbytes, (Size) segbytes);
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * XLogReader read_page callback
  */
-- 
2.22.0


--=-=-=--





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

* [PATCH 6/6] Remove the old implemenations of XLogRead().
@ 2019-09-23 05:40 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-09-23 05:40 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the WALSegmentOpen callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 125 -----------------
 src/backend/replication/walsender.c    | 187 -------------------------
 src/bin/pg_waldump/pg_waldump.c        | 123 ----------------
 3 files changed, 435 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 38c3196168..078d58e34c 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -17,14 +17,11 @@
  */
 #include "postgres.h"
 
-#include <unistd.h>
-
 #include "access/timeline.h"
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogutils.h"
 #include "miscadmin.h"
-#include "pgstat.h"
 #include "storage/smgr.h"
 #include "utils/guc.h"
 #include "utils/hsearch.h"
@@ -639,128 +636,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index a7b3e0ecbe..3ea05add53 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -251,8 +251,6 @@ static void WalSndSegmentOpen(XLogSegNo nextSegNo, TimeLineID *tli_p,
 							  int *file_p, WALOpenSegment *seg);
 
 
-static void XLogReadOld(char *buf, XLogRecPtr startptr, Size count);
-
 /* Initialize walsender process before entering the main command loop */
 void
 InitWalSender(void)
@@ -2358,191 +2356,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, wal_segment_size);
-
-		if (sendSeg->file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->num, sendSeg->size))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->file >= 0)
-				close(sendSeg->file);
-
-			XLByteToSeg(recptr, sendSeg->num, sendSeg->size);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, wal_segment_size);
-				if (sendSeg->num == endSegNo)
-					sendSeg->tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->tli, sendSeg->num, wal_segment_size);
-
-			sendSeg->file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->tli, sendSeg->num))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->off != startoff)
-		{
-			if (lseek(sendSeg->file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->tli, sendSeg->num),
-								startoff)));
-			sendSeg->off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (wal_segment_size - startoff))
-			segbytes = wal_segment_size - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->tli, sendSeg->num),
-							sendSeg->off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->tli, sendSeg->num),
-							sendSeg->off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, wal_segment_size);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->file >= 0)
-		{
-			close(sendSeg->file);
-			sendSeg->file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index cd5f589f03..1de8eb350e 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -14,7 +14,6 @@
 
 #include <dirent.h>
 #include <sys/stat.h>
-#include <unistd.h>
 
 #include "access/xlogreader.h"
 #include "access/xlogrecord.h"
@@ -341,128 +340,6 @@ WALDumpOpenSegment(XLogSegNo nextSegNo, TimeLineID *tli_p, int *file_p,
 	*file_p = file;
 }
 
-/*
- * Read count bytes from a segment file in the specified directory, for the
- * given timeline, containing the specified record pointer; store the data in
- * the passed buffer.
- */
-static void
-XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
-				 XLogRecPtr startptr, char *buf, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static uint32 sendOff = 0;
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, WalSegSz);
-
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, WalSegSz))
-		{
-			char		fname[MAXFNAMELEN];
-			int			tries;
-
-			/* Switch to another logfile segment */
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, WalSegSz);
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-			/*
-			 * In follow mode there is a short period of time after the server
-			 * has written the end of the previous file before the new file is
-			 * available. So we loop for 5 seconds looking for the file to
-			 * appear before giving up.
-			 */
-			for (tries = 0; tries < 10; tries++)
-			{
-				sendFile = open_file_in_directory(directory, fname);
-				if (sendFile >= 0)
-					break;
-				if (errno == ENOENT)
-				{
-					int			save_errno = errno;
-
-					/* File not there yet, try again */
-					pg_usleep(500 * 1000);
-
-					errno = save_errno;
-					continue;
-				}
-				/* Any other error, fall through and fail */
-				break;
-			}
-
-			if (sendFile < 0)
-				fatal_error("could not find file \"%s\": %s",
-							fname, strerror(errno));
-			sendOff = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				int			err = errno;
-				char		fname[MAXPGPATH];
-
-				XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-				fatal_error("could not seek in log file %s to offset %u: %s",
-							fname, startoff, strerror(err));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (WalSegSz - startoff))
-			segbytes = WalSegSz - startoff;
-		else
-			segbytes = nbytes;
-
-		readbytes = read(sendFile, p, segbytes);
-		if (readbytes <= 0)
-		{
-			int			err = errno;
-			char		fname[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-			errno = save_errno;
-
-			if (readbytes < 0)
-				fatal_error("could not read from log file %s, offset %u, length %d: %s",
-							fname, sendOff, segbytes, strerror(err));
-			else if (readbytes == 0)
-				fatal_error("could not read from log file %s, offset %u: read %d of %zu",
-							fname, sendOff, readbytes, (Size) segbytes);
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * XLogReader read_page callback
  */
-- 
2.20.1


--=-=-=--





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

* [PATCH 2/2] Remove the old implemenations of XLogRead().
@ 2019-09-26 11:51 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-09-26 11:51 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the WALSegmentOpen callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 122 ----------------
 src/backend/replication/walsender.c    | 188 -------------------------
 src/bin/pg_waldump/pg_waldump.c        | 122 ----------------
 3 files changed, 432 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 09d42d3112..002b8f72a9 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -639,128 +639,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index dcb84693fc..d7df72fb87 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -253,9 +253,6 @@ static void WalSndSegmentOpen(XLogSegNo nextSegNo, TimeLineID *tli_p,
 							  WALSegmentContext *segcxt);
 
 
-static void XLogReadOld(WALSegmentContext *segcxt, char *buf,
-						XLogRecPtr startptr, Size count);
-
 /* Initialize walsender process before entering the main command loop */
 void
 InitWalSender(void)
@@ -2375,191 +2372,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(WALSegmentContext *segcxt, char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segcxt->ws_segsize);
-
-		if (sendSeg->ws_file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->ws_file >= 0)
-				close(sendSeg->ws_file);
-
-			XLByteToSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->ws_tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, segcxt->ws_segsize);
-				if (sendSeg->ws_segno == endSegNo)
-					sendSeg->ws_tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->ws_tli, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			sendSeg->ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->ws_file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->ws_off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->ws_off != startoff)
-		{
-			if (lseek(sendSeg->ws_file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-								startoff)));
-			sendSeg->ws_off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segcxt->ws_segsize - startoff))
-			segbytes = segcxt->ws_segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->ws_file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->ws_off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, segcxt->ws_segsize);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->ws_file >= 0)
-		{
-			close(sendSeg->ws_file);
-			sendSeg->ws_file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index c97376101c..57dd7df56f 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -321,128 +321,6 @@ WALDumpOpenSegment(XLogSegNo nextSegNo, TimeLineID *tli_p, int *file_p,
 					fname, strerror(errno));
 }
 
-/*
- * Read count bytes from a segment file in the specified directory, for the
- * given timeline, containing the specified record pointer; store the data in
- * the passed buffer.
- */
-static void
-XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
-				 XLogRecPtr startptr, char *buf, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static uint32 sendOff = 0;
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, WalSegSz);
-
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, WalSegSz))
-		{
-			char		fname[MAXFNAMELEN];
-			int			tries;
-
-			/* Switch to another logfile segment */
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, WalSegSz);
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-			/*
-			 * In follow mode there is a short period of time after the server
-			 * has written the end of the previous file before the new file is
-			 * available. So we loop for 5 seconds looking for the file to
-			 * appear before giving up.
-			 */
-			for (tries = 0; tries < 10; tries++)
-			{
-				sendFile = open_file_in_directory(directory, fname);
-				if (sendFile >= 0)
-					break;
-				if (errno == ENOENT)
-				{
-					int			save_errno = errno;
-
-					/* File not there yet, try again */
-					pg_usleep(500 * 1000);
-
-					errno = save_errno;
-					continue;
-				}
-				/* Any other error, fall through and fail */
-				break;
-			}
-
-			if (sendFile < 0)
-				fatal_error("could not find file \"%s\": %s",
-							fname, strerror(errno));
-			sendOff = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				int			err = errno;
-				char		fname[MAXPGPATH];
-
-				XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-				fatal_error("could not seek in log file %s to offset %u: %s",
-							fname, startoff, strerror(err));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (WalSegSz - startoff))
-			segbytes = WalSegSz - startoff;
-		else
-			segbytes = nbytes;
-
-		readbytes = read(sendFile, p, segbytes);
-		if (readbytes <= 0)
-		{
-			int			err = errno;
-			char		fname[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-			errno = save_errno;
-
-			if (readbytes < 0)
-				fatal_error("could not read from log file %s, offset %u, length %d: %s",
-							fname, sendOff, segbytes, strerror(err));
-			else if (readbytes == 0)
-				fatal_error("could not read from log file %s, offset %u: read %d of %zu",
-							fname, sendOff, readbytes, (Size) segbytes);
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * XLogReader read_page callback
  */
-- 
2.20.1


--=-=-=--





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

* [PATCH 2/2] Remove the old implemenations of XLogRead().
@ 2019-10-04 10:07 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-10-04 10:07 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the WALSegmentOpen callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 122 ----------------
 src/backend/replication/walsender.c    | 188 -------------------------
 src/bin/pg_waldump/pg_waldump.c        | 122 ----------------
 3 files changed, 432 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 007974ea99..0211a68640 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -639,128 +639,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index b21b143f99..76a5477389 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -252,9 +252,6 @@ static void WalSndSegmentOpen(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 							  TimeLineID *tli_p, int *file_p);
 
 
-static void XLogReadOld(WALSegmentContext *segcxt, char *buf,
-						XLogRecPtr startptr, Size count);
-
 /* Initialize walsender process before entering the main command loop */
 void
 InitWalSender(void)
@@ -2377,191 +2374,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(WALSegmentContext *segcxt, char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segcxt->ws_segsize);
-
-		if (sendSeg->ws_file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->ws_file >= 0)
-				close(sendSeg->ws_file);
-
-			XLByteToSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->ws_tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, segcxt->ws_segsize);
-				if (sendSeg->ws_segno == endSegNo)
-					sendSeg->ws_tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->ws_tli, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			sendSeg->ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->ws_file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->ws_off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->ws_off != startoff)
-		{
-			if (lseek(sendSeg->ws_file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-								startoff)));
-			sendSeg->ws_off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segcxt->ws_segsize - startoff))
-			segbytes = segcxt->ws_segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->ws_file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->ws_off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, segcxt->ws_segsize);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->ws_file >= 0)
-		{
-			close(sendSeg->ws_file);
-			sendSeg->ws_file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 4c49d1acf4..39686c235c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -321,128 +321,6 @@ WALDumpOpenSegment(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 					fname, strerror(errno));
 }
 
-/*
- * Read count bytes from a segment file in the specified directory, for the
- * given timeline, containing the specified record pointer; store the data in
- * the passed buffer.
- */
-static void
-XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
-				 XLogRecPtr startptr, char *buf, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static uint32 sendOff = 0;
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, WalSegSz);
-
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, WalSegSz))
-		{
-			char		fname[MAXFNAMELEN];
-			int			tries;
-
-			/* Switch to another logfile segment */
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, WalSegSz);
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-			/*
-			 * In follow mode there is a short period of time after the server
-			 * has written the end of the previous file before the new file is
-			 * available. So we loop for 5 seconds looking for the file to
-			 * appear before giving up.
-			 */
-			for (tries = 0; tries < 10; tries++)
-			{
-				sendFile = open_file_in_directory(directory, fname);
-				if (sendFile >= 0)
-					break;
-				if (errno == ENOENT)
-				{
-					int			save_errno = errno;
-
-					/* File not there yet, try again */
-					pg_usleep(500 * 1000);
-
-					errno = save_errno;
-					continue;
-				}
-				/* Any other error, fall through and fail */
-				break;
-			}
-
-			if (sendFile < 0)
-				fatal_error("could not find file \"%s\": %s",
-							fname, strerror(errno));
-			sendOff = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				int			err = errno;
-				char		fname[MAXPGPATH];
-
-				XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-				fatal_error("could not seek in log file %s to offset %u: %s",
-							fname, startoff, strerror(err));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (WalSegSz - startoff))
-			segbytes = WalSegSz - startoff;
-		else
-			segbytes = nbytes;
-
-		readbytes = read(sendFile, p, segbytes);
-		if (readbytes <= 0)
-		{
-			int			err = errno;
-			char		fname[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-			errno = save_errno;
-
-			if (readbytes < 0)
-				fatal_error("could not read from log file %s, offset %u, length %d: %s",
-							fname, sendOff, segbytes, strerror(err));
-			else if (readbytes == 0)
-				fatal_error("could not read from log file %s, offset %u: read %d of %zu",
-							fname, sendOff, readbytes, (Size) segbytes);
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * XLogReader read_page callback
  */
-- 
2.20.1


--=-=-=--





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

* [PATCH 2/2] Remove the old implemenations of XLogRead().
@ 2019-10-11 10:07 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-10-11 10:07 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the WALSegmentOpen callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 122 ----------------
 src/backend/replication/walsender.c    | 188 -------------------------
 src/bin/pg_waldump/pg_waldump.c        | 122 ----------------
 3 files changed, 432 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 007974ea99..0211a68640 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -639,128 +639,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index b21b143f99..76a5477389 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -252,9 +252,6 @@ static void WalSndSegmentOpen(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 							  TimeLineID *tli_p, int *file_p);
 
 
-static void XLogReadOld(WALSegmentContext *segcxt, char *buf,
-						XLogRecPtr startptr, Size count);
-
 /* Initialize walsender process before entering the main command loop */
 void
 InitWalSender(void)
@@ -2377,191 +2374,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(WALSegmentContext *segcxt, char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segcxt->ws_segsize);
-
-		if (sendSeg->ws_file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->ws_file >= 0)
-				close(sendSeg->ws_file);
-
-			XLByteToSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->ws_tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, segcxt->ws_segsize);
-				if (sendSeg->ws_segno == endSegNo)
-					sendSeg->ws_tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->ws_tli, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			sendSeg->ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->ws_file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->ws_off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->ws_off != startoff)
-		{
-			if (lseek(sendSeg->ws_file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-								startoff)));
-			sendSeg->ws_off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segcxt->ws_segsize - startoff))
-			segbytes = segcxt->ws_segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->ws_file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->ws_off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, segcxt->ws_segsize);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->ws_file >= 0)
-		{
-			close(sendSeg->ws_file);
-			sendSeg->ws_file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 4c49d1acf4..39686c235c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -321,128 +321,6 @@ WALDumpOpenSegment(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 					fname, strerror(errno));
 }
 
-/*
- * Read count bytes from a segment file in the specified directory, for the
- * given timeline, containing the specified record pointer; store the data in
- * the passed buffer.
- */
-static void
-XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
-				 XLogRecPtr startptr, char *buf, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static uint32 sendOff = 0;
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, WalSegSz);
-
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, WalSegSz))
-		{
-			char		fname[MAXFNAMELEN];
-			int			tries;
-
-			/* Switch to another logfile segment */
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, WalSegSz);
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-			/*
-			 * In follow mode there is a short period of time after the server
-			 * has written the end of the previous file before the new file is
-			 * available. So we loop for 5 seconds looking for the file to
-			 * appear before giving up.
-			 */
-			for (tries = 0; tries < 10; tries++)
-			{
-				sendFile = open_file_in_directory(directory, fname);
-				if (sendFile >= 0)
-					break;
-				if (errno == ENOENT)
-				{
-					int			save_errno = errno;
-
-					/* File not there yet, try again */
-					pg_usleep(500 * 1000);
-
-					errno = save_errno;
-					continue;
-				}
-				/* Any other error, fall through and fail */
-				break;
-			}
-
-			if (sendFile < 0)
-				fatal_error("could not find file \"%s\": %s",
-							fname, strerror(errno));
-			sendOff = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				int			err = errno;
-				char		fname[MAXPGPATH];
-
-				XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-				fatal_error("could not seek in log file %s to offset %u: %s",
-							fname, startoff, strerror(err));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (WalSegSz - startoff))
-			segbytes = WalSegSz - startoff;
-		else
-			segbytes = nbytes;
-
-		readbytes = read(sendFile, p, segbytes);
-		if (readbytes <= 0)
-		{
-			int			err = errno;
-			char		fname[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-			errno = save_errno;
-
-			if (readbytes < 0)
-				fatal_error("could not read from log file %s, offset %u, length %d: %s",
-							fname, sendOff, segbytes, strerror(err));
-			else if (readbytes == 0)
-				fatal_error("could not read from log file %s, offset %u: read %d of %zu",
-							fname, sendOff, readbytes, (Size) segbytes);
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * XLogReader read_page callback
  */
-- 
2.20.1


--=-=-=--





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

* [PATCH 2/2] Remove the old implemenations of XLogRead().
@ 2019-11-12 10:51 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-11-12 10:51 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the WALSegmentOpen callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 122 ----------------
 src/backend/replication/walsender.c    | 188 -------------------------
 src/bin/pg_waldump/pg_waldump.c        | 122 ----------------
 3 files changed, 432 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 18e436f4fd..98118f484e 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -639,128 +639,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 92a539aa5c..80ef5eb909 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -252,9 +252,6 @@ static void WalSndSegmentOpen(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 							  TimeLineID *tli_p, int *file_p);
 
 
-static void XLogReadOld(WALSegmentContext *segcxt, char *buf,
-						XLogRecPtr startptr, Size count);
-
 /* Initialize walsender process before entering the main command loop */
 void
 InitWalSender(void)
@@ -2376,191 +2373,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(WALSegmentContext *segcxt, char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segcxt->ws_segsize);
-
-		if (sendSeg->ws_file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->ws_file >= 0)
-				close(sendSeg->ws_file);
-
-			XLByteToSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->ws_tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, segcxt->ws_segsize);
-				if (sendSeg->ws_segno == endSegNo)
-					sendSeg->ws_tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->ws_tli, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			sendSeg->ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->ws_file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->ws_off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->ws_off != startoff)
-		{
-			if (lseek(sendSeg->ws_file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-								startoff)));
-			sendSeg->ws_off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segcxt->ws_segsize - startoff))
-			segbytes = segcxt->ws_segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->ws_file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->ws_off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, segcxt->ws_segsize);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->ws_file >= 0)
-		{
-			close(sendSeg->ws_file);
-			sendSeg->ws_file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index c0c2590d56..f1908a8868 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -320,128 +320,6 @@ WALDumpOpenSegment(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 					fname, strerror(errno));
 }
 
-/*
- * Read count bytes from a segment file in the specified directory, for the
- * given timeline, containing the specified record pointer; store the data in
- * the passed buffer.
- */
-static void
-XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
-				 XLogRecPtr startptr, char *buf, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static uint32 sendOff = 0;
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, WalSegSz);
-
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, WalSegSz))
-		{
-			char		fname[MAXFNAMELEN];
-			int			tries;
-
-			/* Switch to another logfile segment */
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, WalSegSz);
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-			/*
-			 * In follow mode there is a short period of time after the server
-			 * has written the end of the previous file before the new file is
-			 * available. So we loop for 5 seconds looking for the file to
-			 * appear before giving up.
-			 */
-			for (tries = 0; tries < 10; tries++)
-			{
-				sendFile = open_file_in_directory(directory, fname);
-				if (sendFile >= 0)
-					break;
-				if (errno == ENOENT)
-				{
-					int			save_errno = errno;
-
-					/* File not there yet, try again */
-					pg_usleep(500 * 1000);
-
-					errno = save_errno;
-					continue;
-				}
-				/* Any other error, fall through and fail */
-				break;
-			}
-
-			if (sendFile < 0)
-				fatal_error("could not find file \"%s\": %s",
-							fname, strerror(errno));
-			sendOff = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				int			err = errno;
-				char		fname[MAXPGPATH];
-
-				XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-
-				fatal_error("could not seek in log file %s to offset %u: %s",
-							fname, startoff, strerror(err));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (WalSegSz - startoff))
-			segbytes = WalSegSz - startoff;
-		else
-			segbytes = nbytes;
-
-		readbytes = read(sendFile, p, segbytes);
-		if (readbytes <= 0)
-		{
-			int			err = errno;
-			char		fname[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFileName(fname, timeline_id, sendSegNo, WalSegSz);
-			errno = save_errno;
-
-			if (readbytes < 0)
-				fatal_error("could not read from log file %s, offset %u, length %d: %s",
-							fname, sendOff, segbytes, strerror(err));
-			else if (readbytes == 0)
-				fatal_error("could not read from log file %s, offset %u: read %d of %zu",
-							fname, sendOff, readbytes, (Size) segbytes);
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * XLogReader read_page callback
  */
-- 
2.20.1


--=-=-=--





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

* [PATCH 2/2] Remove the old implemenations of XLogRead().
@ 2019-11-20 14:11 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 28+ messages in thread

From: Antonin Houska @ 2019-11-20 14:11 UTC (permalink / raw)

Done in a separate patch because the diff looks harder to read if one function
(XLogRead) is removed and another one (the WALSegmentOpen callback) is added
nearby at the same time (the addition and removal of code can get mixed in the
diff).
---
 src/backend/access/transam/xlogutils.c | 122 ----------------
 src/backend/replication/walsender.c    | 188 -------------------------
 2 files changed, 310 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index baca17260c..a16d47f156 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -639,128 +639,6 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
 	forget_invalid_pages(rnode, forkNum, nblocks);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- * in timeline 'tli'.
- *
- * Will open, and keep open, one WAL segment stored in the static file
- * descriptor 'sendFile'. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- *
- * XXX This is very similar to pg_waldump's XLogDumpXLogRead and to XLogRead
- * in walsender.c but for small differences (such as lack of elog() in
- * frontend).  Probably these should be merged at some point.
- */
-static void
-XLogReadOld(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
-			Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-
-	/* state maintained across calls */
-	static int	sendFile = -1;
-	static XLogSegNo sendSegNo = 0;
-	static TimeLineID sendTLI = 0;
-	static uint32 sendOff = 0;
-
-	Assert(segsize == wal_segment_size);
-
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segsize);
-
-		/* Do we need to switch to a different xlog segment? */
-		if (sendFile < 0 || !XLByteInSeg(recptr, sendSegNo, segsize) ||
-			sendTLI != tli)
-		{
-			char		path[MAXPGPATH];
-
-			if (sendFile >= 0)
-				close(sendFile);
-
-			XLByteToSeg(recptr, sendSegNo, segsize);
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-
-			sendFile = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-
-			if (sendFile < 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)));
-			}
-			sendOff = 0;
-			sendTLI = tli;
-		}
-
-		/* Need to seek in the file? */
-		if (sendOff != startoff)
-		{
-			if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
-			{
-				char		path[MAXPGPATH];
-				int			save_errno = errno;
-
-				XLogFilePath(path, tli, sendSegNo, segsize);
-				errno = save_errno;
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								path, startoff)));
-			}
-			sendOff = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segsize - startoff))
-			segbytes = segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendFile, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes <= 0)
-		{
-			char		path[MAXPGPATH];
-			int			save_errno = errno;
-
-			XLogFilePath(path, tli, sendSegNo, segsize);
-			errno = save_errno;
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %lu: %m",
-							path, sendOff, (unsigned long) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendOff += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-}
-
 /*
  * Determine which timeline to read an xlog page from and set the
  * XLogReaderState's currTLI to that timeline ID.
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 11611072b0..d2426b0960 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -252,9 +252,6 @@ static int	WalSndSegmentOpen(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
 							  TimeLineID *tli_p);
 
 
-static void XLogReadOld(WALSegmentContext *segcxt, char *buf,
-						XLogRecPtr startptr, Size count);
-
 /* Initialize walsender process before entering the main command loop */
 void
 InitWalSender(void)
@@ -2376,191 +2373,6 @@ WalSndKill(int code, Datum arg)
 	SpinLockRelease(&walsnd->mutex);
 }
 
-/*
- * Read 'count' bytes from WAL into 'buf', starting at location 'startptr'
- *
- * XXX probably this should be improved to suck data directly from the
- * WAL buffers when possible.
- *
- * Will open, and keep open, one WAL segment stored in the global file
- * descriptor sendFile. This means if XLogRead is used once, there will
- * always be one descriptor left open until the process ends, but never
- * more than one.
- */
-static void
-XLogReadOld(WALSegmentContext *segcxt, char *buf, XLogRecPtr startptr, Size count)
-{
-	char	   *p;
-	XLogRecPtr	recptr;
-	Size		nbytes;
-	XLogSegNo	segno;
-
-retry:
-	p = buf;
-	recptr = startptr;
-	nbytes = count;
-
-	while (nbytes > 0)
-	{
-		uint32		startoff;
-		int			segbytes;
-		int			readbytes;
-
-		startoff = XLogSegmentOffset(recptr, segcxt->ws_segsize);
-
-		if (sendSeg->ws_file < 0 ||
-			!XLByteInSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize))
-		{
-			char		path[MAXPGPATH];
-
-			/* Switch to another logfile segment */
-			if (sendSeg->ws_file >= 0)
-				close(sendSeg->ws_file);
-
-			XLByteToSeg(recptr, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			/*-------
-			 * When reading from a historic timeline, and there is a timeline
-			 * switch within this segment, read from the WAL segment belonging
-			 * to the new timeline.
-			 *
-			 * For example, imagine that this server is currently on timeline
-			 * 5, and we're streaming timeline 4. The switch from timeline 4
-			 * to 5 happened at 0/13002088. In pg_wal, we have these files:
-			 *
-			 * ...
-			 * 000000040000000000000012
-			 * 000000040000000000000013
-			 * 000000050000000000000013
-			 * 000000050000000000000014
-			 * ...
-			 *
-			 * In this situation, when requested to send the WAL from
-			 * segment 0x13, on timeline 4, we read the WAL from file
-			 * 000000050000000000000013. Archive recovery prefers files from
-			 * newer timelines, so if the segment was restored from the
-			 * archive on this server, the file belonging to the old timeline,
-			 * 000000040000000000000013, might not exist. Their contents are
-			 * equal up to the switchpoint, because at a timeline switch, the
-			 * used portion of the old segment is copied to the new file.
-			 *-------
-			 */
-			sendSeg->ws_tli = sendTimeLine;
-			if (sendTimeLineIsHistoric)
-			{
-				XLogSegNo	endSegNo;
-
-				XLByteToSeg(sendTimeLineValidUpto, endSegNo, segcxt->ws_segsize);
-				if (sendSeg->ws_segno == endSegNo)
-					sendSeg->ws_tli = sendTimeLineNextTLI;
-			}
-
-			XLogFilePath(path, sendSeg->ws_tli, sendSeg->ws_segno, segcxt->ws_segsize);
-
-			sendSeg->ws_file = BasicOpenFile(path, O_RDONLY | PG_BINARY);
-			if (sendSeg->ws_file < 0)
-			{
-				/*
-				 * If the file is not found, assume it's because the standby
-				 * asked for a too old WAL segment that has already been
-				 * removed or recycled.
-				 */
-				if (errno == ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("requested WAL segment %s has already been removed",
-									XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno))));
-				else
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open file \"%s\": %m",
-									path)));
-			}
-			sendSeg->ws_off = 0;
-		}
-
-		/* Need to seek in the file? */
-		if (sendSeg->ws_off != startoff)
-		{
-			if (lseek(sendSeg->ws_file, (off_t) startoff, SEEK_SET) < 0)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not seek in log segment %s to offset %u: %m",
-								XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-								startoff)));
-			sendSeg->ws_off = startoff;
-		}
-
-		/* How many bytes are within this segment? */
-		if (nbytes > (segcxt->ws_segsize - startoff))
-			segbytes = segcxt->ws_segsize - startoff;
-		else
-			segbytes = nbytes;
-
-		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
-		readbytes = read(sendSeg->ws_file, p, segbytes);
-		pgstat_report_wait_end();
-		if (readbytes < 0)
-		{
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read from log segment %s, offset %u, length %zu: %m",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, (Size) segbytes)));
-		}
-		else if (readbytes == 0)
-		{
-			ereport(ERROR,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read from log segment %s, offset %u: read %d of %zu",
-							XLogFileNameP(sendSeg->ws_tli, sendSeg->ws_segno),
-							sendSeg->ws_off, readbytes, (Size) segbytes)));
-		}
-
-		/* Update state for read */
-		recptr += readbytes;
-
-		sendSeg->ws_off += readbytes;
-		nbytes -= readbytes;
-		p += readbytes;
-	}
-
-	/*
-	 * After reading into the buffer, check that what we read was valid. We do
-	 * this after reading, because even though the segment was present when we
-	 * opened it, it might get recycled or removed while we read it. The
-	 * read() succeeds in that case, but the data we tried to read might
-	 * already have been overwritten with new WAL records.
-	 */
-	XLByteToSeg(startptr, segno, segcxt->ws_segsize);
-	CheckXLogRemoved(segno, ThisTimeLineID);
-
-	/*
-	 * During recovery, the currently-open WAL file might be replaced with the
-	 * file of the same name retrieved from archive. So we always need to
-	 * check what we read was valid after reading into the buffer. If it's
-	 * invalid, we try to open and read the file again.
-	 */
-	if (am_cascading_walsender)
-	{
-		WalSnd	   *walsnd = MyWalSnd;
-		bool		reload;
-
-		SpinLockAcquire(&walsnd->mutex);
-		reload = walsnd->needreload;
-		walsnd->needreload = false;
-		SpinLockRelease(&walsnd->mutex);
-
-		if (reload && sendSeg->ws_file >= 0)
-		{
-			close(sendSeg->ws_file);
-			sendSeg->ws_file = -1;
-
-			goto retry;
-		}
-	}
-}
-
 /*
  * Callback for XLogRead() to open the next segment.
  */
-- 
2.20.1


--=-=-=--





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

* [PATCH v1 4/9] ci: switch tasks to debugoptimized build
@ 2023-08-08 00:27 Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 28+ messages in thread

From: Andres Freund @ 2023-08-08 00:27 UTC (permalink / raw)

In aggregate the CI tasks burn a lot of cpu hours. Compared to that easy to
read backtraces aren't as important. Still use -ggdb where appropriate, as
that does make backtraces more reliable, particularly in the face of
optimization.

Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
 .cirrus.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index f3d63ff3fb0..bfe251f48e8 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -140,7 +140,7 @@ task:
 
     CCACHE_DIR: /tmp/ccache_dir
     CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
-    CFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
 
   depends_on: SanityCheck
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*freebsd.*'
@@ -181,7 +181,7 @@ task:
   configure_script: |
     su postgres <<-EOF
       meson setup \
-        --buildtype=debug \
+        --buildtype=debugoptimized \
         -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
         -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
         -Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
@@ -275,7 +275,7 @@ task:
     ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0
 
     # SANITIZER_FLAGS is set in the tasks below
-    CFLAGS: -Og -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
+    CFLAGS: -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
     CXXFLAGS: $CFLAGS
     LDFLAGS: $SANITIZER_FLAGS
     CC: ccache gcc
@@ -364,7 +364,7 @@ task:
       configure_script: |
         su postgres <<-EOF
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
@@ -377,7 +377,7 @@ task:
         su postgres <<-EOF
           export CC='ccache gcc -m32'
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -Dllvm=disabled \
@@ -434,8 +434,8 @@ task:
 
     CC: ccache cc
     CXX: ccache c++
-    CFLAGS: -Og -ggdb
-    CXXFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
+    CXXFLAGS: -ggdb
 
   depends_on: SanityCheck
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
@@ -487,7 +487,7 @@ task:
   configure_script: |
     export PKG_CONFIG_PATH="/opt/local/lib/pkgconfig/"
     meson setup \
-      --buildtype=debug \
+      --buildtype=debugoptimized \
       -Dextra_include_dirs=/opt/local/include \
       -Dextra_lib_dirs=/opt/local/lib \
       -Dcassert=true \
-- 
2.38.0


--uu4yojthqnm7ulmd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0005-ci-Move-use-of-Dsegsize_blocks-6-from-macos-to-li.patch"



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

* [PATCH v3 08/10] ci: switch tasks to debugoptimized build
@ 2023-08-08 00:27 Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 28+ messages in thread

From: Andres Freund @ 2023-08-08 00:27 UTC (permalink / raw)

In aggregate the CI tasks burn a lot of cpu hours. Compared to that easy to
read backtraces aren't as important. Still use -ggdb where appropriate, as
that does make backtraces more reliable, particularly in the face of
optimization.

Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
 .cirrus.tasks.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e137769850d..d1730ce08a8 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -136,7 +136,7 @@ task:
 
     CCACHE_DIR: /tmp/ccache_dir
     CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
-    CFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
 
   <<: *freebsd_task_template
 
@@ -171,7 +171,7 @@ task:
   configure_script: |
     su postgres <<-EOF
       meson setup \
-        --buildtype=debug \
+        --buildtype=debugoptimized \
         -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
         -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
         -Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
@@ -266,7 +266,7 @@ task:
     ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0
 
     # SANITIZER_FLAGS is set in the tasks below
-    CFLAGS: -Og -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
+    CFLAGS: -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
     CXXFLAGS: $CFLAGS
     LDFLAGS: $SANITIZER_FLAGS
     CC: ccache gcc
@@ -356,7 +356,7 @@ task:
       configure_script: |
         su postgres <<-EOF
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
@@ -369,7 +369,7 @@ task:
         su postgres <<-EOF
           export CC='ccache gcc -m32'
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -Dllvm=disabled \
@@ -427,8 +427,8 @@ task:
 
     CC: ccache cc
     CXX: ccache c++
-    CFLAGS: -Og -ggdb
-    CXXFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
+    CXXFLAGS: -ggdb
 
   <<: *macos_task_template
 
@@ -479,7 +479,7 @@ task:
   configure_script: |
     export PKG_CONFIG_PATH="/opt/local/lib/pkgconfig/"
     meson setup \
-      --buildtype=debug \
+      --buildtype=debugoptimized \
       -Dextra_include_dirs=/opt/local/include \
       -Dextra_lib_dirs=/opt/local/lib \
       -Dcassert=true \
-- 
2.38.0


--uh2yukyzfvojbe2k
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0009-ci-windows-Disabling-write-cache-flushing-during-.patch"



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

* [PATCH v3 08/10] ci: switch tasks to debugoptimized build
@ 2023-08-08 00:27 Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 28+ messages in thread

From: Andres Freund @ 2023-08-08 00:27 UTC (permalink / raw)

In aggregate the CI tasks burn a lot of cpu hours. Compared to that easy to
read backtraces aren't as important. Still use -ggdb where appropriate, as
that does make backtraces more reliable, particularly in the face of
optimization.

Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
 .cirrus.tasks.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e137769850d..d1730ce08a8 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -136,7 +136,7 @@ task:
 
     CCACHE_DIR: /tmp/ccache_dir
     CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
-    CFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
 
   <<: *freebsd_task_template
 
@@ -171,7 +171,7 @@ task:
   configure_script: |
     su postgres <<-EOF
       meson setup \
-        --buildtype=debug \
+        --buildtype=debugoptimized \
         -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
         -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
         -Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
@@ -266,7 +266,7 @@ task:
     ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0
 
     # SANITIZER_FLAGS is set in the tasks below
-    CFLAGS: -Og -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
+    CFLAGS: -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
     CXXFLAGS: $CFLAGS
     LDFLAGS: $SANITIZER_FLAGS
     CC: ccache gcc
@@ -356,7 +356,7 @@ task:
       configure_script: |
         su postgres <<-EOF
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
@@ -369,7 +369,7 @@ task:
         su postgres <<-EOF
           export CC='ccache gcc -m32'
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -Dllvm=disabled \
@@ -427,8 +427,8 @@ task:
 
     CC: ccache cc
     CXX: ccache c++
-    CFLAGS: -Og -ggdb
-    CXXFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
+    CXXFLAGS: -ggdb
 
   <<: *macos_task_template
 
@@ -479,7 +479,7 @@ task:
   configure_script: |
     export PKG_CONFIG_PATH="/opt/local/lib/pkgconfig/"
     meson setup \
-      --buildtype=debug \
+      --buildtype=debugoptimized \
       -Dextra_include_dirs=/opt/local/include \
       -Dextra_lib_dirs=/opt/local/lib \
       -Dcassert=true \
-- 
2.38.0


--uh2yukyzfvojbe2k
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0009-ci-windows-Disabling-write-cache-flushing-during-.patch"



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

* [PATCH v1 4/9] ci: switch tasks to debugoptimized build
@ 2023-08-08 00:27 Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 28+ messages in thread

From: Andres Freund @ 2023-08-08 00:27 UTC (permalink / raw)

In aggregate the CI tasks burn a lot of cpu hours. Compared to that easy to
read backtraces aren't as important. Still use -ggdb where appropriate, as
that does make backtraces more reliable, particularly in the face of
optimization.

Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
 .cirrus.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index f3d63ff3fb0..bfe251f48e8 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -140,7 +140,7 @@ task:
 
     CCACHE_DIR: /tmp/ccache_dir
     CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
-    CFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
 
   depends_on: SanityCheck
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*freebsd.*'
@@ -181,7 +181,7 @@ task:
   configure_script: |
     su postgres <<-EOF
       meson setup \
-        --buildtype=debug \
+        --buildtype=debugoptimized \
         -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
         -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
         -Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
@@ -275,7 +275,7 @@ task:
     ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0
 
     # SANITIZER_FLAGS is set in the tasks below
-    CFLAGS: -Og -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
+    CFLAGS: -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
     CXXFLAGS: $CFLAGS
     LDFLAGS: $SANITIZER_FLAGS
     CC: ccache gcc
@@ -364,7 +364,7 @@ task:
       configure_script: |
         su postgres <<-EOF
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
@@ -377,7 +377,7 @@ task:
         su postgres <<-EOF
           export CC='ccache gcc -m32'
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -Dllvm=disabled \
@@ -434,8 +434,8 @@ task:
 
     CC: ccache cc
     CXX: ccache c++
-    CFLAGS: -Og -ggdb
-    CXXFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
+    CXXFLAGS: -ggdb
 
   depends_on: SanityCheck
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
@@ -487,7 +487,7 @@ task:
   configure_script: |
     export PKG_CONFIG_PATH="/opt/local/lib/pkgconfig/"
     meson setup \
-      --buildtype=debug \
+      --buildtype=debugoptimized \
       -Dextra_include_dirs=/opt/local/include \
       -Dextra_lib_dirs=/opt/local/lib \
       -Dcassert=true \
-- 
2.38.0


--uu4yojthqnm7ulmd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0005-ci-Move-use-of-Dsegsize_blocks-6-from-macos-to-li.patch"



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

* [PATCH v1 4/9] ci: switch tasks to debugoptimized build
@ 2023-08-08 00:27 Andres Freund <andres@anarazel.de>
  0 siblings, 0 replies; 28+ messages in thread

From: Andres Freund @ 2023-08-08 00:27 UTC (permalink / raw)

In aggregate the CI tasks burn a lot of cpu hours. Compared to that easy to
read backtraces aren't as important. Still use -ggdb where appropriate, as
that does make backtraces more reliable, particularly in the face of
optimization.

Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
 .cirrus.yml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index f3d63ff3fb0..bfe251f48e8 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -140,7 +140,7 @@ task:
 
     CCACHE_DIR: /tmp/ccache_dir
     CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
-    CFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
 
   depends_on: SanityCheck
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*freebsd.*'
@@ -181,7 +181,7 @@ task:
   configure_script: |
     su postgres <<-EOF
       meson setup \
-        --buildtype=debug \
+        --buildtype=debugoptimized \
         -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
         -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
         -Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
@@ -275,7 +275,7 @@ task:
     ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0
 
     # SANITIZER_FLAGS is set in the tasks below
-    CFLAGS: -Og -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
+    CFLAGS: -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
     CXXFLAGS: $CFLAGS
     LDFLAGS: $SANITIZER_FLAGS
     CC: ccache gcc
@@ -364,7 +364,7 @@ task:
       configure_script: |
         su postgres <<-EOF
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
@@ -377,7 +377,7 @@ task:
         su postgres <<-EOF
           export CC='ccache gcc -m32'
           meson setup \
-            --buildtype=debug \
+            --buildtype=debugoptimized \
             -Dcassert=true \
             ${LINUX_MESON_FEATURES} \
             -Dllvm=disabled \
@@ -434,8 +434,8 @@ task:
 
     CC: ccache cc
     CXX: ccache c++
-    CFLAGS: -Og -ggdb
-    CXXFLAGS: -Og -ggdb
+    CFLAGS: -ggdb
+    CXXFLAGS: -ggdb
 
   depends_on: SanityCheck
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
@@ -487,7 +487,7 @@ task:
   configure_script: |
     export PKG_CONFIG_PATH="/opt/local/lib/pkgconfig/"
     meson setup \
-      --buildtype=debug \
+      --buildtype=debugoptimized \
       -Dextra_include_dirs=/opt/local/include \
       -Dextra_lib_dirs=/opt/local/lib \
       -Dcassert=true \
-- 
2.38.0


--uu4yojthqnm7ulmd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0005-ci-Move-use-of-Dsegsize_blocks-6-from-macos-to-li.patch"



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

* [PATCH] doc: Improve "Partition Maintenance" section
@ 2024-03-28 17:43 Alvaro Herrera <alvherre@alvh.no-ip.org>
  0 siblings, 0 replies; 28+ messages in thread

From: Alvaro Herrera @ 2024-03-28 17:43 UTC (permalink / raw)

This adds some reference links and clarifies the wording a bit.

Author: Robert Treat <rob@xzilla.net>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://postgr.es/m/CABV9wwNGn-pweak6_pvL5PJ1mivDNPKfg0Tck_1oTUETv5Y=dg@mail.gmail.com
---
 doc/src/sgml/ddl.sgml | 72 +++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 8616a8e9cc..c671122d86 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4283,18 +4283,20 @@ CREATE TABLE measurement_y2008m02 PARTITION OF measurement
     TABLESPACE fasttablespace;
 </programlisting>
 
-     As an alternative, it is sometimes more convenient to create the
-     new table outside the partition structure, and attach it as a
-     partition later. This allows new data to be loaded, checked, and
-     transformed prior to it appearing in the partitioned table.
+     As an alternative to creating a new partition, it is sometimes more
+     convenient to create a new table separate from the partition structure
+     and attach it as a partition later.  This allows new data to be loaded,
+     checked, and transformed prior to it appearing in the partitioned table.
      Moreover, the <literal>ATTACH PARTITION</literal> operation requires
-     only <literal>SHARE UPDATE EXCLUSIVE</literal> lock on the
-     partitioned table, as opposed to the <literal>ACCESS
-     EXCLUSIVE</literal> lock that is required by <command>CREATE TABLE
-     ... PARTITION OF</command>, so it is more friendly to concurrent
-     operations on the partitioned table.
-     The <literal>CREATE TABLE ... LIKE</literal> option is helpful
-     to avoid tediously repeating the parent table's definition:
+     only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock on the
+     partitioned table rather than the <literal>ACCESS EXCLUSIVE</literal>
+     lock required by <command>CREATE TABLE ... PARTITION OF</command>,
+     so it is more friendly to concurrent operations on the partitioned table;
+     see <link linkend="sql-altertable-attach-partition"><literal>ALTER TABLE ... ATTACH PARTITION</literal></link>
+     for additional details.  Furthermore, the
+     <link linkend="sql-createtable-parms-like"><literal>CREATE TABLE ... LIKE</literal></link>
+     option can be helpful to avoid tediously repeating the parent table's
+     definition; for example:
 
 <programlisting>
 CREATE TABLE measurement_y2008m02
@@ -4313,17 +4315,15 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
     </para>
 
     <para>
-     Before running the <command>ATTACH PARTITION</command> command, it is
-     recommended to create a <literal>CHECK</literal> constraint on the table to
-     be attached that matches the expected partition constraint, as
-     illustrated above. That way, the system will be able to skip the scan
-     which is otherwise needed to validate the implicit
-     partition constraint. Without the <literal>CHECK</literal> constraint,
+     Note that when running the <command>ATTACH PARTITION</command> command,
      the table will be scanned to validate the partition constraint while
      holding an <literal>ACCESS EXCLUSIVE</literal> lock on that partition.
-     It is recommended to drop the now-redundant <literal>CHECK</literal>
-     constraint after the <command>ATTACH PARTITION</command> is complete.  If
-     the table being attached is itself a partitioned table, then each of its
+     As shown above, it is recommended to avoid this scan by creating a
+     <literal>CHECK</literal> constraint matching the expected partition
+     constraint on the table prior to attaching it.  Once the
+     <command>ATTACH PARTITION</command> is complete, it is recommended to drop
+     the now-redundant <literal>CHECK</literal> constraint.
+     If the table being attached is itself a partitioned table, then each of its
      sub-partitions will be recursively locked and scanned until either a
      suitable <literal>CHECK</literal> constraint is encountered or the leaf
      partitions are reached.
@@ -4333,7 +4333,7 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
      Similarly, if the partitioned table has a <literal>DEFAULT</literal>
      partition, it is recommended to create a <literal>CHECK</literal>
      constraint which excludes the to-be-attached partition's constraint.  If
-     this is not done then the <literal>DEFAULT</literal> partition will be
+     this is not done, the <literal>DEFAULT</literal> partition will be
      scanned to verify that it contains no records which should be located in
      the partition being attached.  This operation will be performed whilst
      holding an <literal>ACCESS EXCLUSIVE</literal> lock on the <literal>
@@ -4344,21 +4344,21 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
     </para>
 
     <para>
-     As explained above, it is possible to create indexes on partitioned tables
-     so that they are applied automatically to the entire hierarchy.
-     This is very
-     convenient, as not only will the existing partitions become indexed, but
-     also any partitions that are created in the future will.  One limitation is
-     that it's not possible to use the <literal>CONCURRENTLY</literal>
-     qualifier when creating such a partitioned index.  To avoid long lock
-     times, it is possible to use <command>CREATE INDEX ON ONLY</command>
-     the partitioned table; such an index is marked invalid, and the partitions
-     do not get the index applied automatically.  The indexes on partitions can
-     be created individually using <literal>CONCURRENTLY</literal>, and then
-     <firstterm>attached</firstterm> to the index on the parent using
-     <command>ALTER INDEX .. ATTACH PARTITION</command>.  Once indexes for all
-     partitions are attached to the parent index, the parent index is marked
-     valid automatically.  Example:
+     As mentioned earlier, it is possible to create indexes on partitioned
+     tables so that they are applied automatically to the entire hierarchy.
+     This can be very convenient as not only will all existing partitions be
+     indexed, but any future partitions will be as well.  However, one
+     limitation when creating new indexes on partitioned tables is that it
+     is not possible to use the <literal>CONCURRENTLY</literal>
+     qualifier, which could lead to long lock times.  To avoid this, you can
+     use <command>CREATE INDEX ON ONLY</command> the partitioned table, which
+     creates the new index marked as invalid, preventing automatic application
+     to existing partitions.  Instead, indexes can then be created individually
+     on each partition using <literal>CONCURRENTLY</literal> and
+     <firstterm>attached</firstterm> to the partitioned index on the parent
+     using <command>ALTER INDEX ... ATTACH PARTITION</command>.  Once indexes for
+     all the partitions are attached to the parent index, the parent index will
+     be marked valid automatically.  Example:
 <programlisting>
 CREATE INDEX measurement_usls_idx ON ONLY measurement (unitsales);
 
-- 
2.39.2


--a37p5fzrxyoeka52--





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

* VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
@ 2026-03-24 15:35 Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  0 siblings, 1 reply; 28+ messages in thread

From: Jim Jones @ 2026-03-24 15:35 UTC (permalink / raw)
  To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Antonin Houska <ah@cybertec.at>

Hi,

While testing another patch [1], I noticed that REPACK is blocked when a
temporary table is locked in another session. It also turns out that the
same behaviour occurs with VACUUM FULL and CLUSTER:

== session 1 ==

$ psql postgres
psql (19devel)
Type "help" for help.

postgres=# CREATE TEMPORARY TABLE tmp (id int);
CREATE TABLE
postgres=# BEGIN;
LOCK TABLE tmp IN SHARE MODE;
BEGIN
LOCK TABLE
postgres=*#

== session 2 ==

$ psql postgres
psql (19devel)
Type "help" for help.

postgres=# REPACK;
^CCancel request sent
ERROR:  canceling statement due to user request
CONTEXT:  waiting for AccessExclusiveLock on relation 38458 of database 5
postgres=# VACUUM FULL;
^CCancel request sent
ERROR:  canceling statement due to user request
CONTEXT:  waiting for AccessExclusiveLock on relation 38458 of database 5

Skipping temporary relations in get_tables_to_repack() and
get_all_vacuum_rels() before they're appended to the list seems to do
the trick -- see attached draft.

I can reproduce the same behaviour with CLUSTER and VACUUM FULL in
PG14-PG18. I took a quick look at the code in PG17 and PG18 and the fix
appears to be straightforward, but before I start working on it, I'd
like to hear your thoughts. Is it worth the effort?

Best, Jim

1 - https://www.postgresql.org/message-id/13637.1774342137%40localhost

Attachments:

  [text/x-patch] v1-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch (2.8K, ../../0b555318-2bf2-46df-9377-09629a2a59db@uni-muenster.de/2-v1-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch)
  download | inline diff:
From d57d212305abd2cfb15c6c61f98e503e1a736ab2 Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Tue, 24 Mar 2026 13:50:26 +0100
Subject: [PATCH v1] Skip other sessions' temp tables in REPACK, CLUSTER, and
 VACUUM FULL

get_tables_to_repack() was including other sessions' temporary tables
in the work list, causing REPACK and CLUSTER (without arguments) to
attempt to acquire AccessExclusiveLock on them, potentially blocking
for an extended time. Fix by skipping other-session temp tables early
in get_tables_to_repack(), before they are added to the list. Because
an AccessShareLock has already been acquired per relation at that
point, release it before continuing.

Similarly, get_all_vacuum_rels() suffered from the same problem for
VACUUM FULL. Since no per-relation lock is held during list-building
there, a plain skip suffices.
---
 src/backend/commands/cluster.c | 15 +++++++++++++++
 src/backend/commands/vacuum.c  |  5 +++++
 2 files changed, 20 insertions(+)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 09066db095..f701d426dd 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1699,6 +1699,13 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			if (isOtherTempNamespace(get_rel_namespace(index->indrelid)))
+			{
+				UnlockRelationOid(index->indrelid, AccessShareLock);
+				continue;
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
 												  GetUserId()))
@@ -1753,6 +1760,14 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			if (class->relpersistence == RELPERSISTENCE_TEMP &&
+				isOtherTempNamespace(class->relnamespace))
+			{
+				UnlockRelationOid(class->oid, AccessShareLock);
+				continue;
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index bce3a2daa2..642f4fee1c 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1062,6 +1062,11 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
 
+		/* Skip temp relations belonging to other sessions */
+		if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+			isOtherTempNamespace(classForm->relnamespace))
+			continue;
+
 		/* check permissions of relation */
 		if (!vacuum_is_permitted_for_relation(relid, classForm, options))
 			continue;
-- 
2.43.0



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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-03-25 01:55 ` Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  0 siblings, 1 reply; 28+ messages in thread

From: Chao Li @ 2026-03-25 01:55 UTC (permalink / raw)
  To: Jim Jones <jim.jones@uni-muenster.de>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>



> On Mar 24, 2026, at 23:35, Jim Jones <jim.jones@uni-muenster.de> wrote:
> 
> Hi,
> 
> While testing another patch [1], I noticed that REPACK is blocked when a
> temporary table is locked in another session. It also turns out that the
> same behaviour occurs with VACUUM FULL and CLUSTER:
> 
> == session 1 ==
> 
> $ psql postgres
> psql (19devel)
> Type "help" for help.
> 
> postgres=# CREATE TEMPORARY TABLE tmp (id int);
> CREATE TABLE
> postgres=# BEGIN;
> LOCK TABLE tmp IN SHARE MODE;
> BEGIN
> LOCK TABLE
> postgres=*#
> 
> == session 2 ==
> 
> $ psql postgres
> psql (19devel)
> Type "help" for help.
> 
> postgres=# REPACK;
> ^CCancel request sent
> ERROR:  canceling statement due to user request
> CONTEXT:  waiting for AccessExclusiveLock on relation 38458 of database 5
> postgres=# VACUUM FULL;
> ^CCancel request sent
> ERROR:  canceling statement due to user request
> CONTEXT:  waiting for AccessExclusiveLock on relation 38458 of database 5
> 
> Skipping temporary relations in get_tables_to_repack() and
> get_all_vacuum_rels() before they're appended to the list seems to do
> the trick -- see attached draft.
> 
> I can reproduce the same behaviour with CLUSTER and VACUUM FULL in
> PG14-PG18. I took a quick look at the code in PG17 and PG18 and the fix
> appears to be straightforward, but before I start working on it, I'd
> like to hear your thoughts. Is it worth the effort?
> 
> Best, Jim
> 
> 1 - https://www.postgresql.org/message-id/13637.1774342137%40localhost<v1-0001-Skip-other-sessions-te...;

I think skipping temp tables of another session is reasonable, because anyway they are not accessible from the current session, though visible via pg_class.

Looking at the patch:
```
+			/* Skip temp relations belonging to other sessions */
+			if (class->relpersistence == RELPERSISTENCE_TEMP &&
+				isOtherTempNamespace(class->relnamespace))
```

It uses isOtherTempNamespace(), but I noticed that the header comment of the function says:
```
 * isOtherTempNamespace - is the given namespace some other backend's
* temporary-table namespace (including temporary-toast-table namespaces)?
*
* Note: for most purposes in the C code, this function is obsolete. Use
* RELATION_IS_OTHER_TEMP() instead to detect non-local temp relations.
```

Then looking at RELATION_IS_OTHER_TEMP():
```
#define RELATION_IS_OTHER_TEMP(relation) \
	((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \
	 !(relation)->rd_islocaltemp)
```

It takes a relation as parameter and check relation->rd_islocaltemp, however in the context of this patch, we have only Form_pg_class.

Then checking how rd_islocaltemp is set:
```
		case RELPERSISTENCE_TEMP:
			if (isTempOrTempToastNamespace(relation->rd_rel->relnamespace))
			{
				relation->rd_backend = ProcNumberForTempRelations();
				relation->rd_islocaltemp = true;
			}
			else
			{
				/*
				 * If it's a temp table, but not one of ours, we have to use
				 * the slow, grotty method to figure out the owning backend.
				 *
				 * Note: it's possible that rd_backend gets set to
				 * MyProcNumber here, in case we are looking at a pg_class
				 * entry left over from a crashed backend that coincidentally
				 * had the same ProcNumber we're using.  We should *not*
				 * consider such a table to be "ours"; this is why we need the
				 * separate rd_islocaltemp flag.  The pg_class entry will get
				 * flushed if/when we clean out the corresponding temp table
				 * namespace in preparation for using it.
				 */
				relation->rd_backend =
					GetTempNamespaceProcNumber(relation->rd_rel->relnamespace);
				Assert(relation->rd_backend != INVALID_PROC_NUMBER);
				relation->rd_islocaltemp = false;
			}
			break;
```

It uses isTempOrTempToastNamespace(relation->rd_rel->relnamespace) to decide relation->rd_islocaltemp.

So, I think this patch should also use "!isTempOrTempToastNamespace(classForm->relnamespace)" instead of isOtherTempNamespace(class->relnamespace). I tried that locally, and it works for me.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
@ 2026-03-25 09:05   ` Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 10:32     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  0 siblings, 2 replies; 28+ messages in thread

From: Jim Jones @ 2026-03-25 09:05 UTC (permalink / raw)
  To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>

Hi Chao,

Thanks for the thorough review.

On 25/03/2026 02:55, Chao Li wrote:
> It uses isTempOrTempToastNamespace(relation->rd_rel->relnamespace) to decide relation->rd_islocaltemp.
> 
> So, I think this patch should also use "!isTempOrTempToastNamespace(classForm->relnamespace)" instead of isOtherTempNamespace(class->relnamespace). I tried that locally, and it works for me.

I agree. In get_all_vacuum_rels() and the pg_class scan branch of
get_tables_to_repack(), relpersistence == RELPERSISTENCE_TEMP already
establishes the relation is a temp table, so
!isTempOrTempToastNamespace() alone is sufficient to identify
other-session temp tables.

In the usingindex path of get_tables_to_repack(), Form_pg_class is not
available, so there is no relpersistence to use as a pre-filter. The
explicit isAnyTempNamespace() check is required to avoid incorrectly
skipping permanent tables. This is pretty much what
isOtherTempNamespace() does internally -- the only change is inlining it
to avoid the obsolete wrapper.

* Skip temp relations belonging to other sessions */
{
  Oid nsp = get_rel_namespace(index->indrelid);

  if (!isTempOrTempToastNamespace(nsp) && isAnyTempNamespace(nsp))
  {
    UnlockRelationOid(index->indrelid, AccessShareLock);
    continue;
  }
}


v2 attached.

Thanks!

Best, Jim

Attachments:

  [text/x-patch] v2-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch (3.0K, ../../039cca2d-5574-4dad-92e6-6e72c1747b23@uni-muenster.de/2-v2-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch)
  download | inline diff:
From 5f8528db5d457ddc3158b00b7768c8e088e1e577 Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Wed, 25 Mar 2026 09:34:06 +0100
Subject: [PATCH v2] Skip other sessions' temp tables in REPACK, CLUSTER, and
 VACUUM FULL

get_tables_to_repack() was including other sessions' temporary tables
in the work list, causing REPACK and CLUSTER (without arguments) to
attempt to acquire AccessExclusiveLock on them, potentially blocking
for an extended time. Fix by skipping other-session temp tables early
in get_tables_to_repack(), before they are added to the list. Because
an AccessShareLock has already been acquired per relation at that
point, release it before continuing.

Similarly, get_all_vacuum_rels() suffered from the same problem for
VACUUM FULL. Since no per-relation lock is held during list-building
there, a plain skip suffices.

Author: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
---
 src/backend/commands/cluster.c | 19 +++++++++++++++++++
 src/backend/commands/vacuum.c  |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 09066db0956..386bd34c16b 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1699,6 +1699,17 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			{
+				Oid			nsp = get_rel_namespace(index->indrelid);
+
+				if (!isTempOrTempToastNamespace(nsp) && isAnyTempNamespace(nsp))
+				{
+					UnlockRelationOid(index->indrelid, AccessShareLock);
+					continue;
+				}
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
 												  GetUserId()))
@@ -1753,6 +1764,14 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			if (class->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(class->relnamespace))
+			{
+				UnlockRelationOid(class->oid, AccessShareLock);
+				continue;
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index bce3a2daa24..9b0a5a38a8a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1062,6 +1062,11 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
 
+		/* Skip temp relations belonging to other sessions */
+		if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+			!isTempOrTempToastNamespace(classForm->relnamespace))
+			continue;
+
 		/* check permissions of relation */
 		if (!vacuum_is_permitted_for_relation(relid, classForm, options))
 			continue;
-- 
2.43.0



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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-03-25 10:32     ` Jim Jones <jim.jones@uni-muenster.de>
  1 sibling, 0 replies; 28+ messages in thread

From: Jim Jones @ 2026-03-25 10:32 UTC (permalink / raw)
  To: Chao Li <li.evan.chao@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>

I forgot to create a CF entry. Here it is:
https://commitfest.postgresql.org/patch/6616/

Best, Jim





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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-03-25 20:38     ` Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  1 sibling, 1 reply; 28+ messages in thread

From: Zsolt Parragi @ 2026-03-25 20:38 UTC (permalink / raw)
  To: Jim Jones <jim.jones@uni-muenster.de>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>

Hello!

Shouldn't the patch also include a tap test to verify that the change
works / fails without it?

+ /* Skip temp relations belonging to other sessions */
+ {
+ Oid nsp = get_rel_namespace(index->indrelid);
+
+ if (!isTempOrTempToastNamespace(nsp) && isAnyTempNamespace(nsp))
+ {

Doesn't this result in several repeated syscache lookups?

There's already a SearchSysCacheExsists1 directly above this, then a
get_rel_namespace, then an isAnyTempNamespace. While this probably
isn't performance critical, this should be doable with a single
SearchSysCache1(RELOID...) and then a few conditions, similarly to the
else branch below this?





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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
@ 2026-03-25 23:00       ` Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  0 siblings, 1 reply; 28+ messages in thread

From: Jim Jones @ 2026-03-25 23:00 UTC (permalink / raw)
  To: Zsolt Parragi <zsolt.parragi@percona.com>; +Cc: Chao Li <li.evan.chao@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>

Hi

On 25/03/2026 21:38, Zsolt Parragi wrote:
> Shouldn't the patch also include a tap test to verify that the change
> works / fails without it?

Definitely. I just didn't want to invest much time on tests before
getting feedback on the issue itself.

> + /* Skip temp relations belonging to other sessions */
> + {
> + Oid nsp = get_rel_namespace(index->indrelid);
> +
> + if (!isTempOrTempToastNamespace(nsp) && isAnyTempNamespace(nsp))
> + {
> 
> Doesn't this result in several repeated syscache lookups?
> 
> There's already a SearchSysCacheExsists1 directly above this, then a
> get_rel_namespace, then an isAnyTempNamespace. While this probably
> isn't performance critical, this should be doable with a single
> SearchSysCache1(RELOID...) and then a few conditions, similarly to the
> else branch below this?

You're right. Although it is not performance critical we can solve it
with a single SearchSysCache1.

PFA v3 with the improved fix (0001) and tests (0002).

Thanks for the review!

Best, Jim

Attachments:

  [text/x-patch] v3-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch (3.7K, ../../9de2a59f-9597-4101-bf34-cf3804752e64@uni-muenster.de/2-v3-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch)
  download | inline diff:
From 6699534c03772b0e5b06680b2e382a36eb108a67 Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Wed, 25 Mar 2026 23:14:12 +0100
Subject: [PATCH v3 1/2] Skip other sessions' temp tables in REPACK, CLUSTER,
 and VACUUM FULL

get_tables_to_repack() was including other sessions' temporary tables
in the work list, causing REPACK and CLUSTER (without arguments) to
attempt to acquire AccessExclusiveLock on them, potentially blocking
for an extended time. Fix by skipping other-session temp tables early
in get_tables_to_repack(), before they are added to the list. Because
an AccessShareLock has already been acquired per relation at that
point, release it before continuing.

Similarly, get_all_vacuum_rels() suffered from the same problem for
VACUUM FULL. Since no per-relation lock is held during list-building
there, a plain skip suffices.

Author: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
---
 src/backend/commands/cluster.c | 25 ++++++++++++++++++++++++-
 src/backend/commands/vacuum.c  |  5 +++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 09066db0956..14c11e8e532 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1679,6 +1679,8 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 		{
 			RelToCluster *rtc;
 			Form_pg_index index;
+			HeapTuple	classtup;
+			Form_pg_class classForm;
 			MemoryContext oldcxt;
 
 			index = (Form_pg_index) GETSTRUCT(tuple);
@@ -1693,11 +1695,24 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 
 			/* Verify that the table still exists; skip if not */
-			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(index->indrelid)))
+			classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
+			if (!HeapTupleIsValid(classtup))
 			{
 				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
 			}
+			classForm = (Form_pg_class) GETSTRUCT(classtup);
+
+			/* Skip temp relations belonging to other sessions */
+			if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(classForm->relnamespace))
+			{
+				ReleaseSysCache(classtup);
+				UnlockRelationOid(index->indrelid, AccessShareLock);
+				continue;
+			}
+
+			ReleaseSysCache(classtup);
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
@@ -1753,6 +1768,14 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			if (class->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(class->relnamespace))
+			{
+				UnlockRelationOid(class->oid, AccessShareLock);
+				continue;
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index bce3a2daa24..9b0a5a38a8a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1062,6 +1062,11 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
 
+		/* Skip temp relations belonging to other sessions */
+		if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+			!isTempOrTempToastNamespace(classForm->relnamespace))
+			continue;
+
 		/* check permissions of relation */
 		if (!vacuum_is_permitted_for_relation(relid, classForm, options))
 			continue;
-- 
2.43.0



  [text/x-patch] v3-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch (3.7K, ../../9de2a59f-9597-4101-bf34-cf3804752e64@uni-muenster.de/3-v3-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch)
  download | inline diff:
From 97d1b2023b8c4ac5d92610e9761d112ab68cbe23 Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Wed, 25 Mar 2026 23:30:55 +0100
Subject: [PATCH v3 2/2] Test VACUUM FULL, CLUSTER, and REPACK with locked temp
 tables

This test creates a background session with a temp table and marks
its index as clustered (making it visible to both the pg_class scan
used by VACUUM FULL and REPACK, and the pg_index scan used by CLUSTER),
then holds ACCESS SHARE LOCK in an open transaction.  Each command
runs with lock_timeout = '1ms'.  Since lock_timeout only fires when a
backend actually blocks waiting for a lock, 1ms is sufficient.
---
 src/test/modules/test_misc/meson.build        |  1 +
 .../t/011_vacuum_cluster_temp_tables.pl       | 65 +++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl

diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 6e8db1621a7..d64e8df56bf 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -19,6 +19,7 @@ tests += {
       't/008_replslot_single_user.pl',
       't/009_log_temp_files.pl',
       't/010_index_concurrently_upsert.pl',
+      't/011_vacuum_cluster_temp_tables.pl',
     ],
     # The injection points are cluster-wide, so disable installcheck
     'runningcheck': false,
diff --git a/src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl b/src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl
new file mode 100644
index 00000000000..a612b4d6361
--- /dev/null
+++ b/src/test/modules/test_misc/t/011_vacuum_cluster_temp_tables.pl
@@ -0,0 +1,65 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Verify that no-argument VACUUM FULL, CLUSTER, and REPACK skip temporary
+# tables belonging to other sessions.
+#
+# A background session creates a temp table and marks its index as clustered —
+# making it visible to both the pg_class scan (VACUUM FULL, REPACK) and the
+# pg_index scan (CLUSTER) — then holds ACCESS SHARE LOCK in an open transaction.
+# Each command runs with lock_timeout = '1ms'. Since lock_timeout only
+# fires when a backend actually blocks waiting for a lock, 1ms is sufficient.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('vacuum_cluster_temp');
+$node->init;
+$node->start;
+
+# Session 1: build the temp table and hold a conflicting lock.
+my $psql1 = $node->background_psql('postgres');
+
+$psql1->query_safe(
+	q{CREATE TEMP TABLE temp_repack_test (val int);
+	  INSERT INTO temp_repack_test VALUES (1);
+	  CREATE INDEX temp_repack_idx ON temp_repack_test (val);
+	  CLUSTER temp_repack_test USING temp_repack_idx;});
+
+$psql1->query_safe(q{BEGIN});
+$psql1->query_safe(q{LOCK TABLE temp_repack_test IN ACCESS SHARE MODE});
+
+my ($stdout, $stderr, $ret);
+
+# VACUUM FULL — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; VACUUM FULL;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'VACUUM FULL completes without blocking on another session temp table');
+
+# CLUSTER — pg_index scan path (indisclustered entries).
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; CLUSTER;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'CLUSTER completes without blocking on another session temp table');
+
+# REPACK — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; REPACK;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'REPACK completes without blocking on another session temp table');
+
+$psql1->quit;
+
+done_testing();
-- 
2.43.0



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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-03-26 02:52         ` Chao Li <li.evan.chao@gmail.com>
  2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
  0 siblings, 1 reply; 28+ messages in thread

From: Chao Li @ 2026-03-26 02:52 UTC (permalink / raw)
  To: Jim Jones <jim.jones@uni-muenster.de>; +Cc: Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Antonin Houska <ah@cybertec.at>



> On Mar 26, 2026, at 07:00, Jim Jones <jim.jones@uni-muenster.de> wrote:
> 
> Hi
> 
> On 25/03/2026 21:38, Zsolt Parragi wrote:
>> Shouldn't the patch also include a tap test to verify that the change
>> works / fails without it?
> 
> Definitely. I just didn't want to invest much time on tests before
> getting feedback on the issue itself.
> 
>> + /* Skip temp relations belonging to other sessions */
>> + {
>> + Oid nsp = get_rel_namespace(index->indrelid);
>> +
>> + if (!isTempOrTempToastNamespace(nsp) && isAnyTempNamespace(nsp))
>> + {
>> 
>> Doesn't this result in several repeated syscache lookups?
>> 
>> There's already a SearchSysCacheExsists1 directly above this, then a
>> get_rel_namespace, then an isAnyTempNamespace. While this probably
>> isn't performance critical, this should be doable with a single
>> SearchSysCache1(RELOID...) and then a few conditions, similarly to the
>> else branch below this?
> 
> You're right. Although it is not performance critical we can solve it
> with a single SearchSysCache1.
> 
> PFA v3 with the improved fix (0001) and tests (0002).
> 
> Thanks for the review!
> 
> Best, Jim<v3-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch><v3-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch>

I don't think such a TAP test is necessary.

One reason is that, if we look at the check right above the new one:
```
		/*
		 * We include partitioned tables here; depending on which operation is
		 * to be performed, caller will decide whether to process or ignore
		 * them.
		 */
		if (classForm->relkind != RELKIND_RELATION &&
			classForm->relkind != RELKIND_MATVIEW &&
			classForm->relkind != RELKIND_PARTITIONED_TABLE)
			continue;
```

I don't see a test specifically for that check either. So I don't think we need a test for every individual path.

Second, based on [1] and [2], I got the impression that adding new tests is not always welcome considering overall test runtime. Anyway, maybe I’m wrong, let the committers judge that.

[1] https://postgr.es/m/mtkrkkcn2tlhytumitpch5ubxiprv2jzvprf5r5m3mjeczvq4q@p6wkzbfxuyv2 <https://postgr.es/m/;
[2] https://postgr.es/m/1449781.1773948276@sss.pgh.pa.us

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
@ 2026-03-26 11:25           ` Antonin Houska <ah@cybertec.at>
  2026-03-26 11:53             ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  0 siblings, 1 reply; 28+ messages in thread

From: Antonin Houska @ 2026-03-26 11:25 UTC (permalink / raw)
  To: Chao Li <li.evan.chao@gmail.com>; +Cc: Jim Jones <jim.jones@uni-muenster.de>; Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

Chao Li <li.evan.chao@gmail.com> wrote:

> I don't think such a TAP test is necessary.

+1

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com





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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
@ 2026-03-26 11:53             ` Jim Jones <jim.jones@uni-muenster.de>
  2026-04-06 18:13               ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-05-05 14:32               ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Álvaro Herrera <alvherre@kurilemu.de>
  0 siblings, 2 replies; 28+ messages in thread

From: Jim Jones @ 2026-03-26 11:53 UTC (permalink / raw)
  To: Antonin Houska <ah@cybertec.at>; Chao Li <li.evan.chao@gmail.com>; +Cc: Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>


On 26/03/2026 12:25, Antonin Houska wrote:
> Chao Li <li.evan.chao@gmail.com> wrote:
> 
>> I don't think such a TAP test is necessary.
> +1

I've kept the tests in a separate file so the committer can easily skip
them if needed.

Thanks for the feedback, everyone!

Best, Jim






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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
  2026-03-26 11:53             ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-04-06 18:13               ` Jim Jones <jim.jones@uni-muenster.de>
  2026-04-06 22:52                 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  1 sibling, 1 reply; 28+ messages in thread

From: Jim Jones @ 2026-04-06 18:13 UTC (permalink / raw)
  To: Antonin Houska <ah@cybertec.at>; Chao Li <li.evan.chao@gmail.com>; +Cc: Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

rebase

Jim

Attachments:

  [text/x-patch] v4-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch (3.7K, ../../fb2ef94b-cc95-4b6e-89dc-a2322c76b009@uni-muenster.de/2-v4-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch)
  download | inline diff:
From 7a50ea8527634a39ccb9abbf8f573883faccd353 Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Wed, 25 Mar 2026 23:14:12 +0100
Subject: [PATCH v4 1/2] Skip other sessions' temp tables in REPACK, CLUSTER,
 and VACUUM FULL

get_tables_to_repack() was including other sessions' temporary tables
in the work list, causing REPACK and CLUSTER (without arguments) to
attempt to acquire AccessExclusiveLock on them, potentially blocking
for an extended time. Fix by skipping other-session temp tables early
in get_tables_to_repack(), before they are added to the list. Because
an AccessShareLock has already been acquired per relation at that
point, release it before continuing.

Similarly, get_all_vacuum_rels() suffered from the same problem for
VACUUM FULL. Since no per-relation lock is held during list-building
there, a plain skip suffices.

Author: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
---
 src/backend/commands/cluster.c | 25 ++++++++++++++++++++++++-
 src/backend/commands/vacuum.c  |  5 +++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index f241e18b153..ffbc5ed6f85 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1679,6 +1679,8 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 		{
 			RelToCluster *rtc;
 			Form_pg_index index;
+			HeapTuple	classtup;
+			Form_pg_class classForm;
 			MemoryContext oldcxt;
 
 			index = (Form_pg_index) GETSTRUCT(tuple);
@@ -1693,11 +1695,24 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 
 			/* Verify that the table still exists; skip if not */
-			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(index->indrelid)))
+			classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
+			if (!HeapTupleIsValid(classtup))
 			{
 				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
 			}
+			classForm = (Form_pg_class) GETSTRUCT(classtup);
+
+			/* Skip temp relations belonging to other sessions */
+			if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(classForm->relnamespace))
+			{
+				ReleaseSysCache(classtup);
+				UnlockRelationOid(index->indrelid, AccessShareLock);
+				continue;
+			}
+
+			ReleaseSysCache(classtup);
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
@@ -1753,6 +1768,14 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			if (class->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(class->relnamespace))
+			{
+				UnlockRelationOid(class->oid, AccessShareLock);
+				continue;
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 0ed363d1c85..913dfb4ea8d 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1062,6 +1062,11 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
 
+		/* Skip temp relations belonging to other sessions */
+		if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+			!isTempOrTempToastNamespace(classForm->relnamespace))
+			continue;
+
 		/* check permissions of relation */
 		if (!vacuum_is_permitted_for_relation(relid, classForm, options))
 			continue;
-- 
2.43.0



  [text/x-patch] v4-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch (3.7K, ../../fb2ef94b-cc95-4b6e-89dc-a2322c76b009@uni-muenster.de/3-v4-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch)
  download | inline diff:
From 3e689b85190fc074a4fe7c9e2e1b3f618faf08bf Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Mon, 6 Apr 2026 19:40:56 +0200
Subject: [PATCH v4 2/2] Test VACUUM FULL, CLUSTER, and REPACK with locked temp
 tables

This test creates a background session with a temp table and marks
its index as clustered (making it visible to both the pg_class scan
used by VACUUM FULL and REPACK, and the pg_index scan used by CLUSTER),
then holds ACCESS SHARE LOCK in an open transaction.  Each command
runs with lock_timeout = '1ms'.  Since lock_timeout only fires when a
backend actually blocks waiting for a lock, 1ms is sufficient.
---
 src/test/modules/test_misc/meson.build        |  1 +
 .../t/012_vacuum_cluster_temp_tables.pl       | 65 +++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl

diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 1b25d98f7f3..8995bc837f2 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -20,6 +20,7 @@ tests += {
       't/009_log_temp_files.pl',
       't/010_index_concurrently_upsert.pl',
       't/011_lock_stats.pl',
+      't/012_vacuum_cluster_temp_tables.pl',
     ],
     # The injection points are cluster-wide, so disable installcheck
     'runningcheck': false,
diff --git a/src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl b/src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl
new file mode 100644
index 00000000000..a612b4d6361
--- /dev/null
+++ b/src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl
@@ -0,0 +1,65 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Verify that no-argument VACUUM FULL, CLUSTER, and REPACK skip temporary
+# tables belonging to other sessions.
+#
+# A background session creates a temp table and marks its index as clustered —
+# making it visible to both the pg_class scan (VACUUM FULL, REPACK) and the
+# pg_index scan (CLUSTER) — then holds ACCESS SHARE LOCK in an open transaction.
+# Each command runs with lock_timeout = '1ms'. Since lock_timeout only
+# fires when a backend actually blocks waiting for a lock, 1ms is sufficient.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('vacuum_cluster_temp');
+$node->init;
+$node->start;
+
+# Session 1: build the temp table and hold a conflicting lock.
+my $psql1 = $node->background_psql('postgres');
+
+$psql1->query_safe(
+	q{CREATE TEMP TABLE temp_repack_test (val int);
+	  INSERT INTO temp_repack_test VALUES (1);
+	  CREATE INDEX temp_repack_idx ON temp_repack_test (val);
+	  CLUSTER temp_repack_test USING temp_repack_idx;});
+
+$psql1->query_safe(q{BEGIN});
+$psql1->query_safe(q{LOCK TABLE temp_repack_test IN ACCESS SHARE MODE});
+
+my ($stdout, $stderr, $ret);
+
+# VACUUM FULL — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; VACUUM FULL;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'VACUUM FULL completes without blocking on another session temp table');
+
+# CLUSTER — pg_index scan path (indisclustered entries).
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; CLUSTER;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'CLUSTER completes without blocking on another session temp table');
+
+# REPACK — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; REPACK;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'REPACK completes without blocking on another session temp table');
+
+$psql1->quit;
+
+done_testing();
-- 
2.43.0



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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
  2026-03-26 11:53             ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-04-06 18:13               ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-04-06 22:52                 ` Jim Jones <jim.jones@uni-muenster.de>
  2026-05-01 19:34                   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  0 siblings, 1 reply; 28+ messages in thread

From: Jim Jones @ 2026-04-06 22:52 UTC (permalink / raw)
  To: Antonin Houska <ah@cybertec.at>; Chao Li <li.evan.chao@gmail.com>; +Cc: Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

rebase

Jim

Attachments:

  [text/x-patch] v5-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch (3.7K, ../../217fa6cc-95d9-4991-897d-f70ab1a75c9c@uni-muenster.de/2-v5-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch)
  download | inline diff:
From f7876b7c225146fe3deef681841d0ad6d9782a5d Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Tue, 7 Apr 2026 00:27:59 +0200
Subject: [PATCH v5 1/2] Skip other sessions' temp tables in REPACK, CLUSTER,
 and VACUUM FULL

get_tables_to_repack() was including other sessions' temporary tables
in the work list, causing REPACK and CLUSTER (without arguments) to
attempt to acquire AccessExclusiveLock on them, potentially blocking
for an extended time. Fix by skipping other-session temp tables early
in get_tables_to_repack(), before they are added to the list. Because
an AccessShareLock has already been acquired per relation at that
point, release it before continuing.

Similarly, get_all_vacuum_rels() suffered from the same problem for
VACUUM FULL. Since no per-relation lock is held during list-building
there, a plain skip suffices.

Author: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
---
 src/backend/commands/repack.c | 25 ++++++++++++++++++++++++-
 src/backend/commands/vacuum.c |  5 +++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 17b639b3b44..1806efddb8f 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -2107,6 +2107,8 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 		{
 			RelToCluster *rtc;
 			Form_pg_index index;
+			HeapTuple classtup;
+			Form_pg_class classForm;
 			MemoryContext oldcxt;
 
 			index = (Form_pg_index) GETSTRUCT(tuple);
@@ -2121,11 +2123,24 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 
 			/* Verify that the table still exists; skip if not */
-			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(index->indrelid)))
+			classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
+			if (!HeapTupleIsValid(classtup))
 			{
 				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
 			}
+			classForm = (Form_pg_class) GETSTRUCT(classtup);
+
+			/* Skip temp relations belonging to other sessions */
+			if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(classForm->relnamespace))
+			{
+				ReleaseSysCache(classtup);
+				UnlockRelationOid(index->indrelid, AccessShareLock);
+				continue;
+			}
+
+			ReleaseSysCache(classtup);
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
@@ -2181,6 +2196,14 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			if (class->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(class->relnamespace))
+			{
+				UnlockRelationOid(class->oid, AccessShareLock);
+				continue;
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 99d0db82ed7..a4abb29cf64 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1063,6 +1063,11 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
 
+		/* Skip temp relations belonging to other sessions */
+		if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+			!isTempOrTempToastNamespace(classForm->relnamespace))
+			continue;
+
 		/* check permissions of relation */
 		if (!vacuum_is_permitted_for_relation(relid, classForm, options))
 			continue;
-- 
2.43.0



  [text/x-patch] v5-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch (3.7K, ../../217fa6cc-95d9-4991-897d-f70ab1a75c9c@uni-muenster.de/3-v5-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch)
  download | inline diff:
From 4cc9f3d4b5aa8c3544599c47ab3ec9a62a96381d Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Mon, 6 Apr 2026 19:40:56 +0200
Subject: [PATCH v5 2/2] Test VACUUM FULL, CLUSTER, and REPACK with locked temp
 tables

This test creates a background session with a temp table and marks
its index as clustered (making it visible to both the pg_class scan
used by VACUUM FULL and REPACK, and the pg_index scan used by CLUSTER),
then holds ACCESS SHARE LOCK in an open transaction.  Each command
runs with lock_timeout = '1ms'.  Since lock_timeout only fires when a
backend actually blocks waiting for a lock, 1ms is sufficient.
---
 src/test/modules/test_misc/meson.build        |  1 +
 .../t/012_vacuum_cluster_temp_tables.pl       | 65 +++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl

diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 1b25d98f7f3..8995bc837f2 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -20,6 +20,7 @@ tests += {
       't/009_log_temp_files.pl',
       't/010_index_concurrently_upsert.pl',
       't/011_lock_stats.pl',
+      't/012_vacuum_cluster_temp_tables.pl',
     ],
     # The injection points are cluster-wide, so disable installcheck
     'runningcheck': false,
diff --git a/src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl b/src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl
new file mode 100644
index 00000000000..a612b4d6361
--- /dev/null
+++ b/src/test/modules/test_misc/t/012_vacuum_cluster_temp_tables.pl
@@ -0,0 +1,65 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Verify that no-argument VACUUM FULL, CLUSTER, and REPACK skip temporary
+# tables belonging to other sessions.
+#
+# A background session creates a temp table and marks its index as clustered —
+# making it visible to both the pg_class scan (VACUUM FULL, REPACK) and the
+# pg_index scan (CLUSTER) — then holds ACCESS SHARE LOCK in an open transaction.
+# Each command runs with lock_timeout = '1ms'. Since lock_timeout only
+# fires when a backend actually blocks waiting for a lock, 1ms is sufficient.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('vacuum_cluster_temp');
+$node->init;
+$node->start;
+
+# Session 1: build the temp table and hold a conflicting lock.
+my $psql1 = $node->background_psql('postgres');
+
+$psql1->query_safe(
+	q{CREATE TEMP TABLE temp_repack_test (val int);
+	  INSERT INTO temp_repack_test VALUES (1);
+	  CREATE INDEX temp_repack_idx ON temp_repack_test (val);
+	  CLUSTER temp_repack_test USING temp_repack_idx;});
+
+$psql1->query_safe(q{BEGIN});
+$psql1->query_safe(q{LOCK TABLE temp_repack_test IN ACCESS SHARE MODE});
+
+my ($stdout, $stderr, $ret);
+
+# VACUUM FULL — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; VACUUM FULL;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'VACUUM FULL completes without blocking on another session temp table');
+
+# CLUSTER — pg_index scan path (indisclustered entries).
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; CLUSTER;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'CLUSTER completes without blocking on another session temp table');
+
+# REPACK — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; REPACK;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'REPACK completes without blocking on another session temp table');
+
+$psql1->quit;
+
+done_testing();
-- 
2.43.0



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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
  2026-03-26 11:53             ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-04-06 18:13               ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-04-06 22:52                 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-05-01 19:34                   ` Jim Jones <jim.jones@uni-muenster.de>
  0 siblings, 0 replies; 28+ messages in thread

From: Jim Jones @ 2026-05-01 19:34 UTC (permalink / raw)
  To: Antonin Houska <ah@cybertec.at>; Chao Li <li.evan.chao@gmail.com>; +Cc: Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

rebase

Jim

Attachments:

  [text/x-patch] v6-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch (3.7K, ../../36db8f85-ac29-4dd5-ba0e-15a0d2db5c2c@uni-muenster.de/2-v6-0002-Test-VACUUM-FULL-CLUSTER-and-REPACK-with-locked-t.patch)
  download | inline diff:
From def7fbf223f6f2db2e71eeb42fa10536f16b4c0b Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Fri, 1 May 2026 21:31:25 +0200
Subject: [PATCH v6 2/2] Test VACUUM FULL, CLUSTER, and REPACK with locked temp
 tables

This test creates a background session with a temp table and marks
its index as clustered (making it visible to both the pg_class scan
used by VACUUM FULL and REPACK, and the pg_index scan used by CLUSTER),
then holds ACCESS SHARE LOCK in an open transaction.  Each command
runs with lock_timeout = '1ms'.  Since lock_timeout only fires when a
backend actually blocks waiting for a lock, 1ms is sufficient.
---
 src/test/modules/test_misc/meson.build        |  1 +
 .../t/013_vacuum_cluster_temp_tables.pl       | 65 +++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 src/test/modules/test_misc/t/013_vacuum_cluster_temp_tables.pl

diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 356d8454b39..e74d461db3a 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -21,6 +21,7 @@ tests += {
       't/010_index_concurrently_upsert.pl',
       't/011_lock_stats.pl',
       't/012_ddlutils.pl',
+      't/013_vacuum_cluster_temp_tables.pl',
     ],
     # The injection points are cluster-wide, so disable installcheck
     'runningcheck': false,
diff --git a/src/test/modules/test_misc/t/013_vacuum_cluster_temp_tables.pl b/src/test/modules/test_misc/t/013_vacuum_cluster_temp_tables.pl
new file mode 100644
index 00000000000..a612b4d6361
--- /dev/null
+++ b/src/test/modules/test_misc/t/013_vacuum_cluster_temp_tables.pl
@@ -0,0 +1,65 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Verify that no-argument VACUUM FULL, CLUSTER, and REPACK skip temporary
+# tables belonging to other sessions.
+#
+# A background session creates a temp table and marks its index as clustered —
+# making it visible to both the pg_class scan (VACUUM FULL, REPACK) and the
+# pg_index scan (CLUSTER) — then holds ACCESS SHARE LOCK in an open transaction.
+# Each command runs with lock_timeout = '1ms'. Since lock_timeout only
+# fires when a backend actually blocks waiting for a lock, 1ms is sufficient.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('vacuum_cluster_temp');
+$node->init;
+$node->start;
+
+# Session 1: build the temp table and hold a conflicting lock.
+my $psql1 = $node->background_psql('postgres');
+
+$psql1->query_safe(
+	q{CREATE TEMP TABLE temp_repack_test (val int);
+	  INSERT INTO temp_repack_test VALUES (1);
+	  CREATE INDEX temp_repack_idx ON temp_repack_test (val);
+	  CLUSTER temp_repack_test USING temp_repack_idx;});
+
+$psql1->query_safe(q{BEGIN});
+$psql1->query_safe(q{LOCK TABLE temp_repack_test IN ACCESS SHARE MODE});
+
+my ($stdout, $stderr, $ret);
+
+# VACUUM FULL — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; VACUUM FULL;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'VACUUM FULL completes without blocking on another session temp table');
+
+# CLUSTER — pg_index scan path (indisclustered entries).
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; CLUSTER;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'CLUSTER completes without blocking on another session temp table');
+
+# REPACK — pg_class scan path.
+$ret = $node->psql(
+	'postgres',
+	"SET lock_timeout = '1ms'; REPACK;",
+	stdout => \$stdout,
+	stderr => \$stderr);
+is($ret, 0,
+	'REPACK completes without blocking on another session temp table');
+
+$psql1->quit;
+
+done_testing();
-- 
2.43.0



  [text/x-patch] v6-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch (3.7K, ../../36db8f85-ac29-4dd5-ba0e-15a0d2db5c2c@uni-muenster.de/3-v6-0001-Skip-other-sessions-temp-tables-in-REPACK-CLUSTER.patch)
  download | inline diff:
From a0e7497f095377add06a65b97b79fd0162fabeec Mon Sep 17 00:00:00 2001
From: Jim Jones <jim.jones@uni-muenster.de>
Date: Tue, 7 Apr 2026 00:27:59 +0200
Subject: [PATCH v6 1/2] Skip other sessions' temp tables in REPACK, CLUSTER,
 and VACUUM FULL

get_tables_to_repack() was including other sessions' temporary tables
in the work list, causing REPACK and CLUSTER (without arguments) to
attempt to acquire AccessExclusiveLock on them, potentially blocking
for an extended time. Fix by skipping other-session temp tables early
in get_tables_to_repack(), before they are added to the list. Because
an AccessShareLock has already been acquired per relation at that
point, release it before continuing.

Similarly, get_all_vacuum_rels() suffered from the same problem for
VACUUM FULL. Since no per-relation lock is held during list-building
there, a plain skip suffices.

Author: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
---
 src/backend/commands/repack.c | 25 ++++++++++++++++++++++++-
 src/backend/commands/vacuum.c |  5 +++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 9d162957bc3..da4b309c101 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -2106,6 +2106,8 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 		{
 			RelToCluster *rtc;
 			Form_pg_index index;
+			HeapTuple classtup;
+			Form_pg_class classForm;
 			MemoryContext oldcxt;
 
 			index = (Form_pg_index) GETSTRUCT(tuple);
@@ -2120,11 +2122,24 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 
 			/* Verify that the table still exists; skip if not */
-			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(index->indrelid)))
+			classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
+			if (!HeapTupleIsValid(classtup))
 			{
 				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
 			}
+			classForm = (Form_pg_class) GETSTRUCT(classtup);
+
+			/* Skip temp relations belonging to other sessions */
+			if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(classForm->relnamespace))
+			{
+				ReleaseSysCache(classtup);
+				UnlockRelationOid(index->indrelid, AccessShareLock);
+				continue;
+			}
+
+			ReleaseSysCache(classtup);
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
@@ -2180,6 +2195,14 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				continue;
 			}
 
+			/* Skip temp relations belonging to other sessions */
+			if (class->relpersistence == RELPERSISTENCE_TEMP &&
+				!isTempOrTempToastNamespace(class->relnamespace))
+			{
+				UnlockRelationOid(class->oid, AccessShareLock);
+				continue;
+			}
+
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 99d0db82ed7..a4abb29cf64 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1063,6 +1063,11 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
 
+		/* Skip temp relations belonging to other sessions */
+		if (classForm->relpersistence == RELPERSISTENCE_TEMP &&
+			!isTempOrTempToastNamespace(classForm->relnamespace))
+			continue;
+
 		/* check permissions of relation */
 		if (!vacuum_is_permitted_for_relation(relid, classForm, options))
 			continue;
-- 
2.43.0



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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
  2026-03-26 11:53             ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
@ 2026-05-05 14:32               ` Álvaro Herrera <alvherre@kurilemu.de>
  2026-05-06 06:33                 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  1 sibling, 1 reply; 28+ messages in thread

From: Álvaro Herrera @ 2026-05-05 14:32 UTC (permalink / raw)
  To: Jim Jones <jim.jones@uni-muenster.de>; +Cc: Antonin Houska <ah@cybertec.at>; Chao Li <li.evan.chao@gmail.com>; Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

On 2026-Mar-26, Jim Jones wrote:

> 
> On 26/03/2026 12:25, Antonin Houska wrote:
> > Chao Li <li.evan.chao@gmail.com> wrote:
> > 
> >> I don't think such a TAP test is necessary.
> > +1
> 
> I've kept the tests in a separate file so the committer can easily skip
> them if needed.

Thanks for noticing and patching this issue.  I have pushed the 0001
patch just now.

I decided against pushing the other patch.  Although I would have
preferred to add a test, its cost seems not trivial: there are three
full-database scans in it (one for each command), and that seemed a bit
excessive.  (There's also one extra initdb, but I'm not sure that part
is too bad since we've optimized that particular part.)

I also considered backpatching, since the code has been like this
essentially forever (i.e. at least since pg14).  However, I don't
remember any complaints about this and I would hate to destabilize
things for people without an excellent reason.  Maybe we can reconsider
after this month's minors, if somebody shows up with vehement opinions
about it.

Thanks again,

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Crear es tan difícil como ser libre" (Elsa Triolet)





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

* Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables
  2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
  2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
  2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
  2026-03-26 11:53             ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
  2026-05-05 14:32               ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Álvaro Herrera <alvherre@kurilemu.de>
@ 2026-05-06 06:33                 ` Jim Jones <jim.jones@uni-muenster.de>
  0 siblings, 0 replies; 28+ messages in thread

From: Jim Jones @ 2026-05-06 06:33 UTC (permalink / raw)
  To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Antonin Houska <ah@cybertec.at>; Chao Li <li.evan.chao@gmail.com>; Zsolt Parragi <zsolt.parragi@percona.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

On 05/05/2026 16:32, Álvaro Herrera wrote:
> I decided against pushing the other patch.  Although I would have
> preferred to add a test, its cost seems not trivial: there are three
> full-database scans in it (one for each command), and that seemed a bit
> excessive.  (There's also one extra initdb, but I'm not sure that part
> is too bad since we've optimized that particular part.)

Fair enough.

> I also considered backpatching, since the code has been like this
> essentially forever (i.e. at least since pg14).  However, I don't
> remember any complaints about this and I would hate to destabilize
> things for people without an excellent reason.  Maybe we can reconsider
> after this month's minors, if somebody shows up with vehement opinions
> about it.

Yeah, since pretty much nobody complained about it, I guess it's indeed 
safer to leave it in PG19.

Thanks for pushing it!

Best, Jim






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


end of thread, other threads:[~2026-05-06 06:33 UTC | newest]

Thread overview: 28+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 09:54 [PATCH 5/5] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2019-09-09 09:53 [PATCH 4/4] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2019-09-23 05:40 [PATCH 6/6] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2019-09-26 11:51 [PATCH 2/2] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2019-10-04 10:07 [PATCH 2/2] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2019-10-11 10:07 [PATCH 2/2] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2019-11-12 10:51 [PATCH 2/2] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2019-11-20 14:11 [PATCH 2/2] Remove the old implemenations of XLogRead(). Antonin Houska <ah@cybertec.at>
2023-08-08 00:27 [PATCH v1 4/9] ci: switch tasks to debugoptimized build Andres Freund <andres@anarazel.de>
2023-08-08 00:27 [PATCH v3 08/10] ci: switch tasks to debugoptimized build Andres Freund <andres@anarazel.de>
2023-08-08 00:27 [PATCH v3 08/10] ci: switch tasks to debugoptimized build Andres Freund <andres@anarazel.de>
2023-08-08 00:27 [PATCH v1 4/9] ci: switch tasks to debugoptimized build Andres Freund <andres@anarazel.de>
2023-08-08 00:27 [PATCH v1 4/9] ci: switch tasks to debugoptimized build Andres Freund <andres@anarazel.de>
2024-03-28 17:43 [PATCH] doc: Improve "Partition Maintenance" section Alvaro Herrera <alvherre@alvh.no-ip.org>
2026-03-24 15:35 VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-03-25 01:55 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
2026-03-25 09:05   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-03-25 10:32     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-03-25 20:38     ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Zsolt Parragi <zsolt.parragi@percona.com>
2026-03-25 23:00       ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-03-26 02:52         ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Chao Li <li.evan.chao@gmail.com>
2026-03-26 11:25           ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Antonin Houska <ah@cybertec.at>
2026-03-26 11:53             ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-04-06 18:13               ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-04-06 22:52                 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-05-01 19:34                   ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>
2026-05-05 14:32               ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Álvaro Herrera <alvherre@kurilemu.de>
2026-05-06 06:33                 ` Re: VACUUM FULL, CLUSTER, and REPACK block on other sessions' temp tables Jim Jones <jim.jones@uni-muenster.de>

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