Date: Fri, 14 Jan 2022 12:28:48 -0800
From: Andres Freund <andres@anarazel.de>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Andrew Dunstan <andrew@dunslane.net>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Why is src/test/modules/committs/t/002_standby.pl flaky?
Message-ID: <20220114202848.x2ctpjgyreqx22dv@alap3.anarazel.de>
References: <CA+hUKGKaq1Nv79sQptmDCgJc2tGUKogW-YxAyTyF1WvJerR0RQ@mail.gmail.com>
 <CA+hUKGKPpbC2EM2tSy5-1cfj+-1d2PVwLU4kBNg8E+og+B3fbg@mail.gmail.com>
 <5d507424-13ce-d19f-2f5d-ab4c6a987316@gmail.com>
 <CA+hUKG+o_QzN1LrEPGJLa6JiZU4DpLRybkwZjdCErs2qsB6axg@mail.gmail.com>
 <1fe6b898-1722-d190-03c0-bf4c4884a748@gmail.com>
 <CA+hUKGLP+j9vuyJ2U8m+xKgx-gfCA9aJqfrchH0dWUCgYwr4eQ@mail.gmail.com>
 <2763901.1641932210@sss.pgh.pa.us>
 <d6633d6c-e547-52d1-0798-a7848583b501@gmail.com>
 <CA+hUKGKFajQiUMerS4h_=eCNKoXMYoYSkyvwMZip7WV3KUckrg@mail.gmail.com>
 <CA+hUKGK1QszjvPC_bchaQ8-DUF4jMTyqxgXpsi4L8YhN=NLpcA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+hUKGK1QszjvPC_bchaQ8-DUF4jMTyqxgXpsi4L8YhN=NLpcA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20220114202848.x2ctpjgyreqx22dv%40alap3.anarazel.de>
Precedence: bulk

Hi,

On 2022-01-14 20:31:22 +1300, Thomas Munro wrote:
> Andres and I chatted about this stuff off list and he pointed out
> something else about the wrappers in socket.c: there are more paths in
> there that work with socket events, which means more ways to lose the
> precious FD_CLOSE event.

I think it doesn't even need to touch socket.c to cause breakage. Using two
different WaitEventSets is enough.

https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsaeventselect
says:

> The FD_CLOSE network event is recorded when a close indication is received
> for the virtual circuit corresponding to the socket. In TCP terms, this
> means that the FD_CLOSE is recorded when the connection goes into the TIME
> WAIT or CLOSE WAIT states. This results from the remote end performing a
> shutdown on the send side or a closesocket. FD_CLOSE being posted after all
> data is read from a socket

So FD_CLOSE is *recorded* internally when the connection is closed. But only
posted to the visible event once all data is read. All good so far. But
combine that with:

> Issuing a WSAEventSelect for a socket cancels any previous WSAAsyncSelect or
> WSAEventSelect for the same socket and clears the internal network event
> record.

Note the bit about clearing the internal network event record. Which seems to
pretty precisely explain why we're loosing FD_CLOSEs?

And it does also explain why this is more likely after the shutdown changes:
It's more likely the network stack knows it has readable data *and* that the
connection closed. Which is recorded in the "internal network event
record". But once all the data is read, walsender.c will do another
WaitLatchOrSocket(), which does WSAEventSelect(), clearing the "internal event
record" and loosing the FD_CLOSE.


My first inclination was that we ought to wrap the socket created for windows
in pgwin32_socket() in a custom type with some additional data - including
information about already received events, an EVENT, etc. I think that might
help to remove a lot of the messy workarounds we have in socket.c etc. But: It
wouldn't do much good here, because the socket is not a socket created by
socket.c but by libpq :(.


Greetings,

Andres Freund