Re: Re: BUG #12990: Missing pg_multixact/members files (appears to have wrapped, then truncated)

Robert Haas wrote:

> It seems to me that the most obvious places where
> DetermineSafeOldestOffset() should be called are (1) at startup or
> after recovery, to initialize the value; and (2) each time we truncate
> the SLRU, to update the value. Other than that, this doesn't change.
> The startup calls are there, in apparently reasonable places, but it's
> not obvious to me how this gets called in the TruncateMultiXact path.
> Instead it seems to get set via the SetMultiXactIdLimit path. Maybe
> that's OK, but it would seem to imply that we're OK with overwriting
> old members information if that information was slated for truncation
> at the next checkpoint anyway, which seems scary.

I considered this question in the previous commit: is it okay to
overwrite a file that is no longer used (per the limits set by vacuum)
but not yet removed (by checkpoint)? It seems to me that there is no
data-loss issue with doing that -- which is why the advance-oldest code
is called during vacuum and not during checkpoint.

> Wouldn't it be better to only advance the threshold once the
> checkpoint completes and the truncation is done?

*shrug*

There's also a question about this function's "else" branch:

void
MultiXactAdvanceOldest(MultiXactId oldestMulti, Oid oldestMultiDB)
{
if (MultiXactIdPrecedes(MultiXactState->oldestMultiXactId, oldestMulti))
SetMultiXactIdLimit(oldestMulti, oldestMultiDB);
else
DetermineSafeOldestOffset(oldestMulti);
}

The reason the "else" is there is that I chickened out about not calling
DetermineSafeOldestOffset() (which SetMultiXactIdLimit calls) when the
oldestMulti does not advance; what if in the previous run we failed to
get to this point for whatever reason? The calls to SetMultiXactIdLimit
are seldom enough that it seemed better to be protected against previous
runs not finishing. Then again, I might be worrying over nothing and
this is just dead code.

Also, if we have extra calls in other places after Thomas' patch,
perhaps this one is not necessary.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services