On Wed, Mar 13, 2019 at 2:27 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > [...] Aside from some refactoring which I > think looks good anyway and prepares for future patches, the main > effect of this patch is to force the checkpointer to open and close > the files every time which seems OK to me.
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). Thoughts? It all needs to be pgindented, I'll do that later. I'll post a rebase of my undo stuff on top of this soon, to show how it looks with this interface. [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGLMPXMnSLDwgnNRFPyxvf_0bJ5HwXcHWjCp7Cvh7G%3DxEA%40mail.gmail.com -- Thomas Munro https://enterprisedb.com
0001-Refactor-the-fsync-mechanism-to-support-future-S-v13.patch
Description: Binary data
0002-fixup-v13.patch
Description: Binary data
