Re: Concurrency bug in UPDATE of partition-key

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.