Details
-
Task
-
Resolution: Unresolved
-
Major
-
None
-
master
-
None
Description
Related to the cause of MB-32181, currently the APIs to schedule, run and snooze GlobalTasks in ep-engine are brittle and easy to use incorrectly:
- GlobalTasks maintain their own wakeTime as a member variable; however these tasks are then referenced from a FutureQueue (std::priority_queue) in wakeTime order.
- If a GlobalTask has it's wakeTime changed; we must then re-sort the FutureQueue.
- If we don't re-sort (as was the case with
MB-32181), then scheduling breaks horribly. - By default a task will be re-scheduled immediately if the run() method completes (and the method doesn't return false to cancel the task).
- To override this behaviour the run() method must call snooze() or wakeUp() to modify the wakeTime - however those functions are public and can be called by anyone, even though it would be unsafe to call them if the FutureQueue isn't going to be re-sorted.
Possible improvements to this vary:
One modest change would be to change the run() method API instead of returning just a bool (true=schedule again, false=cancel), it returns an optional<time_point> :-
- If the time point is empty then cancel the task.
- If the time point is non-empty then it specifies the next time to schedule the task
This would enforce that a task specifies exactly once, when it should be run again (and removes the immediate re-scheudule assumption which actually a lot of tasks don't want). The GlobalTask::wakeUp and GlobalTask::snooze methods could then be removed.
Another approach would be something more ambitious - maybe look to move to a totally new scheduling API such as folly::future or the wangle library...
Attachments
Issue Links
- relates to
-
MB-36956 Migrate to Facebook Folly executors for CPU & IO background tasks
- Closed