Thread: LISTEN and "tuple concurrently updated"

Hi all,

A user on IRC came across the following "tuple concurrently updated" error
when using LISTEN/NOTIFY intensively. The user managed to isolate the
problem and SQL generating the problem. A few sessions are required to
generate the error.

T1:
---
begin;
listen test;
commit;

T2:
---
begin;
notify test;
commit;

T1:
---
begin;
-- got notify
unlisten test;


T3:
---
begin;
notify test;
commit;
-- blocks

T1:
---
commit;

T3 then receives:

WARNING:  AbortTransaction and not in in-progress state
ERROR:  tuple concurrently updated

A brief look into this:

heap_update() in T3 (called by AtCommit_Notify()) calls
HeapTupleSatisfiesUpdate(). This returns HeapTupleBeingUpdated. Once we
issue COMMIT; in T1 updates pg_listen and the tuple T3 is trying to
update no longer exists.

I've attached a patch which solves this problem. Basically, T1 will now
just hold AccessExclusiveLock on pg_listen for the rest of the
transaction. I've also modified AsyncExistsPendingNotify() to step through
pg_listen which allows T3's NOTIFY to block until T1 commits. This is not
really necessary due to the semantics of LISTEN/NOTIFY -- it is not an
error if a record exists in pg_listen already.

Thanks,

Gavin

Gavin Sherry <swm@linuxworld.com.au> writes:
> ERROR:  tuple concurrently updated

> A brief look into this:

> heap_update() in T3 (called by AtCommit_Notify()) calls
> HeapTupleSatisfiesUpdate(). This returns HeapTupleBeingUpdated. Once we
> issue COMMIT; in T1 updates pg_listen and the tuple T3 is trying to
> update no longer exists.

Ugh.

> I've attached a patch which solves this problem. Basically, T1 will now
> just hold AccessExclusiveLock on pg_listen for the rest of the
> transaction.

That seems quite unworkable --- it creates the potential for deadlock,
and in any case the exclusive lock could be held for an unreasonably
long time.

> I've also modified AsyncExistsPendingNotify() to step through
> pg_listen which allows T3's NOTIFY to block until T1 commits. This is not
> really necessary due to the semantics of LISTEN/NOTIFY -- it is not an
> error if a record exists in pg_listen already.

This appears to turn AtCommit_Notify into an O(N^2) operation, which
doesn't strike me as a pleasant answer at all.  I think it also breaks
the semantics of the other caller, Async_Notify.

What we probably need to do instead of this is not use
simple_heap_update in AtCommit_Notify; instead we have to use
heap_update directly and cope with concurrent-update situations.
The simple_heap_delete calls may need work too, now that I think
about it ...
        regards, tom lane

Gavin Sherry <swm@linuxworld.com.au> writes:
> A user on IRC came across the following "tuple concurrently updated" error
> when using LISTEN/NOTIFY intensively.

I've applied a fix for this to CVS tip.

Thinking about the implications of rolling back UNLISTEN, it occurs to
me that there is another possible solution, which is to queue up LISTEN
and UNLISTEN commands locally in the backend and not apply them to
pg_listener until commit.  Doing it this way, it'd be okay to hold the
pg_listener lock until commit, which would eliminate the problem of
uncommitted unlistens being visible to notifiers.  However it would
require adding a fair amount more code to async.c.

I think that whenever we get around to rewriting LISTEN/NOTIFY to use
shared memory messages instead of a table, it will be necessary to apply
listen/unlisten commands that way (hold them until commit) to preserve
transactional semantics.  But for now, I'm not going to do the extra
work.
        regards, tom lane

On Mon, 15 Sep 2003, Tom Lane wrote:

> Gavin Sherry <swm@linuxworld.com.au> writes:
> > A user on IRC came across the following "tuple concurrently updated" error
> > when using LISTEN/NOTIFY intensively.
>
> I've applied a fix for this to CVS tip.

Great.

> I think that whenever we get around to rewriting LISTEN/NOTIFY to use
> shared memory messages instead of a table, it will be necessary to apply
> listen/unlisten commands that way (hold them until commit) to preserve
> transactional semantics.  But for now, I'm not going to do the extra
> work.

I wasn't thinking about the deadlock/performance problems when I sent in
that patch. It was more a proof of my theory. I was certainly thinking
about the various discussions about reworking LISTEN/NOTIFY into
shared memory when looking at the code, but as you say, its not a job for
right now :-).

Thanks,

Gavin