Re: [PATCH] Transaction traceability - txid_status(bigint)

On Wed, Sep 21, 2016 at 3:40 AM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> The only non-horrible one of those is IMO to just let the caller see
> an error if they lose the race. It's a function that's intended for
> use when you're dealing with indeterminate transaction state after a
> server or application error anyway, so it's part of an error path
> already. So I say we just document the behaviour.

I am not keen on that idea. The errors we're likely to be exposing
are going to look like low-level internal failures, which might scare
some DBA. Even if they don't, I think it's playing with fire. The
system is designed on the assumption that nobody will try to look up
an XID that's too old, and if we start to violate that assumption I
think we're undermining the design integrity of the system in a way
we'll likely come to regret. To put that more plainly, when the code
is written with the assumption that X will never happen, it's usually
a bad idea to casually add code that does X.

> Because slru.c
> doesn't release its LWLock on error we also need to ensure
> txid_status(...) is also only called from a toplevel xact so the user
> doesn't attempt to wrap it in plpgsql BEGIN ... EXCEPTION block and it
> causes the xact to abort.

I think this is muddled, because an error aborts the transaction, and
AbortTransaction() and AbortSubTransaction() start with
LWLockReleaseAll().

It might not be too hard to add a second copy of oldestXid in shared
memory that is updated before truncation rather than afterward... but
yeah, like you, I'm not finding this nearly as straightforward as
might have been hoped.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company