Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups

On 20 January 2017 at 21:40, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:

> One option would be to add another limit Xid which advances before the
> truncation but which is not used for other decisions other than limiting
> what can users consult.

This could be useful for other things, but it's probably heavier than needed.

What I've done in the latest revision of the txid_status() patch is
simply to advance OldestXid _before_ truncating the clog. The rest of
the xid info is advanced after. Currently this is incorporated into
the txid_status patch, but can be separated if desired.

Relevant commit message portion:

There was previously no way to look up an arbitrary xid without
running the risk of having clog truncated out from under you. This
hasn't been a problem because anything looking up xids in clog knows
they're protected by datminxid, but that's not the case for arbitrary
user-supplied XIDs. clog was truncated before we advance oldestXid so
taking XidGenLock was insufficient. There's no way to look up a
SLRU with soft-failure. To address this, increase oldestXid under XidGenLock
before we trunate clog rather than after, so concurrent access is safe.

Note that while oldestXid is advanced before clog truncation, the xid
limits are advanced _after_ it. If we advanced the xid limits before
truncation too, we'd theoretically run the risk of allocating an xid
from the clog section we're about to truncate, which would be no fun.
(In practice it can't really happen since we only use 1/2 the
available space at a time).

Moving the lower bound up, truncating, and moving the upper bound up
is the way to go IMO.

> Another option is not to implement direct reads
> from the clog.

I think there's a pretty decent argument for having clog lookups;
txid_status(...) serves as a useful halfway position between accepting
indeterminate commit status on connection loss and using full 2PC.

> Yet another option is that before we add such interface
> somebody produces proof that the problem does not, in fact, exist.

It does exist.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services