On Tue, Dec 14, 2021 at 7:53 AM Andres Freund <and...@anarazel.de> wrote: > On 2021-12-13 17:51:00 +1300, Thomas Munro wrote: > > I tried that. It seems OK, and gets rid of the PG_FINALLY(), which is > > nice. Latches still have higher priority, and still have the fast > > return if already set and you asked for only one event, but now if you > > ask for nevents > 1 we'll make the syscall too so we'll see the > > WL_SOCKET_CLOSED. > > Isn't a certain postgres committer that cares a lot about unnecessary syscalls > going to be upset about this one? Even with the nevents > 1 optimization? Yes, > right now there's no other paths that do so, but I don't like the corner this > paints us in.
Well, I was trying to avoid bikeshedding an API change just for a hypothetical problem we could solve when the time comes (say, after fixing the more egregious problems with Append's WES usage), but here goes: we could do something like AddWaitEventToSet(FeBeWaitSet, WL_LATCH_SET_LOPRIO, ...) that is translated to WL_LATCH_SET internally but also sets a flag to enable this no-really-please-poll-all-the-things-if-there-is-space behaviour. > From a different angle: Why do we need to perform the client connection check > if the latch is set? Imagine a parallel message that arrives just as our connection check CFI routine runs, and sets the latch. It'd be arbitrary and bizarre if that caused us to skip the check. So we have to ignore it, and the question is just how. I presented two different ways. A third way would be to create an entirely new WES for this use case; nope, that's either wasteful of an fd or wasteful of system calls for a temporary WES and likely more PG_TRY() stuff to avoid leaking it. A fourth could be to modify the WES like the simple code in v2/v3, but make the WES code no-throw or pretend it's no-throw, which I didn't seriously consider (I mean, if, say, WaitForMultipleObjects() returns WAIT_FAILED and ERRORs then your session is pretty much hosed and your WES is probably never going to work correctly again, but it still seems to break basic rules of programming decency and exception safety to leave the WES sans latch on non-local exit, which is why I added the PG_FINALLY() that offended you to v3).
