On Sun, Dec 8, 2019 at 10:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > So, just idly looking at the code in src/backend/port/win32/signal.c
> > and src/port/kill.c, I have to wonder why we have this baroque-looking
> > design of using *two* signal management threads. And, if I'm
> > reading it right, we create an entire new pipe object and an entire
> > new instance of the second thread for each incoming signal. Plus, the
> > signal senders use CallNamedPipe (hence, underneath, TransactNamedPipe)
> > which means they in effect wait for the recipient's signal-handling
> > thread to ack receipt of the signal. Maybe there's a good reason for
> > all this but it sure seems like a lot of wasted cycles from here.
>
> Here's a possible patch (untested by me) to get rid of the second thread
> and the new-pipe-for-every-request behavior. I believe that the existing
> logic may be based on Microsoft's "Multithreaded Pipe Server" example [1]
> or something similar, but that's based on an assumption that servicing
> a client request may take a substantial amount of time and it's worth
> handling requests concurrently. Neither point applies in this context.
>
> Doing it like this seems attractive to me because it gets rid of two
> different failure modes: inability to create a new thread and inability
> to create a new pipe handle. Now on the other hand, it means that
> inability to complete the read/write transaction with a client right
> away will delay processing of other signals. But we know that the
> client is engaged in a CallNamedPipe operation, so how realistic is
> that concern?
>
Right, the client is engaged in a CallNamedPipe operation, but the
current mechanism can allow multiple such clients and that might lead
to faster processing of signals. I am not sure how much practical
advantage we have with the current implementation over proposed
change, so not sure if we should just get rid of it on that grounds.
Ideally, we can run a couple of tests to see if there is any help in
servicing the signals with this mechanism over proposed change on
different Windows machines, but is it really worth the effort?
Your patch looks good to me and I don't see much problem if you want
to proceed with it, but I am just not sure if the current mechanism is
completely bogus.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com