On Mon, May 31, 2021 at 8:12 AM Andres Freund <and...@anarazel.de> wrote: > On 2021-05-30 16:39:48 +1200, Thomas Munro wrote: > > I thought about a few different ways to encapsulate this API > > difference in PostgreSQL, and toyed with two: > > > > 1. We could define our own fake O_DIRECT flag, and translate that to > > the right thing inside BasicOpenFilePerm(). That seems a bit icky. > > We'd have to be careful not to collide with system defined flags and > > worry about changes. We do that sort of thing for Windows, though > > that's a bit different, there we translate *all* the flags from > > POSIXesque to Windowsian. > > > > 2. We could make an extended BasicOpenFilePerm() variant that takes a > > separate boolean parameter for direct, so that we don't have to hijack > > any flag space, but now we need new interfaces just to tolerate a > > rather niche system. > > I don't think 2) really covers the problem on its own. It's fine for > things that directly use BasicOpenFilePerm(), but what about "virtual > file descriptors" (PathNameOpenFile())? I.e. what md.c et al use? There > we need to store the fact that we want non-buffered IO as part of the > vfd, otherwise we'll loose that information when re-opening the file > later.
Right, a bit more API perturbation is required to traffic the separate flags around for VFDs, which is all a bit unpleasant for a feature that most people don't care about. For comparison, here is my sketch of idea #1. I pick an arbitrary value to use as PG_O_DIRECT (I don't want to define O_DIRECT for fear of breaking other code that might see it and try to pass it into open()... for all I know, it might happen to match OS-internal value O_NASAL_DEMONS), and statically assert that it doesn't collide with standard flags we're using, and I strip it out of the flags I pass in to open(). As I said, a bit icky, but it's a tiny and localised patch, which is nice. I also realised that it probably wasn't right to raise an ERROR, so in this version I return -1 when fcntl() fails.
0001-Support-direct-I-O-on-macOS--idea1.patch
Description: Binary data
