From 77015abe0bcfde67bff519b1d48393a513015f77 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 19 Jun 2020 11:50:27 +0200 Subject: [PATCH] MEDIUM: tasks: clean up the front side of the wait queue in wake_expired_tasks() Due to the way the wait queue works, some tasks might be postponed but not requeued. However when we exit wake_expired_tasks() on a not-yet-expired task and leave it in this situation, the next call to next_timer_expiry() will use this first task's key in the tree as an expiration date, but this date might be totally off and cause needless wakeups just to reposition it. This patch makes sure that we leave wake_expired_tasks with a clean state of frontside tasks and that their tree's key matches their expiration date. Doing so we can already observe a ~15% reduction of the number of wakeups when dealing with large numbers of health checks. The patch looks large because the code was rearranged but the real change is to take the wakeup/requeue decision on the task's expiration date instead of the tree node's key, the rest is unchanged. --- src/task.c | 69 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/src/task.c b/src/task.c index 3d615622e7..1a7f44d916 100644 --- a/src/task.c +++ b/src/task.c @@ -177,13 +177,6 @@ void wake_expired_tasks() break; } - if (tick_is_lt(now_ms, eb->key)) - break; - - /* timer looks expired, detach it from the queue */ - task = eb32_entry(eb, struct task, wq); - __task_unlink_wq(task); - /* It is possible that this task was left at an earlier place in the * tree because a recent call to task_queue() has not moved it. This * happens when the new expiration date is later than the old one. @@ -195,14 +188,31 @@ void wake_expired_tasks() * the same place, before , so we have to check if this happens, * and adjust , otherwise we may skip it which is not what we want. * We may also not requeue the task (and not point eb at it) if its - * expiration time is not set. + * expiration time is not set. We also make sure we leave the real + * expiration date for the next task in the queue so that when calling + * next_timer_expiry() we're guaranteed to see the next real date and + * not the next apparent date. This is in order to avoid useless + * wakeups. */ - if (!tick_is_expired(task->expire, now_ms)) { + + task = eb32_entry(eb, struct task, wq); + if (tick_is_expired(task->expire, now_ms)) { + /* expired task, wake it up */ + __task_unlink_wq(task); + task_wakeup(task, TASK_WOKEN_TIMER); + } + else if (task->expire != eb->key) { + /* task is not expired but its key doesn't match so let's + * update it and skip to next apparently expired task. + */ + __task_unlink_wq(task); if (tick_isset(task->expire)) __task_queue(task, &tt->timers); - goto lookup_next_local; } - task_wakeup(task, TASK_WOKEN_TIMER); + else { + /* task not expired and correctly placed */ + break; + } } #ifdef USE_THREAD @@ -240,32 +250,25 @@ void wake_expired_tasks() break; } - if (tick_is_lt(now_ms, eb->key)) - break; - - /* timer looks expired, detach it from the queue */ task = eb32_entry(eb, struct task, wq); - __task_unlink_wq(task); - - /* It is possible that this task was left at an earlier place in the - * tree because a recent call to task_queue() has not moved it. This - * happens when the new expiration date is later than the old one. - * Since it is very unlikely that we reach a timeout anyway, it's a - * lot cheaper to proceed like this because we almost never update - * the tree. We may also find disabled expiration dates there. Since - * we have detached the task from the tree, we simply call task_queue - * to take care of this. Note that we might occasionally requeue it at - * the same place, before , so we have to check if this happens, - * and adjust , otherwise we may skip it which is not what we want. - * We may also not requeue the task (and not point eb at it) if its - * expiration time is not set. - */ - if (!tick_is_expired(task->expire, now_ms)) { + if (tick_is_expired(task->expire, now_ms)) { + /* expired task, wake it up */ + __task_unlink_wq(task); + task_wakeup(task, TASK_WOKEN_TIMER); + } + else if (task->expire != eb->key) { + /* task is not expired but its key doesn't match so let's + * update it and skip to next apparently expired task. + */ + __task_unlink_wq(task); if (tick_isset(task->expire)) - __task_queue(task, &timers); + __task_queue(task, &tt->timers); goto lookup_next; } - task_wakeup(task, TASK_WOKEN_TIMER); + else { + /* task not expired and correctly placed */ + break; + } HA_RWLOCK_WRUNLOCK(TASK_WQ_LOCK, &wq_lock); } -- 2.39.5