Thread: New socket code for win32
Here's an attempt at new socket and signal code for win32. It works on the principle of turning sockets into non-blocking, and then emulate blocking behaviour on top of that, while allowing signals to run. Signals are now implemented using an event instead of APCs, thus getting rid of the issue of APCs not being compatible with "old style" sockets functions. It also moves the win32 specific code away from pqsignal.h/c into port/win32, and also removes the "thread style workaround" of the APC issue previously in place. In order to make things work, a few things are also changed in pgstat.c: 1) There is now a separate pipe to the collector and the bufferer. This is required because the pipe will otherwise only be signalled in one of the processes when the postmaster goes down. The MS winsock code for select() must have some kind of workaround for this behaviour, but I have found no stable way of doing that. You really are not supposed to use the same socket from more than one process (unless you use WSADuplicateSocket(), in which case the docs specifically say that only one will be flagged). 2) The check for "postmaster death" is moved into a separate select() call after the main loop. The previous behaviour select():ed on the postmaster pipe, while later explicitly saying "we do NOT check for postmaster exit inside the loop". The issue was that the code relies on the same select() call seeing both the postmaster pipe *and* the pgstat pipe go away. This does not always happen, and it appears that useing WSAEventSelect() makes it even more common that it does not. Since it's only called when the process exits, I don't think using a separate select() call will have any significant impact on how the stats collector works. Anyway. Here is a patch for review. The two files go in backend/port/win32/. Passes all regression tests expected (time and ordering ones still fails, pgstat works) //Magnus
Attachment
I have been running with this patch for a week now, and it is more stable than what's in there now for me. So I think it's ready to go. I have attached an updated socket.c file that changes the following: * Corrects two incorrect comments * Zeroes out the fd_sets in select at EINTR for consistency //Magnus >-----Original Message----- >From: Magnus Hagander >Sent: den 4 april 2004 22:08 >To: pgsql-patches@postgresql.org >Subject: [PATCHES] New socket code for win32 > > >Here's an attempt at new socket and signal code for win32. > >It works on the principle of turning sockets into >non-blocking, and then >emulate blocking behaviour on top of that, while allowing signals to >run. Signals are now implemented using an event instead of APCs, thus >getting rid of the issue of APCs not being compatible with "old style" >sockets functions. > >It also moves the win32 specific code away from pqsignal.h/c into >port/win32, and also removes the "thread style workaround" of the APC >issue previously in place. > >In order to make things work, a few things are also changed in >pgstat.c: > >1) There is now a separate pipe to the collector and the bufferer. This >is required because the pipe will otherwise only be signalled in one of >the processes when the postmaster goes down. The MS winsock code for >select() must have some kind of workaround for this behaviour, but I >have found no stable way of doing that. You really are not supposed to >use the same socket from more than one process (unless you use >WSADuplicateSocket(), in which case the docs specifically say that only >one will be flagged). > >2) The check for "postmaster death" is moved into a separate select() >call after the main loop. The previous behaviour select():ed on the >postmaster pipe, while later explicitly saying "we do NOT check for >postmaster exit inside the loop". >The issue was that the code relies on the same select() call >seeing both >the postmaster pipe *and* the pgstat pipe go away. This does not always >happen, and it appears that useing WSAEventSelect() makes it even more >common that it does not. >Since it's only called when the process exits, I don't think using a >separate select() call will have any significant impact on how >the stats >collector works. > > > >Anyway. Here is a patch for review. The two files go in >backend/port/win32/. > >Passes all regression tests expected (time and ordering ones still >fails, pgstat works) > >//Magnus > >
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Magnus Hagander wrote:
> I have been running with this patch for a week now, and it is more
> stable than what's in there now for me. So I think it's ready to go.
>
> I have attached an updated socket.c file that changes the following:
> * Corrects two incorrect comments
> * Zeroes out the fd_sets in select at EINTR for consistency
>
>
> //Magnus
>
>
> >-----Original Message-----
> >From: Magnus Hagander
> >Sent: den 4 april 2004 22:08
> >To: pgsql-patches@postgresql.org
> >Subject: [PATCHES] New socket code for win32
> >
> >
> >Here's an attempt at new socket and signal code for win32.
> >
> >It works on the principle of turning sockets into
> >non-blocking, and then
> >emulate blocking behaviour on top of that, while allowing signals to
> >run. Signals are now implemented using an event instead of APCs, thus
> >getting rid of the issue of APCs not being compatible with "old style"
> >sockets functions.
> >
> >It also moves the win32 specific code away from pqsignal.h/c into
> >port/win32, and also removes the "thread style workaround" of the APC
> >issue previously in place.
> >
> >In order to make things work, a few things are also changed in
> >pgstat.c:
> >
> >1) There is now a separate pipe to the collector and the bufferer. This
> >is required because the pipe will otherwise only be signalled in one of
> >the processes when the postmaster goes down. The MS winsock code for
> >select() must have some kind of workaround for this behaviour, but I
> >have found no stable way of doing that. You really are not supposed to
> >use the same socket from more than one process (unless you use
> >WSADuplicateSocket(), in which case the docs specifically say that only
> >one will be flagged).
> >
> >2) The check for "postmaster death" is moved into a separate select()
> >call after the main loop. The previous behaviour select():ed on the
> >postmaster pipe, while later explicitly saying "we do NOT check for
> >postmaster exit inside the loop".
> >The issue was that the code relies on the same select() call
> >seeing both
> >the postmaster pipe *and* the pgstat pipe go away. This does not always
> >happen, and it appears that useing WSAEventSelect() makes it even more
> >common that it does not.
> >Since it's only called when the process exits, I don't think using a
> >separate select() call will have any significant impact on how
> >the stats
> >collector works.
> >
> >
> >
> >Anyway. Here is a patch for review. The two files go in
> >backend/port/win32/.
> >
> >Passes all regression tests expected (time and ordering ones still
> >fails, pgstat works)
> >
> >//Magnus
> >
> >
Content-Description: socket.c
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Patch applied with later socket.c file used. Thanks. This is a major Win32 accomplishment. --------------------------------------------------------------------------- Magnus Hagander wrote: > Here's an attempt at new socket and signal code for win32. > > It works on the principle of turning sockets into non-blocking, and then > emulate blocking behaviour on top of that, while allowing signals to > run. Signals are now implemented using an event instead of APCs, thus > getting rid of the issue of APCs not being compatible with "old style" > sockets functions. > > It also moves the win32 specific code away from pqsignal.h/c into > port/win32, and also removes the "thread style workaround" of the APC > issue previously in place. > > In order to make things work, a few things are also changed in pgstat.c: > > 1) There is now a separate pipe to the collector and the bufferer. This > is required because the pipe will otherwise only be signalled in one of > the processes when the postmaster goes down. The MS winsock code for > select() must have some kind of workaround for this behaviour, but I > have found no stable way of doing that. You really are not supposed to > use the same socket from more than one process (unless you use > WSADuplicateSocket(), in which case the docs specifically say that only > one will be flagged). > > 2) The check for "postmaster death" is moved into a separate select() > call after the main loop. The previous behaviour select():ed on the > postmaster pipe, while later explicitly saying "we do NOT check for > postmaster exit inside the loop". > The issue was that the code relies on the same select() call seeing both > the postmaster pipe *and* the pgstat pipe go away. This does not always > happen, and it appears that useing WSAEventSelect() makes it even more > common that it does not. > Since it's only called when the process exits, I don't think using a > separate select() call will have any significant impact on how the stats > collector works. > > > > Anyway. Here is a patch for review. The two files go in > backend/port/win32/. > > Passes all regression tests expected (time and ordering ones still > fails, pgstat works) > > //Magnus Content-Description: newsignal.patch [ Attachment, skipping... ] Content-Description: signal.c [ Attachment, skipping... ] Content-Description: socket.c [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073