On Mon, Jan 23, 2017 at 1:32 AM, Craig Ringer <cr...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
