On Wed, Mar 15, 2017 at 6:43 AM, Pavel Stehule <pavel.steh...@gmail.com> wrote: > I don't understand - CHECK_FOR_INTERRUPTS called from executor implicitly.
True. So there shouldn't be any problem here. I'm confused as can be about what you want changed. Some review of the patch itself: + pq_redirect_to_shm_mq(session->seg, session->command_qh); + pq_beginmessage(&msg, 'X'); + pq_endmessage(&msg); + pq_stop_redirect_to_shm_mq(); shm_redirect_to_shm_mq() wasn't really designed to be used this way; it's designed for use by the worker, not the process that launched it. If an error occurs while output is redirected, bad things will happen. I think it would be better to find a way of sending that message to the queue without doing this. Also, I suspect this is deadlock-prone. If we get stuck trying to send a message to the background session while the queue is full, and at the same time the session is stuck trying to send us a long error message, we will have an undetected deadlock. That's why pg_background() puts the string being passed to the worker into the DSM segment in its entirety, rather than sending it through a shm_mq. + elog(ERROR, "no T before D"); That's not much of an error message, even for an elog. + elog(ERROR, "already received a T message"); Nor that. None of these functions have function header comments. Or much in the way of internal comments. For example: + case '1': + break; + case 'E': + rethrow_errornotice(&msg); + break; That's really not clear without more commentary. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
