Re: XMin Hot Standby Feedback patch

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