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