On Mon, Oct 31, 2016 at 5:48 PM, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > Honestly, I have no idea what to think about this ...
I think a lot of the details here depend on OS scheduler behavior. For example, here's one of the first scalability graphs I ever did: http://rhaas.blogspot.com/2011/09/scalability-in-graphical-form-analyzed.html It's a nice advertisement for fast-path locking, but look at the funny shape of the red and green lines between 1 and 32 cores. The curve is oddly bowl-shaped. As the post discusses, we actually dip WAY under linear scalability in the 8-20 core range and then shoot up like a rocket afterwards so that at 32 cores we actually achieve super-linear scalability. You can't blame this on anything except Linux. Someone shared BSD graphs (I forget which flavor) with me privately and they don't exhibit this poor behavior. (They had different poor behaviors instead - performance collapsed at high client counts. That was a long time ago so it's probably fixed now.) This is why I think it's fundamentally wrong to look at this patch and say "well, contention goes down, and in some cases that makes performance go up, but because in other cases it decreases performance or increases variability we shouldn't commit it". If we took that approach, we wouldn't have fast-path locking today, because the early versions of fast-path locking could exhibit *major* regressions precisely because of contention shifting to other locks, specifically SInvalReadLock and msgNumLock. (cf. commit b4fbe392f8ff6ff1a66b488eb7197eef9e1770a4). If we say that because the contention on those other locks can get worse as a result of contention on this lock being reduced, or even worse, if we try to take responsibility for what effect reducing lock contention might have on the operating system scheduler discipline (which will certainly differ from system to system and version to version), we're never going to get anywhere, because there's almost always going to be some way that reducing contention in one place can bite you someplace else. I also believe it's pretty normal for patches that remove lock contention to increase variability. If you run an auto race where every car has a speed governor installed that limits it to 80 kph, there will be much less variability in the finish times than if you remove the governor, but that's a stupid way to run a race. You won't get much innovation around increasing the top speed of the cars under those circumstances, either. Nobody ever bothered optimizing the contention around msgNumLock before fast-path locking happened, because the heavyweight lock manager burdened the system so heavily that you couldn't generate enough contention on it to matter. Similarly, we're not going to get much traction around optimizing the other locks to which contention would shift if we applied this patch unless we apply it. This is not theoretical: EnterpriseDB staff have already done work on trying to optimize WALWriteLock, but it's hard to get a benefit. The more contention other contention we eliminate, the easier it will be to see whether a proposed change to WALWriteLock helps. Of course, we'll also be more at the mercy of operating system scheduler discipline, but that's not all a bad thing either. The Linux kernel guys have been known to run PostgreSQL to see whether proposed changes help or hurt, but they're not going to try those tests after applying patches that we rejected because they expose us to existing Linux shortcomings. I don't want to be perceived as advocating too forcefully for a patch that was, after all, written by a colleague. However, I sincerely believe it's a mistake to say that a patch which reduces lock contention must show a tangible win or at least no loss on every piece of hardware, on every kernel, at every client count with no increase in variability in any configuration. Very few (if any) patches are going to be able to meet that bar, and if we make that the bar, people aren't going to write patches to reduce lock contention in PostgreSQL. For that to be worth doing, you have to be able to get the patch committed in finite time. We've spent an entire release cycle dithering over this patch. Several alternative patches have been written that are not any better (and the people who wrote those patches don't seem especially interested in doing further work on them anyway). There is increasing evidence that the patch is effective at solving the problem it claims to solve, and that any downsides are just the result of poor lock-scaling behavior elsewhere which we could be working on fixing if we weren't still spending time on this. Is that really not good enough? -- 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