Simon Riggs wrote:
> On Thu, 2009-06-25 at 13:25 -0400, Tom Lane wrote:
>> I looked through the callers of smgrdounlink(), and found that
>> FinishPreparedTransaction passes isRedo = true. I'm not sure at the
>> moment what the reasoning behind that was, but it does seem to mean that
>> checking InArchiveRecovery instead of isRedo may not be quite right.
>
> I think that's because FinishPreparedTransaction() implicitly assumes
> that if a file still exists we are in recovery. I can't comment on
> whether that is necessarily true for some reason, but it doesn't sound
> like it is a safe assumption. I would guess it's using isRedo as an
> implicit override rather than using the correct meaning of the variable.
It looks like a copy-pasto. In 8.3 it used to be:
> for (i = 0; i < hdr->ncommitrels; i++)
> smgrdounlink(smgropen(commitrels[i]), false, false);
and now it's:
> for (fork = 0; fork <= MAX_FORKNUM; fork++)
> {
> if (smgrexists(srel, fork))
> {
> XLogDropRelation(delrels[i], fork);
> smgrdounlink(srel, fork, false, true);
> }
> }
That clearly assumes that we're in recovery, hence the XLogDropRelation
call, but FinishPreparedTransaction is never called in recovery.
I don't think this is related to the recovery bug we have at hand. Still
needs to be fixed, though it's not that urgent. AFAICS it causes no
other ill effect except that we don't complain if the file doesn't
exist, and we don't leave it lingering like we do for other files to
avoid the OID wraparound issue. I'll fix this as a separate patch.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com