Re: XMin Hot Standby Feedback patch

On Tue, Feb 15, 2011 at 4:15 PM, Simon Riggs <simon(at)2ndquadrant(dot)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