On Tue, Mar 1, 2011 at 3:21 AM, Simon Riggs <si...@2ndquadrant.com> wrote: > On Tue, 2011-03-01 at 15:51 +0900, Fujii Masao wrote: >> Thanks for update of the patch! >> >> On Tue, Mar 1, 2011 at 3:40 AM, Simon Riggs <si...@2ndquadrant.com> wrote: >> >> SyncRepRemoveFromQueue seems not to be as short-term as we can >> >> use the spinlock. Instead, LW lock should be used there. >> >> You seem to have forgotten to fix the above-mentioned issue. > > Not forgotten. > >> A spinlock can be used only for very short-term operation like >> read/write of some shared-variables. The operation on the queue >> is not short, so should be protected by LWLock, I think. > > There's no need to sleep while holding locks and the operations are very > short in most cases. The code around it isn't trivial, but that's no > reason to use LWlocks. > > LWlocks are just spinlocks plus sem sleeps, so I don't see the need for > that in the current code. Other views welcome.
An LWLock is a lot safer, in general, than a spinlock. A spinlock mustn't do anything that could emit an error or abort (among other things). I doubt that the performance cost of using an LWLock rather than a spin lock here is enough to matter, and the spin lock seems more likely to result in hard-to-find bugs. -- 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
