On Tue, Feb 15, 2011 at 4:15 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Tue, 2011-02-15 at 12:15 -0500, Robert Haas wrote:
>> Looks pretty good to me, though I haven't tested it. I like some of
>> the safety valves you put in there, but I don't understand this part
>
> Reworked logic covering all feedback, plus tests, plus docs.
>
> Last comments before commit please.
What happens if someone has hot_standby_feedback on and then turns it
off? I think in XLogWalRcvSendReply() you need
if (hot_standby_feedback)
{stuff
}
else
{ reply_message.xmin = InvaidXID; reply_message.epoch = 0; /* or something */
}
Also this part looks kludgy to me:
+ GetNextXidAndEpoch(&nextXid, &nextEpoch);
+ if (nextXid < reply_message.xmin)
+ nextEpoch--;
How about introducing a GetOldestXminAndEpoch function instead?
Would it make sense to avoid grabbing the ProcArrayLock except when we
truly need to update MyProc->xmin? ProcessStandbyReplyMessage gets
called a lot...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
