Re: Hot standby, recovery procs

Simon Riggs wrote:
> On Tue, 2009-02-24 at 21:59 +0200, Heikki Linnakangas wrote:
>> We only need the lsn atrribute because we when we take the snapshot
>> of 
>> running xids, we don't write it to the WAL immediately, and a new 
>> transaction might begin after that. If we close that gap in the
>> master, 
>> we don't need the lsn in recovery procs.
>>
>> Actually, I think the patch doesn't get that right as it stands:
>>
>> 0. Transactions 1 is running in master
>> 1. Get list of running transactions
>> 2. Transaction 1 commits.
>> 3. List of running xacts is written to WAL
>>
>> When the standby replays the xl_running_xacts record, it will create
>> a 
>> recovery proc and mark the transaction as running again, even though
>> it 
>> has already committed.
> 
> No, because we check whether TransactionIdDidCommit().

Oh, right... But we have the same problem with the subtransactions, 
don't we? This block:

>         /*
>          * If our state information is later for this proc, then 
>          * overwrite it. It's possible for a commit and possibly
>          * a new transaction record to have arrived in WAL in between
>          * us doing GetRunningTransactionData() and grabbing the
>          * WALInsertLock, so we musn't assume we always know best.
>          */
>         if (XLByteLT(proc->lsn, lsn))
>         {
>             TransactionId     *subxip = (TransactionId *) &(xlrec->xrun[xlrec->xcnt]);
> 
>             proc->lsn = lsn;
>             /* proc-> pid stays 0 for Recovery Procs */
> 
>             proc->subxids.nxids = rxact[xid_index].nsubxids;
>             proc->subxids.overflowed = rxact[xid_index].overflowed;
> 
>             memcpy(proc->subxids.xids, subxip, 
>                         rxact[xid_index].nsubxids * sizeof(TransactionId));
> 
>             /* Remove subtransactions from UnobservedXids also */
>             if (unobserved)
>             {
>                 for (index = 0; index < rxact[xid_index].nsubxids; index++)
>                     UnobservedTransactionsRemoveXid(subxip[index + rxact[xid_index].subx_offset], false);
>             }
>         }

overwrites subxids array, and will resurrect any already aborted 
subtransaction.

Isn't XLByteLT(proc->lsn, lsn) always true, because 'lsn' is the lsn of 
the WAL record we're redoing, so there can't be any procs with an LSN 
higher than that?

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com