Re: wrong fds used for refilenodes after pg_upgrade relfilenode changes Reply-To: - Mailing list pgsql-hackers

From Thomas Munro
Subject Re: wrong fds used for refilenodes after pg_upgrade relfilenode changes Reply-To:
Date
Msg-id CA+hUKGJ1Yox9jqXJ3qgCAt-JEguch0xSNQJvyYay3Hw-HESnwA@mail.gmail.com
Whole thread Raw
In response to Re: wrong fds used for refilenodes after pg_upgrade relfilenode changes Reply-To:  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: wrong fds used for refilenodes after pg_upgrade relfilenode changes Reply-To:  (Thomas Munro <thomas.munro@gmail.com>)
List pgsql-hackers
On Tue, Apr 5, 2022 at 2:18 AM Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Apr 1, 2022 at 5:03 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> > Another idea would be to call a new function DropPendingWritebacks(),
> > and also tell all the SMgrRelation objects to close all their internal
> > state (ie the fds + per-segment objects) but not free the main
> > SMgrRelationData object, and for good measure also invalidate the
> > small amount of cached state (which hasn't been mentioned in this
> > thread, but it kinda bothers me that that state currently survives, so
> > it was one unspoken reason I liked the smgrcloseall() idea).  Since
> > DropPendingWritebacks() is in a rather different module, perhaps if we
> > were to do that we'd want to rename PROCSIGNAL_BARRIER_SMGRRELEASE to
> > something else, because it's not really a SMGR-only thing anymore.
>
> I'm not sure that it really matters, but with the idea that I proposed
> it's possible to "save" a pending writeback, if we notice that we're
> accessing the relation with a proper lock after the barrier arrives
> and before we actually try to do the writeback. With this approach we
> throw them out immediately, so they're just gone. I don't mind that
> because I don't think this will happen often enough to matter, and I
> don't think it will be very expensive when it does, but it's something
> to think about.

The checkpointer never takes heavyweight locks, so the opportunity
you're describing can't arise.

This stuff happens entirely within the scope of a call to
BufferSync(), which is called by CheckPointBuffer().  BufferSync()
does WritebackContextInit() to set up the object that accumulates the
writeback requests, and then runs for a while performing a checkpoint
(usually spread out over time), and then at the end does
IssuePendingWritebacks().  A CFI() can be processed any time up until
the IssuePendingWritebacks(), but not during IssuePendingWritebacks().
That's the size of the gap we need to cover.

I still like the patch I posted most recently.  Note that it's not
quite as I described above: there is no DropPendingWritebacks(),
because that turned out to be impossible to implement in a way that
the barrier handler could call -- it needs access to the writeback
context.  Instead I had to expose a counter so that
IssuePendingWritebacks() could detect whether any smgrreleaseall()
calls had happened while the writeback context was being filled up
with writeback requests, by comparing with the level recorded therein.



pgsql-hackers by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: shared-memory based stats collector - v68
Next
From: Andres Freund
Date:
Subject: Re: shared-memory based stats collector - v68