Re: Refactoring the checkpointer's fsync request queue

On Tue, Mar 26, 2019 at 09:22:56AM -0400, Robert Haas wrote:
> On Tue, Mar 26, 2019 at 12:20 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> > I've been trying to decide if that is a problem.  Maybe there is a
> > performance angle, and I'm also wondering if it might increase the
> > risk of missing a write-back error.  Of course we'll find a proper
> > solution to that problem (perhaps fd-passing or sync-on-close[1]), but
> > I don't want to commit anything in the name of refactoring that might
> > make matters worse incidentally.  Or perhaps those worries are bogus,
> > since the checkpointer calls smgrcloseall() at the end anyway.
> >
> > On balance, I'm inclined to err on the side of caution and try to keep
> > things a bit closer to the way they are for now.
> >
> > Here's a fixup patch.  0001 is the same as Shawn's v12 patch, and 0002
> > has my proposed changes to switch to callbacks that actually perform
> > the sync and unlink operations given a file tag, and do so via the
> > SMGR fd cache, rather than exposing the path to sync.c.  This moves us
> > back towards the way I had it in an earlier version of the patch, but
> > instead of using smgrsyncimmed() as I had it, it goes via Shawn's new
> > sync handler function lookup table, allowing for non-smgr components
> > to use this machinery in future (as requested by Andres).
> 
> Strong +1.  Not only might closing and reopening the files have
> performance and reliability implications, but a future smgr might talk
> to the network, having no local file to sync.

Makes sense for now. When we re-visit the fd-passing or sync-on-close 
implementations, we can adapt the changes relatively easily given the 
rest of the framework is staying intact. I am hoping these patches do 
not delay the last fsync-gate issue discussion further.  

-- 
Shawn Debnath
Amazon Web Services (AWS)