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