I wrote: > Simon Riggs <si...@2ndquadrant.com> writes: >> Both of these, as attached up thread. >> Simon's patch - dropallforks.v1.patch >> Jeff's patch - DropRelFileNodeBuffers_unlock_v1.patch >> (needs a little tidyup)
> OK, will take a look. I didn't like dropallforks.v1.patch at all as presented, for several reasons: * Introducing an AllForks notation that only works in some contexts is a foot-gun of large caliber. This concern is not academic: you broke dropping of temp relations entirely, in the patch as presented, because for temp rels DropRelFileNodeBuffers would hand off to DropRelFileNodeLocalBuffers and the latter had not been taught about AllForks. * Since we have found out in this thread that the inner loop of DropRelFileNodeBuffers is performance-critical for the cases we're dealing with, it seems inappropriate to me to make its tests more complex. We want simpler, and we can have simpler given that the relation-drop case cares neither about fork nor block number. * The patch modified behavior of XLogDropRelation, which has not been shown to be performance-critical, and probably can't be because the hashtable it searches should never be all that large. It certainly doesn't matter to the speed of normal execution. I thought it would be a lot safer and probably a little bit quicker if we just split DropRelFileNodeBuffers into two routines, one for the specific-fork case and one for the all-forks case; and then the same for its main caller smgrdounlink. So I modified the patch along those lines and committed it. As committed, the smgrdounlinkfork case is actually dead code; it's never called from anywhere. I left it in place just in case we want it someday. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers