On Wed, Sep 11, 2013 at 8:47 PM, Peter Geoghegan <p...@heroku.com> wrote: > In practice the vast majority of insertions don't involve TOASTing. > That's not an excuse for allowing the worst case to be really bad in > terms of its impact on query response time, but it may well justify > having whatever ameliorating measures we take result in bloat. It's at > least the kind of bloat we're more or less used to dealing with, and > have already invested a lot in controlling. Plus bloat-wise it can't > be any worse than just inserting the tuple and having the transaction > abort on a duplicate, since that already happens after toasting has > done its work with regular insertion.
Andres is being very polite here, but the reality is that this approach has zero chance of being accepted. You can't hold buffer locks for a long period of time across complex operations. Full stop. It's a violation of the rules that are clearly documented in src/backend/storage/buffer/README, which have been in place for a very long time, and this patch is nowhere near important enough to warrant a revision of those rules. We are not going to risk breaking every bit of code anywhere in the backend or in third-party code that takes a buffer lock. You are never going to convince me, or Tom, that the benefit of doing that is worth the risk; in fact, I have a hard time believing that you'll find ANY committer who thinks this approach is worth considering. Even if you get the code to run without apparent deadlocks, that doesn't mean there aren't any; it just means that you haven't found them all yet. And even if you managed to squash every such hazard that exists today, so what? Fundamentally, locking protocols that don't include deadlock detection don't scale. You can use such locks in limited contexts where proofs of correctness are straightforward, but trying to stretch them beyond that point results not only in bugs, but also in bad performance and unmaintainable code. With a much more complex locking regimen, even if your code is absolutely bug-free, you've put a burden on the next guy who wants to change anything; how will he avoid breaking things? Our buffer locking regimen suffers from painful complexity and serious maintenance difficulties as is. Moreover, we've already got performance and scalability problems that are attributable to every backend in the system piling up waiting on a single lwlock, or a group of simultaneously-held lwlocks. Dramatically broadening the scope of where lwlocks are used and for how long they're held is going to make that a whole lot worse. What's worse, the problems will be subtle, restricted to the people using this feature, and very difficult to measure on production systems, and I have no confidence they'd ever get fixed. A further problem is that a backend which holds even one lwlock can't be interrupted. We've had this argument before and it seems that you don't think that non-interruptibility is a problem, but it project policy to allow for timely interrupts in all parts of the backend and we're not going to change that policy for this patch. Heavyweight locks are heavy weight precisely because they provide services - like deadlock detection, satisfactory interrupt handling, and, also importantly, FIFO queuing behavior - that are *important* for locks that are held over an extended period of time. We're not going to go put those services into the lightweight lock mechanism because then it would no longer be light weight, and we're not going to ignore the importance of them, either. -- 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
