Make lock_wait() wait less

Updated patch sets $delay * 10 so that the $delay argument behaves the same.

The result of this is ten select queries per second where there would have been one, but see the performance improvement when combined with #802446: Cache stampede protection: drupal_render() and page cache - in that example, processes made to wait ended up returning in 250ms instead of 1050ms without this patch, or 400ms without stampede protection.

Since lock.inc is pluggable this change wouldn't have to go into core, but I think it's a fair trade-off to make. We could also make the polling time a variable so it could be configured without a custom lock.inc -which would be a very simple patch too, but not sure if we need that level of control here or not compared to using a non-SQL backend for it.

This is better done once the poll decay in the other issue is in.

That other patch has been RTBC for months now, but this is still a big issue. I'm bumping this to major since the variable_init() lock is the equivalent of a global writeread lock across the whole of a site, which means it's very easy to suddenly have a tonne of processes waiting one or more seconds on a variable cache miss, which happens a lot.

New patch does both the usleep(100000); and the decay polling.

I split this out from #802446: Cache stampede protection: drupal_render() and page cache so it could get in separate from adding more locking. Re-attaching the patch from #6.

Just thought of something - we should double the wait time until we reach two seconds, but not continue to double it after that. One query every two seconds is nothing.

So this will now look like:

100ms (100)
200ms (300)
400ms (700)
800ms (1500)
1600ms (3100)
2000ms (5100)
2000ms (7100)
etc.

Paul pointed out in irc that min(2 etc.) was going to mean two usec all the time, not a maximum of two seconds. Re-rolled to correct that.

Meant to bump this to major in #7 but never actually did it. This is the scenario:

1. variable_initialize() acquires a lock
2. Several other requests hit immediately afterwards and have to lock_wait()
3. Your site stops serving requests altogether for a while because your apache processes are maxed out while PHP is sleeping.

While people can install other options, a lot of sites won't, and fixing the core implementation will encourage people to look at how best to handle this in the alternatives.

No that's fair enough, I've updated the comments with more detail.

While writing the comments, I started feeling even more paranoid.

If using normal page caching, the variables cache is loaded when serving a cached page. If there's a lock on the variables table, then requests that normally take 20-30ms to serve in total, would be waiting for at least 100ms (and building and saving the variables cache should take less than 100ms anyway). Also, even if a lock is acquired and released in say 200ms, a lot of requests could start waiting 150ms or 175ms into this process - and they won't release until say 275ms.

Let's say there's even half a second between a cache miss and the last process to start lock waiting to exit from lock_wait() (and it could be more), if you serve 100 requests / second and have max clients set to 25, you could already start to run into trouble with apache itself getting backed up with requests. Something like css/js aggregation that's calling variable_set() multiple times from multiple different pages is going to prolong this process - so some could wait across other processes acquiring the same lock and take much longer to exit than it does to build the cache once.

So this patch also reduces the minimum wait time to 25ms, and instead of doubling, adds 25ms each wait instead.

So it'll look like:

poll time / total time spent waiting so far

25 (25)
50 (75)
75 (150)
100 (250)
125 (375)
150 (525)
175 (700)
200 (900)
225 (1125)
250 (1375)
275 (1650) (10 queries)
300 (1950)
325 (2275)
350 (2625)
375 (3000)
400 (3400)
425 (3825)
450 (4275)
475 (4750)
500 (5250)
500 (5750)...

This still means that a process that needs to wait for one second, will issue less db queries than the original patch waiting 100ms every time. Drupal 6 would sometimes issue several hundred queries to build one page with no waiting between them, so this is still a tiny number of queries comparatively, and it's still possible to use memcache-lock.inc too to eliminate them altogether.

grr, had to make that change /after/ updating the comments.

#17: lock_wait_802856.patch queued for re-testing.

Hmm, no reason at all. Attached patch which just changes the min() call.

Tagging.

Can be applied to 6 but cnw due to the windows issue pwolanin outlined.