On Wed, Nov 4, 2020 at 2:57 PM Tomas Vondra
<tomas.von...@2ndquadrant.com> wrote:
> On Wed, Nov 04, 2020 at 02:49:24PM +1300, Thomas Munro wrote:
> >On Wed, Nov 4, 2020 at 2:32 PM Tomas Vondra
> ><tomas.von...@2ndquadrant.com> wrote:
> >> After a while (~1h on my machine) the pg_multixact gets over 10GB, which
> >> triggers a more aggressive cleanup (per MultiXactMemberFreezeThreshold).
> >> My guess is that this discards some of the files, but checkpointer is
> >> not aware of that, or something like that. Not sure.
> >
> >Urgh.  Thanks.  Looks like perhaps the problem is that I have
> >RegisterSyncRequest(&tag, SYNC_FORGET_REQUEST, true) in one codepath
> >that unlinks files, but not another.  Looking.
>
> Maybe. I didn't have time to investigate this more deeply, and it takes
> quite a bit of time to reproduce. I can try again with extra logging or
> test some proposed fixes, if you give me a patch.

I think this should be fixed by doing all unlinking through a common
code path.  Does this pass your test?
From 03f51c9e800a9fdff830c639344988f2b71ae746 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 4 Nov 2020 17:14:04 +1300
Subject: [PATCH] Fix unlinking of SLRU segments.

Commit dee663f7 intended to drop any queued up fsync requests before
unlinking segment files, but missed a code path.  Fix, by centralizing
the forget-and-unlink code into a single function.

Reported-by: Tomas Vondra <tomas.von...@2ndquadrant.com>
Discussion: https://postgr.es/m/20201104013205.icogbi773przyny5%40development
---
 src/backend/access/transam/slru.c | 44 +++++++++++++------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 16a7898697..0aa33cc325 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -145,7 +145,7 @@ static int	SlruSelectLRUPage(SlruCtl ctl, int pageno);
 
 static bool SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename,
 									  int segpage, void *data);
-static void SlruInternalDeleteSegment(SlruCtl ctl, char *filename);
+static void SlruInternalDeleteSegment(SlruCtl ctl, int segno);
 
 /*
  * Initialization of shared memory
@@ -1298,19 +1298,28 @@ restart:;
 }
 
 /*
- * Delete an individual SLRU segment, identified by the filename.
+ * Delete an individual SLRU segment.
  *
  * NB: This does not touch the SLRU buffers themselves, callers have to ensure
  * they either can't yet contain anything, or have already been cleaned out.
  */
 static void
-SlruInternalDeleteSegment(SlruCtl ctl, char *filename)
+SlruInternalDeleteSegment(SlruCtl ctl, int segno)
 {
 	char		path[MAXPGPATH];
 
-	snprintf(path, MAXPGPATH, "%s/%s", ctl->Dir, filename);
-	ereport(DEBUG2,
-			(errmsg("removing file \"%s\"", path)));
+	/* Forget any fsync requests queued for this segment. */
+	if (ctl->sync_handler != SYNC_HANDLER_NONE)
+	{
+		FileTag		tag;
+
+		INIT_SLRUFILETAG(tag, ctl->sync_handler, segno);
+		RegisterSyncRequest(&tag, SYNC_FORGET_REQUEST, true);
+	}
+
+	/* Unlink the file. */
+	snprintf(path, MAXPGPATH, "%s/%04X", ctl->Dir, segno);
+	ereport(DEBUG2, (errmsg("removing file \"%s\"", path)));
 	unlink(path);
 }
 
@@ -1322,7 +1331,6 @@ SlruDeleteSegment(SlruCtl ctl, int segno)
 {
 	SlruShared	shared = ctl->shared;
 	int			slotno;
-	char		path[MAXPGPATH];
 	bool		did_write;
 
 	/* Clean out any possibly existing references to the segment. */
@@ -1364,23 +1372,7 @@ restart:
 	if (did_write)
 		goto restart;
 
-	snprintf(path, MAXPGPATH, "%s/%04X", ctl->Dir, segno);
-	ereport(DEBUG2,
-			(errmsg("removing file \"%s\"", path)));
-
-	/*
-	 * Tell the checkpointer to forget any sync requests, before we unlink the
-	 * file.
-	 */
-	if (ctl->sync_handler != SYNC_HANDLER_NONE)
-	{
-		FileTag		tag;
-
-		INIT_SLRUFILETAG(tag, ctl->sync_handler, segno);
-		RegisterSyncRequest(&tag, SYNC_FORGET_REQUEST, true);
-	}
-
-	unlink(path);
+	SlruInternalDeleteSegment(ctl, segno);
 
 	LWLockRelease(shared->ControlLock);
 }
@@ -1413,7 +1405,7 @@ SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename, int segpage, void *data)
 	int			cutoffPage = *(int *) data;
 
 	if (ctl->PagePrecedes(segpage, cutoffPage))
-		SlruInternalDeleteSegment(ctl, filename);
+		SlruInternalDeleteSegment(ctl, segpage / SLRU_PAGES_PER_SEGMENT);
 
 	return false;				/* keep going */
 }
@@ -1425,7 +1417,7 @@ SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename, int segpage, void *data)
 bool
 SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int segpage, void *data)
 {
-	SlruInternalDeleteSegment(ctl, filename);
+	SlruInternalDeleteSegment(ctl, segpage / SLRU_PAGES_PER_SEGMENT);
 
 	return false;				/* keep going */
 }
-- 
2.20.1

Reply via email to