On 2013-08-07 09:40:24 -0500, Merlin Moncure wrote: > > I don't think the unlocked increment of nextVictimBuffer is a good idea > > though. nextVictimBuffer jumping over NBuffers under concurrency seems > > like a recipe for disaster to me. At the very, very least it will need a > > good wad of comments explaining what it means and how you're allowed to > > use it. The current way will lead to at least bgwriter accessing a > > nonexistant/out of bounds buffer via StrategySyncStart(). > > Possibly it won't even save that much, it might just increase the > > contention on the buffer header spinlock's cacheline. > > I agree; at least then it's not unambiguously better. if you (in > effect) swap all contention on allocation from a lwlock to a spinlock > it's not clear if you're improving things; it would have to be proven > and I'm trying to keep things simple.
I think converting it to a spinlock actually is a good idea, you just need to expand the scope a bit. > Attached is a scaled down version of the patch that keeps the freelist > lock but still removes the spinlock during the clock sweep. This > still hits the major objectives of reducing the chance of scheduling > out while holding the BufFreelistLock and mitigating the worst case > impact of doing so if it does happen. FWIW, I am not convinced this is the trigger for the problems you're seing. It's a good idea nonetheless though. 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
