feat: ensure task scheduling fairness and FIFO order

[kokovtsev]

Overview

Fixes #1031

Note: I was unsure about the target branch for this change (v5 vs current), please let me know if the PR should be recreated to target a different branch.

In some code paths, a task is shifted from the tail of the queue, but then if we failed to distribute it to a worker it is pushed to the head of the queue.

This breaks the FIFO ordering of the tasks, which is not a big problem by itself, because the real order of execution is non-deterministic in a multi-threaded environment. However, this leads to the fairness issues, where some tasks are waiting in queue more that others, causing latency jitter. In some edge cases (I have consistently observed this when using 1 worker), the same task may lose its place in the queue numerous times, getting delayed by dozens of seconds.

Implementation details:

1. _distributeTask is not responsible for returning a task back to the queue anymore. Now the code calling _distributeTask should return the task into a correct place (head or tail, skipQueue or taskQueue)
2. A new operation unshift has been added to the TaskQueue interface and the 2 builtin implementations. This operation is made optional, so that clients who provide custom implementations of TaskQueue can continue using them without needing to write new code.

Category

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation
• Maintenance (refactoring, dependency updates, etc.)

Contribution Checklist

• I have reviewed the guidelines for this project.
• I agree to follow the Code of Conduct.
• My code adheres to the project's style and conventions.
• I have added tests to cover my changes.
• All tests passed locally.
• I have updated relevant documentation.

DCO Sign-off

• I certify that I have the right to submit this contribution under the open source license (DCO 1.1).

[metcoder95]

Thanks for the contribution! LGTM

The only concern I've is the change on the current behavior. Although we provide backwards compatibility for custom task_queues that does not provides the unshift method, the PR per-se changes the current behaviour of the queues that implementations might already have some expectations on.

Not sure if we should guard this through a flag or similar, just to preserve the current behavior and allow opt in for the new one.

[kokovtsev]

Good point, thanks!

I can add a new stricterFIFO flag for enabling this change, does that sound right for you?

[metcoder95]

Sure, let's do that so it can also land on v5 👍

[metcoder95]

I'm gonna go ahead and send it to v6, feel free to open a pr againg v5 branch to back port it

[metcoder95]

lint seems failing

[github-actions]

This issue has been marked as stale because it has been opened 45 days without activity. Remove stale label or comment or this will be closed in 10 days.