On Tue, Feb 15, 2011 at 4:15 PM, Simon Riggs <si...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to