Re: WL_SOCKET_ACCEPT fairness on Windows - Mailing list pgsql-hackers

On Thu, May 18, 2023 at 8:53 PM Wei Wang (Fujitsu)
<wangw.fnst@fujitsu.com> wrote:
> On Sat, April 1, 2023 at 11:00 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> I tried to test this patch on Windows. And I did cover the new code path below:
> ```
> +               /* We have another event to decode. */
> +               cur_event = &set->events[next_pos + (rc - WAIT_OBJECT_0)];
> ```
>
> But I have one thing want to confirm:
> In my tests, I think the scenario with two different events (e.g., one ending
> socket connection and one incoming socket connection) has been optimized.
> However, it seems that when there are multiple incoming socket connections, the
> function WaitEventSetWaitBlock is called multiple times instead of being called
> once. Is this our expected result?

Thanks for testing!  Maybe I am misunderstanding something: what I
expect to happen is that we call *WaitForMultipleObjects()* one extra
time (to see if there is another event available immediately, and
usually there is not), but not WaitEventSetWaitBlock().

> Here are my test details:
> I use the debugger to ensure that multiple events occur when the function
> WaitEventSetWaitBlock is called. First, I added a breakpoint in the below code:
> ```
>                 /*
>                 * Sleep.
>                 *
>                 * Need to wait for ->nevents + 1, because signal handle is in [0].
>                 */
> b               rc = WaitForMultipleObjects(set->nevents + 1, set->handles, FALSE,
>                                                                         cur_timeout);
> ```
> And then make sure that the postmaster process enters the function
> WaitForMultipleObjects. (I think the postmaster process will only return from
> the function WaitForMultipleObjects when any object is signaled or timeout
> occurs). Before the timeout occurs, I set up multiple socket connections using
> psql (the first connection makes the process returns from the function
> WaitForMultipleObjects). Then, as I continued debugging, multiple socket
> connections were handled by different calls of the function
> WaitEventSetWaitBlock.

This is a good test.  Also, to test the exact scenario I was worrying
about, you could initiate 4 psql sessions while the server is blocked
in a debugger, 2 on a Unix domain socket, and 2 on a TCP socket, and
then when you "continue" the server with a break on (say) accept() so
that you can "continue" each time, you should see that it alternates
between the two sockets accepting from both "fairly", instead of
draining one socket entirely first.

> Please let me know if I am missing something.

I think your report already shows that it basically works correctly.
Do you think it's worth committing for 17 to make it work more like
Unix, or was I just being paranoid?