On Mon, Jun 27, 2016 at 10:21 AM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Sun, Jun 26, 2016 at 3:57 PM, Julien Rouhaud
> <julien.rouh...@dalibo.com> wrote:
>> On 26/06/2016 08:37, Amit Kapila wrote:
>>> @@ -370,6 +379,8 @@ ForgetBackgroundWorker(slist_mutable_iter *cur)
>>> Assert(rw->rw_shmem_slot <
>>> slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot];
>>> slot->in_use =
>>> + if (slot->parallel)
>>> + BackgroundWorkerData->parallel_terminate_count++;
>>> I think operations on parallel_terminate_count are not safe.
>>> ForgetBackgroundWorker() and RegisterDynamicBackgroundWorker() can try
>>> to read write at same time. It seems you need to use atomic
>>> operations to ensure safety.
>> But operations on parallel_terminate_count are done by the postmaster,
>> and per Robert's previous email postmaster can't use atomics:
> I think as the parallel_terminate_count is only modified by postmaster
> and read by other processes, such an operation will be considered
> atomic. We don't need to specifically make it atomic unless multiple
> processes are allowed to modify such a counter. Although, I think we
> need to use some barriers in code to make it safe.
> @@ -272,6 +279,8 @@ BackgroundWorkerStateChange(void)
> slot->pid = 0;
> slot->in_use = false;
> + if (slot->parallel)
> I think the check of slot->parallel and increment to
> parallel_terminate_count should be done before marking slot->in_use to
> false. Consider the case if slot->in_use is marked as false and then
> before next line execution, one of the backends registers dynamic
> worker (non-parallel worker), so that
> backend can see this slot as free and use it. It will also mark the
> parallel flag of slot as false. Now when postmaster will check the
> slot->parallel flag, it will find it false and then skip the increment
> to parallel_terminate_count.
If you agree with above theory, then you need to use write barrier as
well after incrementing the parallel_terminate_count to avoid
re-ordering with slot->in_use flag.
Sent via pgsql-hackers mailing list (email@example.com)
To make changes to your subscription: