Hi, On 2022-01-17 14:50:27 +0100, Magnus Hagander wrote: > On Mon, Jan 17, 2022 at 12:31 AM Andres Freund <and...@anarazel.de> wrote: > > > > Hi, > > > > On 2022-01-16 15:28:00 -0800, Andres Freund wrote: > > > I hacked that up last night. And a fix or two later, it seems to be > > > working. What I'd missed at first is that the event needs to be reset in > > > reached_end_position(), otherwise we'll busy loop. > > You can create the event with bManualReset set to False to avoid that, > no? With this usecase, I don't really see a reason not to do that > instead?
The problem I'm referring to is that some types of events are edge triggered. Which we've been painfully reminded of recently: https://www.postgresql.org/message-id/CA%2BhUKG%2BOeoETZQ%3DQw5Ub5h3tmwQhBmDA%3DnuNO3KG%3DzWfUypFAw%40mail.gmail.com It appears there's no guarantee that you'll see e.g. FD_CLOSE if you use short-lived events (the FD_CLOSE is recorded internally but not signalled immediately if there's still readable data, and the internal record is reset by WSAEventSelect()). > > > I wonder if using a short-lived event handle would have dangers of missing > > > FD_CLOSE here as well? It'd probably be worth avoiding the risk by > > > creating > > > the event just once. > > > > > > I just wasn't immediately sure where to stash it. Probably just by adding > > > a > > > field in StreamCtl, that ReceiveXlogStream() then sets? So far it's > > > constant > > > once passed to ReceiveXlogStream(), but I don't really see a reason why > > > it'd > > > need to stay that way? > > > > Oops, attached the patch this time. > > Do we really want to create a new event every time? Those are kernel > objects, so they're not entirely free, but that part maybe doesn't > matter. Wouldn't it be cleaner to do it like we do in > pgwin32_waitforsinglesocket() which is create it once and store it in > a static variable? Or is that what you're suggesting above in the "I > wonder if" part? Yes, that's what I was suggesting. I wasn't thinking of using a static var, but putting it in StreamCtl. Note that what pgwin32_waitforsinglesocket() is doing doesn't protect against the problem referenced above, because it still is reset by WSAEventSelect. Greetings, Andres Freund