Re: background workers, round three

I briefly checked these patches. Let me add some comments.

* terminate-worker-v1.patch
TerminateBackgroundWorker() turns on slot->terminate flag under
LW_SHARED lock. Is it reasonable because all the possible caller
is the background worker process itself, isn't it?

* ephemeral-precious-v1.patch
AtEOXact_BackgroundWorker() is located around other AtEOXact_*
routines. Doesn't it makes resource management complicated?
In case when main process goes into error handler but worker
process is still running in health, it may continue to calculate
something and put its results on shared memory segment, even
though main process suggest postmaster to kill it.

All the ResourceOwnerRelease() callbacks are located prior to
AtEOXact_BackgroundWorker(), it is hard to release resources
being in use by background worker, because they are healthy
running until it receives termination signal, but sent later.
In addition, it makes implementation complicated if we need to
design background workers to release resources if and when it
is terminated. I don't think it is a good coding style, if we need
to release resources in different location depending on context.

So, I'd like to propose to add a new invocation point of
ResourceOwnerRelease() after all AtEOXact_* jobs, with
new label something like RESOURCE_RELEASE_FINAL.

In addition, AtEOXact_BackgroundWorker() does not synchronize
termination of background worker processes being killed.
Of course it depends on situation, I think it is good idea to wait
for completion of worker processes to be terminated, to ensure
resource to be released is backed to the main process if above
ResourceOwnerRelease() do the job.

Thanks,

2013/10/11 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Fri, Oct 11, 2013 at 9:27 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> Finally I got the chance to put my hands on this code. Really sorry
>> for the late replay.
>
> Thanks for the review. I'll respond to this in more detail later, but
> to make a long story short, I'm looking to apply
> terminate-worker-v1.patch (possibly with modifications after going
> over your review comments), but I'm not feeling too good any more
> about ephemeral-precious-v1.patch, because my experience with those
> facilities has so far proved unsatisfying. I think I'd like to
> withdraw the latter patch pending further study.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>