| From: | Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com> | 
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> | 
| Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Josh Berkus <josh(at)agliodbs(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Peter Geoghegan <pg(at)heroku(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> | 
| Subject: | Re: Rename max_parallel_degree? | 
| Date: | 2016-06-28 21:27:24 | 
| Message-ID: | 9f84548d-8b59-b5c8-510d-991f8ee3ab20@dalibo.com | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
On 28/06/2016 04:44, Amit Kapila wrote:
> On Mon, Jun 27, 2016 at 10:35 PM, Julien Rouhaud
>>
>> There's already a pg_memory_barrier() call in
>> BackgroundWorkerStateChange(), to avoid reordering the notify_pid load.
>> Couldn't we use it to also make sure the parallel_terminate_count
>> increment happens before the slot->in_use store?
>>
> 
> Yes, that is enough, as memory barrier ensures that both loads and
> stores are completed before any loads and stores that are after
> barrier.
> 
>>  I guess that a write
>> barrier will be needed in ForgetBacgroundWorker().
>>
> 
> Yes.
> 
>>>> 2.
>>>> + if (parallel && (BackgroundWorkerData->parallel_register_count -
>>>> +
>>>> BackgroundWorkerData->parallel_terminate_count) >=
>>>> +
>>>> max_parallel_workers)
>>>> + {
>>>> + LWLockRelease(BackgroundWorkerLock);
>>>> + return
>>>> false;
>>>> + }
>>>> +
>>>>
>>>> I think we need a read barrier here, so that this check doesn't get
>>>> reordered with the for loop below it.
>>
>> You mean between the end of this block and the for loop?
>>
> 
> Yes.
> 
>>>>  Also, see if you find the code
>>>> more readable by moving the after && part of check to next line.
>>
>> I think I'll just pgindent the file.
>>
> 
> make sense.
> 
> 
Thanks a lot for the help!
PFA v6 which should fix all the issues mentioned.  Also, after second
thought I didn't add the extra hint about max_worker_processes in the
max_parallel_worker paragraph, since this line was a duplicate of the
precedent paragraph, it seemed better to leave the text as is.
-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org
| Attachment | Content-Type | Size | 
|---|---|---|
| global_max_parallel_workers-v6.diff | text/plain | 12.1 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Oleg Bartunov | 2016-06-28 21:27:46 | Re: Should phraseto_tsquery('simple', 'blue blue') @@ to_tsvector('simple', 'blue') be true ? | 
| Previous Message | Markus Wanner | 2016-06-28 20:31:32 | Re: IPv6 link-local addresses and init data type |