On Fri, Mar 18, 2011 at 1:15 PM, Simon Riggs <si...@2ndquadrant.com> wrote: > On Thu, 2011-03-17 at 09:33 -0400, Robert Haas wrote: >> Thanks for the review! > > Lets have a look here... > > You've added a test inside the lock to see if there is a standby, which > I took out for performance reasons. Maybe there's another way, I know > that code is fiddly. > > You've also added back in the lock acquisition at wakeup with very > little justification, which was a major performance hit. > > Together that's about a >20% hit in performance in Yeb's tests. I think > you should spend a little time thinking how to retune that.
Ouch. Do you have a link that describes his testing methodology? I will look at it. > I see handling added for ProcDiePending and QueryCancelPending directly > into syncrep.c without any comments in postgres.c to indicate that you > bypass ProcessInterrupts() in some cases. That looks pretty hokey to me. I can add some comments. Unfortunately, it's not feasible to call ProcessInterrupts() directly from this point in the code - it causes a database panic. > SyncRepUpdateSyncStandbysDefined() is added into walwriter, which means > waiters won't be released if we do a sighup during a fast shutdown, > since the walwriter gets killed as soon as that starts. I'm thinking > bgwriter should handle that now. Hmm. I was thinking that doing it in WAL writer would make it more responsive, but since this is a fairly unlikely scenario, it's probably not worth complicating the shutdown sequence to do it the way I did. I'll move it to bgwriter. -- 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