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

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.

Regards,
Pavel Borisov,
Supabase.