On Sat, Mar 11, 2017 at 1:32 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 11 March 2017 at 05:09, Robert Haas <robertmhaas@gmail.com> wrote:
>> On the other
>> hand, there really are two separate notions of the "oldest" XID.
>> There's the oldest XID that we can safely look up, and then there's
>> the oldest XID that we can't reuse. These two are the same when no
>> truncation is in progress, but when a truncation is in progress then
>> they're different: the oldest XID that's safe to look up is the first
>> one after whatever we're truncating away, but the oldest XID that we
>> can't reuse is the newest one preceding the stuff that we're busy
>> truncating.
>
> Right.
>
> My view here is that the oldest xid we cannot reuse is already guarded
> by xidWrapLimit, which we advance after clog truncation. Whether as
> this advances at the same time as or after we advance oldestXid and
> truncate clog doesn't actually matter, we must just ensure that it
> never advances _before_.
>
> So tracking a second copy of oldestXid whose only purpose is to
> recalculate xidWrapLimit serves no real purpose.
Hmm, so what this patch is doing changed quite a bit between January
23rd and January 25th. In the January 23rd version, oldestXid and
oldestXidDB are changed to track the oldest XID that we can safely
look up, and the remaining related fields are still relative to the
oldest XID that can be reused. That seems, as I said before, scary.
But in the January 25th version, *all* of the related fields have been
changed to track the oldest XID that we can safely look up, because
SetTransactionIdLimit() now uses the values set by AdvanceOldestXid()
to compute all of the other values, which seems flat-out incorrect.
AdvanceOldestXid() is called to advance that limit before clog
truncation happens, and if somebody then calls SetTransactionIdLimit()
before clog truncation is complete, we'll advanced those derived
limits prematurely.
For example, suppose vacuum #1 comes along, advances the limits,
truncates clog, and then gets descheduled. Now vacuum #2 comes along,
advances the limits further, and then gets descheduled. Now vacuum #1
wakes up and calls SetTransactionIdLimit() and prematurely advances
xidWrapLimit. Oops.
The way you put it is that having a second copy of oldestXid whose
only purpose is to recompute xidWrapLimit is pointless, but the way
I'd say is that you're trying to make one variable do two jobs.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
