On Tue, May 24, 2011 at 08:53:11AM -0400, Robert Haas wrote: > On Tue, May 24, 2011 at 5:07 AM, Noah Misch <n...@leadboat.com> wrote: > > This drops the part about only transferring fast-path entries once when a > > strong_lock_counts cell transitions from zero to one. > > Right: that's because I don't think that's what we want to do. I > don't think we want to transfer all per-backend locks to the shared > hash table as soon as anyone attempts to acquire a strong lock; > instead, I think we want to transfer only those fast-path locks which > have the same locktag as the strong lock someone is attempting to > acquire. If we do that, then it doesn't matter whether the > strong_lock_counts[] cell is transitioning from 0 to 1 or from 6 to 7: > we still have to check for strong locks with that particular locktag.
Oh, I see. I was envisioning that you'd transfer all locks associated with the strong_lock_counts cell; that is, all the locks that would now go directly to the global lock table when requested going forward. Transferring only exact matches seems fine too, and then I agree with your other conclusions. > > Granted, that itself > > requires some yet-undiscussed locking. ?For that matter, we can't have > > multiple strong lockers completing transfers on the same cell in parallel. > > Perhaps add a FastPathTransferLock, or an array of per-cell locks, that each > > strong locker holds for that entire "if" body and while decrementing the > > strong_lock_counts cell at lock release. > > I was imagining that the per-backend LWLock would protect the list of > fast-path locks. So to transfer locks, you would acquire the > per-backend LWLock for the backend which has the lock, and then the > lock manager partition LWLock, and then perform the transfer. I see later in your description that the transferer will delete each fast-path lock after transferring it. Given that, this does sound adequate. > > As far as the level of detail of this pseudocode goes, there's no need to > > hold > > the per-backend LWLock while transferring the fast-path entries. ?You just > > need to hold it sometime between bumping strong_lock_counts and transferring > > the backend's locks. ?This ensures that, for example, the backend is not > > sleeping in the middle of a fast-path lock acquisition for the whole > > duration > > of this code. > > See above; I'm lost. It wasn't a particularly useful point. > > To validate the locking at this level of detail, I think we need to sketch > > the > > unlock protocol, too. ?On each strong lock release, we'll decrement the > > strong_lock_counts cell. ?No particular interlock with fast-path lockers > > should be needed; a stray AccessShareLock needlessly making it into the > > global > > lock table is no problem. ?As mentioned above, we _will_ need an interlock > > with lock transfer operations. ?How will transferred fast-path locks get > > removed from the global lock table? ?Presumably, the original fast-path > > locker > > should do so at transaction end; anything else would contort the life cycle. > > Then add a way for the backend to know which locks had been transferred as > > well as an interlock against concurrent transfer operations. ?Maybe that's > > all. > > I'm thinking that the backend can note, in its local-lock table, > whether it originally acquired a lock via the fast-path or not. Any > lock not originally acquired via the fast-path will be released just > as now. For any lock that WAS originally acquired via the fast-path, > we'll take our own per-backend lwlock, which protects the fast-path > queue, and scan the fast-path queue for a matching entry. If none is > found, then we know the lock was transferred, so release the > per-backend lwlock and do it the regular way (take lock manager > partition lock, etc.). Sounds good. > > To put it another way: the current system is fair; the chance of hitting > > lock > > exhaustion is independent of lock level. ?The new system would be unfair; > > lock > > exhaustion is much more likely to appear for a > ShareUpdateExclusiveLock > > acquisition, through no fault of that transaction. ?I agree this isn't > > ideal, > > but it doesn't look to me like an unacceptable weakness. ?Making lock slots > > first-come, first-served is inherently unfair; we're not at all set up to > > justly arbitrate between mutually-hostile lockers competing for slots. ?The > > overall situation will get better, not worse, for the admin who wishes to > > defend against hostile unprivileged users attempting a lock table DOS. > > Well, it's certainly true that the proposed system is far less likely > to bomb out trying to acquire an AccessShareLock than what we have > today, since in the common case the AccessShareLock doesn't use up any > shared resources. And that should make a lot of people happy. But as > to the bad scenario, one needn't presume that the lockers are hostile > - it may just be that the system is running on the edge of a full lock > table. In the worst case, someone wanting a strong lock on a table > may end up transferring a hundred or more locks (up to one per > backend) into that table. Even if they all fit and the strong locker > gets his lock, it may now happen that the space is just about > exhausted and other transactions start rolling back, apparently at > random. > > Or, even more pathologically, one backend grabs a strong lock on table > X, but it just so happens that there is another table Y in the same > lock partition which is highly trafficked but, as all of the locks > involved are weak, uncontended. Now that strong_lock_counts[] is > non-zero for that partition, all those locks start going into the main > lock manager table. Performance will drop, which isn't great but we > can live with it. But maybe all the locks don't fit. Now we have a > situation in which, due to one backend acquiring a strong lock on > table A, a bunch of other backends are unable to obtain weak locks on > table B, and transactions start rolling back all over the place. > > Now maybe you can argue that this scenario is sufficiently unlikely in > practice that we shouldn't worry about it, but if it does happen the > DBA will be incredibly confused, because an apparently innocuous > operation on one table will have resulted in rollbacks acquiring locks > on an apparently unrelated table. Maybe you want to argue that's > sufficiently rare that we shouldn't worry about it, but the > unpleasantness factor seems pretty high to me. Let's see if I understand the risk better now: the new system will handle lock load better, but when it does hit a limit, understanding why that happened will be more difficult. Good point. No silver-bullet ideas come to mind for avoiding that. A lock transfer operation could abort if it sees itself burning all the shared memory, but I'd be pessimistic about identifying a concrete heuristic not subject to its own confusing corner cases. Overall, the original outcome doesn't sound especially confusing to me. YMMV. Will the pg_locks view scan fast-path lock tables? If not, we probably need another view that does. We can then encourage administrators to monitor for fast-path lock counts that get high relative to shared memory capacity. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers