Re: Concurrency bug in UPDATE of partition-key - Mailing list pgsql-hackers

From Dilip Kumar
Subject Re: Concurrency bug in UPDATE of partition-key
Date
Msg-id CAFiTN-uWnvib-3S+_8Ea2_VGgtFs1grpNh8c94qSOT7JgbsUyg@mail.gmail.com
Whole thread Raw
In response to Re: Concurrency bug in UPDATE of partition-key  (Amit Khandekar <amitdkhan.pg@gmail.com>)
List pgsql-hackers

On Mon, Jun 11, 2018 at 10:19 AM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
On 8 June 2018 at 16:44, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> On Fri, Jun 8, 2018 at 2:23 PM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>
>> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar <amitdkhan.pg@gmail.com>
>> wrote:
>>>
>>> Attached is a rebased patch version. Also included it in the upcoming
>>> commitfest :
>>> https://commitfest.postgresql.org/18/1660/
>>>
>>> In the rebased version, the new test cases are added in the existing
>>> isolation/specs/partition-key-update-1.spec test.
>>
>>
>> /*
>> +  * If this is part of an UPDATE of partition-key, the
>> +  * epq tuple will contain the changes from this
>> +  * transaction over and above the updates done by the
>> +  * other transaction. The caller should now use this
>> +  * tuple as its NEW tuple, rather than the earlier NEW
>> +  * tuple.
>> +  */
>> + if (epqslot)
>> + {
>> + *epqslot = my_epqslot;
>> + return NULL;
>> + }
>>
>> I think we need simmilar fix if there are BR Delete trigger and the
>> ExecDelete is blocked on heap_lock_tuple because the concurrent transaction
>> is updating the same row.  Because in such case it would have already got
>> the final tuple so the hep_delete will return MayBeUpdated.
>
>
> Amit Kapila had pointed (offlist) that you are already trying to fix this
> issue.

Yes, his comment about triggers made me realize the trigger issue
which you mentioned, about which I also explained in earlier reply.

> I have one more question,  Suppose the first time we come for
> ExecDelete and fire the BR delete trigger and then realize that we need to
> retry because of the concurrent update.  Now, during retry, we realize that
> because of the latest value we don't need to route the tuple to the
> different partition so now we will call ExecUpdate and may fire the BRUpdate
> triggers as well?

No, we don't fire the BR update trigger again. We go to lreplace
label, and BR trigger code is above that label. The reason we don't
need to run the trigger check again is because if we had already fired
it before, the row must have been locked, so concurrency scenario
won't arise in the first place.
 
Ok, that make sense. 

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: [HACKERS] Transactions involving multiple postgres foreignservers, take 2
Next
From: Michael Paquier
Date:
Subject: Re: Portability concerns over pq_sendbyte?