On 12.06.2021 07:24, Thomas Munro wrote:
> On Fri, Apr 30, 2021 at 2:23 PM Thomas Munro <thomas.munro@gmail.com> wrote:
>> Here's something I wanted to park here to look into for the next
>> cycle: it turns out that kqueue's EV_EOF flag also has the right
>> semantics for this. That leads to the idea of exposing the event via
>> the WaitEventSet API, and would the bring
>> client_connection_check_interval feature to 6/10 of our OSes, up from
>> 2/10. Maybe Windows' FD_CLOSE event could get us up to 7/10, not
>> sure.
> Rebased. Added documentation tweak and a check to reject the GUC on
> unsupported OSes.
Good work. I have tested your patch on Linux and FreeBSD on three basic
cases: client killing, cable breakdown (via manipulations with firewall)
and silent closing client connection before completion of previously
started query in asynchronous manner. And all works fine.
Some comments from me:
bool
pq_check_connection(void)
{
-#if defined(POLLRDHUP)
+ WaitEvent events[3];
3 is looks like as magic constant. We might to specify a constant for
all event groups in FeBeWaitSet.
+ ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET, NULL);
+ ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetSocketPos,
WL_SOCKET_CLOSED, NULL);
+ rc = WaitEventSetWait(FeBeWaitSet, 0, events, lengthof(events), 0);
+ ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetLatchPos, WL_LATCH_SET,
MyLatch);
AFAICS, side effect to
(FeBeWaitSet->events[FeBeWaitSetSocketPos]).events by setting
WL_SOCKET_CLOSED value under calling of pq_check_connection() function
doesn't have negative impact later, does it? That is, all
WaitEventSetWait() calls have to setup socket events on its own from
scratch.
--
Regards,
Maksim Milyutin
