Hi, On 2019-12-13 17:41:56 +1300, Thomas Munro wrote: > From 9609c9a153232fb2de169bf76158781d354c633b Mon Sep 17 00:00:00 2001 > From: Thomas Munro <tmu...@postgresql.org> > Date: Fri, 13 Dec 2019 17:12:42 +1300 > Subject: [PATCH] Don't use _mdfd_getseg() in mdsyncfiletag(). > > _mdfd_getseg() opens all segments up to the requested one. That > causes problems for mdsyncfiletag(), if mdunlinkfork() has > already unlinked other segment files. Open the file we want > directly by name instead, if we don't have it already. > > The consequence of this bug was a rare panic in the checkpointer, > made more likely if you saturated the sync request queue so that > the SYNC_FORGET_REQUEST messages for a given relation were more > likely to be absorbed in separate cycles by the checkpointer. > > Back-patch to 12. Defect in commit 3eb77eba. > > Author: Thomas Munro > Reported-by: Justin Pryzby > Discussion: https://postgr.es/m/20191119115759.GI30362%40telsasoft.com > --- > src/backend/storage/smgr/md.c | 56 ++++++++++++++++++++++++----------- > 1 file changed, 39 insertions(+), 17 deletions(-) > > diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c > index 8a9eaf6430..1a7b91b523 100644 > --- a/src/backend/storage/smgr/md.c > +++ b/src/backend/storage/smgr/md.c > @@ -1280,25 +1280,47 @@ int > mdsyncfiletag(const FileTag *ftag, char *path) ... > - /* Try to open the requested segment. */ > - v = _mdfd_getseg(reln, > - ftag->forknum, > - ftag->segno * (BlockNumber) > RELSEG_SIZE, > - false, > - EXTENSION_RETURN_NULL | > EXTENSION_DONT_CHECK_SIZE); > - if (v == NULL) > - return -1; > - > - /* Try to fsync the file. */ > - return FileSync(v->mdfd_vfd, WAIT_EVENT_DATA_FILE_SYNC); > + return result; > }
This was the only user of EXTENSION_DONT_CHECK_SIZE. It's not generally safe, as its comment says: /* * Allow opening segments which are preceded by segments smaller than * RELSEG_SIZE, e.g. inactive segments (see above). Note that this breaks * mdnblocks() and related functionality henceforth - which currently is ok, * because this is only required in the checkpointer which never uses * mdnblocks(). */ But since the above change checkpointer doesn't use EXTENSION_DONT_CHECK_SIZE anymore. Seems we should remove this code? Greetings, Andres
