Обсуждение: pqsignal - to be or (in this case) not to be

AKA department of second thoughts once you tested.


I guess I've spent too much time listening to people, including myself,
assuming how things work without actually testing it.

It turns out that both select() and recv() (and thus, probably send() as
well) put the thread in alertable state. This means that while we are in
a blocking select() or recv(), *our signals will be delivered using an
APC*.

What does this mean? Well, no need for our own implementation of
select(). No need for any cruft around the recv() parts of libpq
(especially nice when you start considering SSL). Etc.

I have confirmed this by strace:ing a backend. At the time of blocking,
we get this:
NtWaitForSingleObject (1948, 1, {-1, 2147483647}, ...

Which clearly shows the alertable flag set to 1...

This makes a lot of things easier. I can't beleive I missed this one
before, but I guess I was too busy considering how difficult it would be
to work around the (non existant) issue...

See my latest patch for one that works fine, and does not use any cruft.


Note - we stil need to put PG_POLL_SIGNALS() here and there, because ATM
we can only receive signals when performing a network I/O wait. This may
be fine for the postmaster, but we can for example not cancel a query as
it is now...



//Magnus

Magnus Hagander wrote:
> I guess I've spent too much time listening to people, including
myself,
> assuming how things work without actually testing it.
>
> It turns out that both select() and recv() (and thus, probably send()
as
> well) put the thread in alertable state. This means that while we are
in
> a blocking select() or recv(), *our signals will be delivered using an
> APC*.

Ha!  That makes things easy then, doesn't it!

As for the polling, adding a poll to one or two strategic plaes (like
the I/O subsystem) should cover 99% of the reasonable cases...

Merlin

"Merlin Moncure" <merlin.moncure@rcsonline.com> writes:
> As for the polling, adding a poll to one or two strategic plaes (like
> the I/O subsystem) should cover 99% of the reasonable cases...

Put it into the macro that checks for query cancel.

            regards, tom lane

>"Merlin Moncure" <merlin.moncure@rcsonline.com> writes:
>> As for the polling, adding a poll to one or two strategic plaes (like
>> the I/O subsystem) should cover 99% of the reasonable cases...
>
>Put it into the macro that checks for query cancel.

That sounds like a very good idea :-)

Are there other places that you know offhand that we should check for
signals? Consider other signals like TERM etc as well. Or is that macro
pretty much used at the points where we want signals delivered during
execution?


//Magnus

"Magnus Hagander" <mha@sollentuna.net> writes:
>> Put it into the macro that checks for query cancel.

> Are there other places that you know offhand that we should check for
> signals?

I'd like to think that that is the only place.  There are places in the
code where we need to insert more cancel checks (I think the planner is
missing a few for instance) but there is not any justification for
cluttering things with a Windows-specific addition.

            regards, tom lane