BUG/MEDIUM: task: bound the number of tasks picked from the wait queue at once
authorWilly Tarreau <w@1wt.eu>
Fri, 16 Oct 2020 07:26:22 +0000 (09:26 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 16 Oct 2020 13:18:48 +0000 (15:18 +0200)
commit3cfaa8d1e075b5037b15775ec6b81ef16c4d81ce
treebbfed460d7a954ab9d0ce421b4bb8848e0e702f8
parentba29687bc196e0eb1c1752181e3d49a10a631887
BUG/MEDIUM: task: bound the number of tasks picked from the wait queue at once

There is a theorical problem in the wait queue, which is that with many
threads, one could spend a lot of time looping on the newly expired tasks,
causing a lot of contention on the global wq_lock and on the global
rq_lock. This initially sounds bening, but if another thread does just
a task_schedule() or task_queue(), it might end up waiting for a long
time on this lock, and this wait time will count on its execution budget,
degrading the end user's experience and possibly risking to trigger the
watchdog if that lasts too long.

The simplest (and backportable) solution here consists in bounding the
number of expired tasks that may be picked from the global wait queue at
once by a thread, given that all other ones will do it as well anyway.

We don't need to pick more than global.tune.runqueue_depth tasks at once
as we won't process more, so this counter is updated for both the local
and the global queues: threads with more local expired tasks will pick
less global tasks and conversely, keeping the load balanced between all
threads. This will guarantee a much lower latency if/when wakeup storms
happen (e.g. hundreds of thousands of synchronized health checks).

Note that some crashes have been witnessed with 1/4 of the threads in
wake_expired_tasks() and, while the issue might or might not be related,
not having reasonable bounds here definitely justifies why we can spend
so much time there.

This patch should be backported, probably as far as 2.0 (maybe with
some adaptations).
src/task.c