Hi, I see you've committed this, cool. Sorry for not getting back to the topic earlier..
On 2014-03-13 22:44:03 +0200, Heikki Linnakangas wrote: > On 03/12/2014 09:29 PM, Andres Freund wrote: > >On 2014-03-07 17:54:32 +0200, Heikki Linnakangas wrote: > >>So there are some unexplained differences there, but based on these results, > >>I'm still OK with committing the patch. > > > >So, I am looking at this right now. > > > >I think there are some minor things I'd like to see addressed: > > > >1) I think there needs to be a good sized comment explaining why > > WaitXLogInsertionsToFinish() isn't racy due to the unlocked read at > > the beginning of LWLockWait(). > > There's a comment inside LWLockWait(). I think that's the right place for > it; it's LWLockWait() that's cheating by not acquiring the spinlock before > reading lock->exclusive. I don't find that argument convincing. After all it's only correct because the API user does things in a particular way. So there should be comment at the callsite to make sure that's not changed. > >3) I am the wrong one to complain, I know, but the comments above struct > > WALInsertLock are pretty hard to read from th sentence structure. > > Hmm, ok. I reworded that, I hope it's more clear now. Yes, it is. The committed version doesn't compile with LWLOCK_STATS... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
