From: | Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Concurrency bug in UPDATE of partition-key |
Date: | 2018-06-18 04:51:04 |
Message-ID: | CAJ3gD9ds8=WmoseQ6T8E0YLLAJYFp0oHm=MEb8wVun5TG2uZWA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11 June 2018 at 15:29, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Jun 11, 2018 at 3:02 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Thu, Jun 7, 2018 at 1:53 PM, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
>>> On 7 June 2018 at 11:44, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>>> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>
>>>> wrote:
>>>>
>>>> I think this will allow before row delete triggers to be fired more than
>>>> once. Normally, if the EvalPlanQual testing generates a new tuple, we don't
>>>> fire the triggers again.
>>>
>>> If there are BR delete triggers, the tuple will be locked using
>>> GetTupleForTrigger(). So the subsequent EvalPlanQual testing won't be
>>> run, since the tuple is already locked due to triggers having run.
>>>
>>> But that leads me to think : The same concurrency issue can occur in
>>> GetTupleForTrigger() also. Say, a concurrent session has already
>>> locked the tuple, and GetTupleForTrigger() would wait and then return
>>> the updated tuple in its last parameter newSlot. In that case, we need
>>> to pass this slot back through ExecBRDeleteTriggers(), and further
>>> through epqslot parameter of ExecDelete(). But yes, in this case, we
>>> should avoid calling this trigger function the second time.
>>>
>>> If you agree on the above, I will send an updated patch.
>>>
>>
>> Sounds reasonable to me.
Attached is v2 version of the patch. It contains the above
trigger-related issue fixed.
The updated tuple is passed back using the existing newslot parameter
of GetTupleForTrigger(). When ExecBRDeleteTriggers() is called using a
new epqslot parameter, it means caller wants to skip the trigger
execution, because the updated tuple needs to be again checked for
constraints. I have added comments of this behaviour in the
ExecBRDeleteTriggers() function header.
Because the trigger execution is skipped the first time ExecDelete()
is called, I didn't have to explicitly add any changes for preventing
trigger execution the second time.
> Try to add a test case which covers BR trigger code path where you are
> planning to update.
Added the test cases. Also added cases where the concurrent session
causes the EvalPlanQual() test to fail, so the partition key update
does not happen.
--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company
Attachment | Content-Type | Size |
---|---|---|
fix_concurrency_bug_v2.patch | application/octet-stream | 12.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alexander Korotkov | 2018-06-18 04:56:34 | Re: [HACKERS] GUC for cleanup indexes threshold. |
Previous Message | Michael Paquier | 2018-06-18 04:42:03 | Re: Fix some error handling for read() and errno |