From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andrew Borodin <amborodin(at)acm(dot)org>, amul sul <sulamul(at)gmail(dot)com> |
Subject: | Re: background sessions |
Date: | 2017-03-15 16:58:56 |
Message-ID: | CA+TgmoY1fQE8Cw=NRcX7AtPR1n-=v-YTkhR=tNUVrEu04peNJw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Mar 15, 2017 at 6:43 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-03-15 17:01:30 | Re: [POC] hash partitioning |
Previous Message | Tom Lane | 2017-03-15 16:55:49 | Re: [COMMITTERS] pgsql: Use asynchronous connect API in libpqwalreceiver |