Re: Bug in autovacuum.c?

Excerpts from Robert Haas's message of jue mar 31 13:58:41 -0300 2011:
> On Thu, Mar 31, 2011 at 12:17 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > Looking over the autovacuum.c code, I see:
> >
> >    /*
> >     * Determine the oldest datfrozenxid/relfrozenxid that we will allow to
> >     * pass without forcing a vacuum.  (This limit can be tightened for
> >     * particular tables, but not loosened.)
> >     */
> >    recentXid = ReadNewTransactionId();
> >    xidForceLimit = recentXid - autovacuum_freeze_max_age;
> >    /* ensure it's a "normal" XID, else TransactionIdPrecedes misbehaves */
> >    if (xidForceLimit < FirstNormalTransactionId)
> >        xidForceLimit -= FirstNormalTransactionId;
> >
> > This last line doesn't look right to me;  should it be:
> >
> >        xidForceLimit = FirstNormalTransactionId;
>
> That would probably work, but the existing coding actually makes more
> sense. It's essentially trying to scan backwards by
> autovacuum_freeze_max_age XIDs through the circular XID space. But
> the XID space isn't actually circular, because there are 3 special
> values. So if we land on one of those values we want to skip backward
> by 3. Here FirstNormalTransactionId doesn't represent itself, but
> rather the number of special XIDs that exist.

Yeah, I think this change would have the effect of moving the freeze
limit by one (or two?) counts. Given the moving nature of values
returned by ReadNewTransactionId this would probably have no practical
effect. Still, the code as is seems more natural to me (Tom wrote this
bit IIRC, not me).

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support