Re: Concurrency bug in UPDATE of partition-key

On Mon, Jun 11, 2018 at 10:19 AM, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>
wrote:

> On 8 June 2018 at 16:44, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > On Fri, Jun 8, 2018 at 2:23 PM, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
> wrote:
> >>
> >> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)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