On Tue, Dec 5, 2023 at 1:35 PM Tristan Partin <tris...@neon.tech> wrote: > On Wed Nov 29, 2023 at 11:48 AM CST, Tristan Partin wrote: > > I am not completely in love with the code I have written. Lots of > > conditional compilation which makes it hard to read. Looking forward to > > another round of review to see what y'all think. > > Ok. Here is a patch which just uses select(2) with a timeout of 1s or > pselect(2) if it is available. I also moved the state machine processing > into its own function.
Hmm, this adds a function called pqSocketPoll to psql/command.c. But there already is such a function in libpq/fe-misc.c. It's not quite the same, though. Having the same function in two different modules with subtly different definitions seems like it's probably not the right idea. Also, this seems awfully complicated for something that's supposed to (checks notes) wait for a file descriptor to become ready for I/O for up to 1 second. It's 160 lines of code in pqSocketPoll and another 50 in the caller. If this is really the simplest way to do this, we really need to rethink our libpq APIs or, uh, something. I wonder if we could make this simpler by, say: - always use select - always use a 1-second timeout - if it returns faster because the race condition doesn't happen, cool - if it doesn't, well then you get to wait for a second, oh well I don't feel strongly that that's the right way to go and if Heikki or some other committer wants to go with this more complex conditional approach, that's fine. But to me it seems like a lot. -- Robert Haas EDB: http://www.enterprisedb.com
