Re: pg_background (and more parallelism infrastructure patches)

On Sat, Jul 26, 2014 at 9:32 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 25, 2014 at 4:16 PM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com> wrote:
> > On Fri, Jul 25, 2014 at 02:11:32PM -0400, Robert Haas wrote:
> >> + pq_mq_busy = true;
> >> +
> >> + iov[0].data = &msgtype;
> >> + iov[0].len = 1;
> >> + iov[1].data = s;
> >> + iov[1].len = len;
> >> +
> >> + Assert(pq_mq_handle != NULL);
> >> + result = shm_mq_sendv(pq_mq_handle, iov, 2, false);
> >> +
> >> + pq_mq_busy = false;
> >
> > Don't you need a PG_TRY block here to reset pq_mq_busy?
>
> No. If shm_mq_sendv is interrupted, we can't use the shm_mq any more.
> But since that should only happen if an interrupt arrives while the
> queue is full, I think that's OK.

I think here not only on interrupt, but any other error in this
function shm_mq_sendv() path (one example is WaitLatch())
could lead to same behaviour.

> (Think about the alternatives: if
> the queue is full, we have no way of notifying the launching process
> without waiting for it to retrieve the results, but it might not do
> that right away, and if we've been killed we need to die *now* not
> later.)

So in such cases what is the advise to users, currently they will
see the below message:
postgres=# select * from pg_background_result(5124) as (x int);
ERROR: lost connection to worker process with PID 5124

One way is to ask them to check logs, but what about if they want
to handle error and take some action?

Another point about error handling is that to execute the sql in
function pg_background_worker_main(), it starts the transaction
which I think doesn't get aborted if error occurs and similarly handling
for timeout seems to be missing in error path.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com