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

On 23 December 2016 at 01:26, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> This patch contains unresolved merge conflicts.

Ah, it conflicts with fe0a0b59, the PostmasterRandom changes. I
thought I'd rebased more recently than that.

> Maybe SetPendingTransactionIdLimit could happen in TruncateCLOG rather than
> the caller.

I've spent a while looking at how committs and multixact handle the
race hazard of the gap between clog truncation and shmem state change
- and the related race on standby, where the hazard is the gap between
replay of CLOG_TRUNCATE records and replay of the next checkpoint
containing updated limits.

pg_xact_commit_timestamp() and the underlying
TransactionIdGetCommitTsData are supposed to accept arbitrary xids,
like txid_status(), so they should handle this.

However, as far as I can tell, committs has the same issues presented
by txid_status(). We only update ShmemVariableCache->oldestCommitTsXid
_after_ we truncate the committs SLRU, no lock is held over the
duration, and no other lock-protected var is set before truncation. We
don't record the new limit values in COMMIT_TS_TRUNCATE records so
they only becomes known to the standby at replay of the next
checkpoint.

multixact instead runs a single critical section to write xlog, set
shmem state, and _then_ truncate the SLRUs. It's more complex than
clog or commit_ts since it has two inter-related SLRUs, though.

So: I think we should be setting ShmemVariableCache->oldestXid (under
XidGenLock) from TruncateCLOG, after we write xlog but _before_
truncation. Then the rest of SetTransactionIdLimit(...) proceeds as
before. I don't _think_ we need the critical section since we only
have the one SLRU to worry about. We should also add oldestXid to
CLOG_TRUNCATE xlog records so it is up to date on standbys.

It's a little annoying to take XidGenLock twice - once for oldestXid,
once for the other xid limits - but we can just delay advancing
oldestXid entirely until we've got a clog page to truncate away, in
which case there's a big enough cost that it doesn't matter. We have
to wait to advance the other limits until after we truncate clog to
prevent new (wrapped) xids trying to set values in clog for the
before-wrap xids we're about to truncate away.

On the upside, the need for pendingOldestXid, which always felt hacky,
goes away.

While we're at it, lets do the same thing for commit_ts. Advance its
limit before truncation, not after, and record that limit in its xlog
records for redo. There's no need for any special dancing around
there.

> Instead of using an LWLock, how about a spin lock?

I wrote an explanation of how that wouldn't work, but then I found the
problem with standby, so it no longer matters.

I'll have to follow up with a patch soon, as it's Toddler o'Clock.

(It's interesting that most of what I'm doing at the moment is
wrestling with resource retention limits and how they're often quite
implicit. All the catalog_xmin stuff for logical decoding on standby
has parallels to this, for example, though not enough to create useful
overlap of functionality.)

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