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

On Mon, Jan 23, 2017 at 1:32 AM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> Rebased patch attached. I've split the clog changes out from
> txid_status() its self.

I think it's fairly surprising that TruncateCLOG() has responsibility
for writing the xlog record that protects advancing
ShmemVariableCache->oldestXid, but not the responsibility for actually
advancing that value. In other words, I think the AdvanceOldestXid()
call in vac_truncate_clog() should be moved into TruncateClog().
(Similarly, one wonders why AdvanceOldestCommitTsXid() isn't the
responsibility of TruncateCommitTs().)

I think it is not correct to advance oldestXid but not oldestXidDB.
Otherwise, GetNewTransactionId() might complain about the wrong
database.

The way that SetTransactionIdLimit() now works looks a bit dangerous.
xidWrapLimit, xidStopLimit, and xidWarnLimit are computed based on the
passed-in oldestXid value and written straight into shared memory.
But the shared memory copy of oldestXid could have a different value.
I'm not sure if that breaks anything, but it certainly weakens any
confidence callers might have had that all those values are consistent
with each other.

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