Re: wrong fds used for refilenodes after pg_upgrade relfilenode changes Reply-To:

On Tue, Apr 5, 2022 at 2:18 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Apr 1, 2022 at 5:03 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)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.