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