[COMMITTERS] pgsql: Fix low-probability loss of NOTIFY messages due to XIDwraparoun - Mailing list pgsql-committers

From Tom Lane
Subject [COMMITTERS] pgsql: Fix low-probability loss of NOTIFY messages due to XIDwraparoun
Date
Msg-id E1e2Lkf-0003x6-9G@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Fix low-probability loss of NOTIFY messages due to XID wraparound.

Up to now async.c has used TransactionIdIsInProgress() to detect whether
a notify message's source transaction is still running.  However, that
function has a quick-exit path that reports that XIDs before RecentXmin
are no longer running.  If a listening backend is doing nothing but
listening, and not running any queries, there is nothing that will advance
its value of RecentXmin.  Once 2 billion transactions elapse, the
RecentXmin check causes active transactions to be reported as not running.
If they aren't committed yet according to CLOG, async.c decides they
aborted and discards their messages.  The timing for that is a bit tight
but it can happen when multiple backends are sending notifies concurrently.
The net symptom therefore is that a sufficiently-long-surviving
listen-only backend starts to miss some fraction of NOTIFY traffic,
but only under heavy load.

The only function that updates RecentXmin is GetSnapshotData().
A brute-force fix would therefore be to take a snapshot before
processing incoming notify messages.  But that would add cycles,
as well as contention for the ProcArrayLock.  We can be smarter:
having taken the snapshot, let's use that to check for running
XIDs, and not call TransactionIdIsInProgress() at all.  In this
way we reduce the number of ProcArrayLock acquisitions from one
per message to one per notify interrupt; that's the same under
light load but should be a benefit under heavy load.  Light testing
says that this change is a wash performance-wise for normal loads.

I looked around for other callers of TransactionIdIsInProgress()
that might be at similar risk, and didn't find any; all of them
are inside transactions that presumably have already taken a
snapshot.

Problem report and diagnosis by Marko Tiikkaja, patch by me.
Back-patch to all supported branches, since it's been like this
since 9.0.

Discussion: https://postgr.es/m/20170926182935.14128.65278@wrigleys.postgresql.org

Branch
------
REL9_4_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/0e84f70752cfda9bb25ace3fd15de176e968b0ee

Modified Files
--------------
src/backend/commands/async.c   | 39 +++++++++++++++++++++++++++++----------
src/backend/utils/time/tqual.c |  9 ++++-----
src/include/utils/tqual.h      |  1 +
3 files changed, 34 insertions(+), 15 deletions(-)


--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [COMMITTERS] pgsql: Add port/strnlen support to libpq and ecpg Makefiles.
Next
From: Robert Haas
Date:
Subject: [COMMITTERS] pgsql: Fix mistakes in comments.