On 25 January 2017 at 13:44, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 24 January 2017 at 23:49, Robert Haas <robertmhaas@gmail.com> wrote:
>
>> 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().)
>
> Agreed, and done in attached.
>
> I haven't done the same for commit_ts but will do so on the thread for
> the commit_ts race fix if we go ahead with this change here.
>
>> I think it is not correct to advance oldestXid but not oldestXidDB.
>> Otherwise, GetNewTransactionId() might complain about the wrong
>> database.
>
> That's a clear oversight. Fixed in attached.
New attached also records it in xlog and applies it to the standby.
If we want to save the 4 bytes per xmin advance (probably not worth
caring) we can instead skip setting it on the standby, in which case
it'll be potentially wrong until the next checkpoint. I'd rather make
sure it stays correct.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers