On Wed, Oct 26, 2016 at 5:54 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Mon, Oct 24, 2016 at 3:48 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: >> On Thu, Oct 13, 2016 at 5:28 AM, Robert Haas <robertmh...@gmail.com> wrote: >>> On Wed, Oct 12, 2016 at 8:13 AM, Peter Eisentraut >>> <peter.eisentr...@2ndquadrant.com> wrote: >>>> On 10/4/16 10:16 AM, Julien Rouhaud wrote: >>>>> Please find attached v9 of the patch, adding the parallel worker class >>>>> and changing max_worker_processes default to 16 and max_parallel_workers >>>>> to 8. I also added Amit's explanation for the need of a write barrier >>>>> in ForgetBackgroundWorker(). >>>> >>>> This approach totally messes up the decoupling between the background >>>> worker facilities and consumers of those facilities. Having dozens of >>>> lines of code in bgworker.c that does the accounting and resource >>>> checking on behalf of parallel.c looks very suspicious. Once we add >>>> logical replication workers or whatever, we'll be tempted to add even >>>> more stuff in there and it will become a mess. >>> >>> I attach a new version of the patch that I've been hacking on in my >>> "spare time", which reduces the footprint in bgworker.c quite a bit. >>> >> >> Couple of comments - >> >> @@ -370,6 +388,9 @@ ForgetBackgroundWorker(slist_mutable_iter *cur) >> >> Assert(rw->rw_shmem_slot < >> max_worker_processes); >> slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; >> + if ((rw- >>>rw_worker.bgw_flags & BGWORKER_CLASS_PARALLEL) != 0) >> + BackgroundWorkerData- >>>parallel_terminate_count++; >> + >> slot->in_use = false; >> >> It seems upthread [1], we agreed to have a write barrier before the >> setting slot->in_use, but I don't see the same in patch. > > That's because I removed it. The reason given for the barrier was > that otherwise it might be reordered before the check of > is_parallel_worker, but that's now done by checking the postmaster's > backend-private copy of the flags, not the copy in shared memory. So > the reordering can't affect the result. >
You are right. I missed to notice that. The patch looks good to me. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers