Re: POC: Lock updated tuples in tuple_update() and tuple_delete()

On Fri, Jan 6, 2023 at 4:46 AM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:

> Hi, Alexander!
>
> On Thu, 5 Jan 2023 at 15:11, Alexander Korotkov <aekorotkov(at)gmail(dot)com>
> wrote:
> >
> > On Wed, Jan 4, 2023 at 5:05 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com>
> wrote:
> > > On Wed, Jan 4, 2023 at 3:43 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>
> wrote:
> > > > One more update of a patchset to avoid compiler warnings.
> > >
> > > Thank you for your help. I'm going to provide the revised version of
> > > patch with comments and commit message in the next couple of days.
> >
> > The revised patch is attached. It contains describing commit message,
> > comments and some minor code improvements.
>
> I've looked through the patch once again. It seems in a nice state to
> be committed.
> I also noticed that in tableam level and NodeModifyTable function
> calls we have a one-to-one correspondence between *lockedSlot и bool
> lockUpdated, but no checks on this in case something changes in the
> code in the future. I'd propose combining these variables to remain
> free from these checks. See v5 of a patch. Tests are successfully
> passed.
> Besides, the new version has only some minor changes in the comments
> and the commit message.
>
> Kind regards,
> Pavel Borisov,
> Supabase.
>

It looks good, and the greater the concurrency the greater the benefit will
be. Just a few minor suggestions regarding comments.

"ExecDeleteAct() have already locked the old tuple for us", change "have"
to "has".

The comments in heapam_tuple_delete() and heapam_tuple_update() might be a
little clearer with something like:

"If the tuple has been concurrently updated, get lock already so that on
retry it will succeed, provided that the caller asked to do this by
providing a lockedSlot."

Also, not too important, but perhaps better clarify in the commit message
that the repeated work is driven by ExecUpdate and ExecDelete and can
happen multiple times depending on the concurrency.

Best Regards,

Mason