On Tue, Mar 26, 2019 at 12:20 AM Thomas Munro <thomas.mu...@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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
