HandleParallelMessages contains CHECK_FOR_INTERRUPTS?

Tom Lane wrote:

$SUBJECT seems like a pretty bad idea, because it implies a recursive
entry to ProcessInterrupts and thence to HandleParallelMessages itself.
By what reasoning is that call necessary where it's placed?

I notice you just removed the CHECK_FOR_INTERRUPTS in
HandleParallelMessages(). Did you notice that HandleParallelMessages
calls shm_mq_receive(), which calls shm_mq_receive_bytes(), which
contains a CHECK_FOR_INTERRUPTS() call?

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Tom Lane wrote:

$SUBJECT seems like a pretty bad idea, because it implies a recursive
entry to ProcessInterrupts and thence to HandleParallelMessages itself.
By what reasoning is that call necessary where it's placed?

I notice you just removed the CHECK_FOR_INTERRUPTS in
HandleParallelMessages(). Did you notice that HandleParallelMessages
calls shm_mq_receive(), which calls shm_mq_receive_bytes(), which
contains a CHECK_FOR_INTERRUPTS() call?

I figured there were probably other cases of recursion; it's unlikely
we could prevent them altogether. But a check in a function that's
called *only* from ProcessInterrupts seems pretty dubious.

I wonder whether we should make use of HOLD_INTERRUPTS/RESUME_INTERRUPTS
to avoid the recursion scenario here. It's not really clear to me whether
this stack of function would behave sanely if it's invoked recursively
when the outer level is partway through reading a message. At the very
least, it seems like there might be a risk for out-of-order message
processing (inner recursion reads and processes next message while outer
level has read but not yet processed previous message).

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

I notice you just removed the CHECK_FOR_INTERRUPTS in
HandleParallelMessages(). Did you notice that HandleParallelMessages
calls shm_mq_receive(), which calls shm_mq_receive_bytes(), which
contains a CHECK_FOR_INTERRUPTS() call?

After study, I believe that that CHECK_FOR_INTERRUPTS() is unreachable
given that HandleParallelMessages passes nowait = true. But it's not
unlikely that future changes in shm_mq.c might introduce such calls that
are reachable.

I wonder whether we should make use of HOLD_INTERRUPTS/RESUME_INTERRUPTS
to avoid the recursion scenario here.

I concluded that that would be good future-proofing, whether or not it's
strictly necessary today, so I pushed it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers