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

On Wed, 4 Jan 2023 at 12:52, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:
>
> Hi, Vignesh!
>
> On Wed, 4 Jan 2023 at 12:41, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Fri, 1 Jul 2022 at 16:49, Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> > >
> > > Hackers,
> > >
> > > When working in the read committed transaction isolation mode
> > > (default), we have the following sequence of actions when
> > > tuple_update() or tuple_delete() find concurrently updated tuple.
> > >
> > > 1. tuple_update()/tuple_delete() returns TM_Updated
> > > 2. tuple_lock()
> > > 3. Re-evaluate plan qual (recheck if we still need to update/delete
> > > and calculate the new tuple for update)
> > > 4. tuple_update()/tuple_delete() (this time should be successful,
> > > since we've previously locked the tuple).
> > >
> > > I wonder if we should merge steps 1 and 2. We could save some efforts
> > > already done during tuple_update()/tuple_delete() for locking the
> > > tuple. In heap table access method, we've to start tuple_lock() with
> > > the first tuple in the chain, but tuple_update()/tuple_delete()
> > > already visited it. For undo-based table access methods,
> > > tuple_update()/tuple_delete() should start from the last version, why
> > > don't place the tuple lock immediately once a concurrent update is
> > > detected. I think this patch should have some performance benefits on
> > > high concurrency.
> > >
> > > Also, the patch simplifies code in nodeModifyTable.c getting rid of
> > > the nested case. I also get rid of extra
> > > table_tuple_fetch_row_version() in ExecUpdate. Why re-fetch the old
> > > tuple, when it should be exactly the same tuple we've just locked.
> > >
> > > I'm going to check the performance impact. Thoughts and feedback are welcome.
> >
> > The patch does not apply on top of HEAD as in [1], please post a rebased patch:
> > === Applying patches on top of PostgreSQL commit ID
> > eb5ad4ff05fd382ac98cab60b82f7fd6ce4cfeb8 ===
> > === applying patch
> > ./0001-Lock-updated-tuples-in-tuple_update-and-tuple_del-v1.patch
> > patching file src/backend/executor/nodeModifyTable.c
> > ...
> > Hunk #3 FAILED at 1376.
> > ...
> > 1 out of 15 hunks FAILED -- saving rejects to file
> > src/backend/executor/nodeModifyTable.c.rej
> >
> > [1] - http://cfbot.cputube.org/patch_41_4099.log
>
> The rebased patch is attached. It's just a change in formatting, no
> changes in code though.

One more update of a patchset to avoid compiler warnings.

Regards,
Pavel Borisov