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: | Raw Message | Whole Thread | 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 |