Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add generic thread queues, use them to implement autocue-specific queue #4151

Merged
merged 6 commits into from
Oct 2, 2024

Conversation

toots
Copy link
Member

@toots toots commented Oct 1, 2024

This PR changes the way we handle queues in the scheduler.

Previously, we had the following queues:

  • non_blocking, for processing fast tasks, typically for the harbor server and telnet.
  • blocking, for blocking tasks.
  • maybe_blocking for... ? This was never clear.

These were maped to, resp, non_blocking, generic and fastqueues, fast queues processing maybe_blocking tasks.. 🤔.

By default, there were 0 fast queues (again, not sure why) and 2 generic queues. Generic queues process any available task.

With the adoption of autocue internal resolution via source.drop, we started seeing some interesting queue deadlock:

  1. 2 generic queues are created
  2. 2 request.dynamic or playlist are created, each requesting a request resolution.
  3. Each request resolution kicks in an autocue computation which, in turn, also requires a request resolution.
  4. No more queues are available to resolve the autocue request resolution
  5. The top-level request resolution never terminate.

This PR cleans up the scheduler notion of queues and allows for dedicated queues to fix this dependency issue.

Default queues, named queues

By default, we only have 2 type of queues:

  • non_blocking queues, defaults to 2
  • generic, defaults to 2.

We also support named queues, which are queues with any arbitrary string name value. These are created when requested by the user.

New queues can be configured as follows:

settings.scheduler.queues.set([
   ...settings.scheduler.queues(),
  ("my-queue", 4)
])

Now, each type of queue only process tasks for their own queue, making it possible to control the concurrency of each tasks sent to them.

All the thread.run.* functions are updated to remove the fast parameter and add an explicit queue parameter instead, which controls to which queue a given task is sent.

autocue queue

autocue resolutions are not sent to a dedicated autocue queue. Only one autocue queue is created by default.

This is made possible by adding a thread_queue parameter to request.dynamic and all the operators relying on it.

This way, the autocue computations happen one at a time by default, which help controlling CPU usage with them, and never prevent request resolution from completing.

@toots toots requested a review from smimram October 2, 2024 05:05
@toots toots marked this pull request as ready for review October 2, 2024 05:05
@toots
Copy link
Member Author

toots commented Oct 2, 2024

Comment from @smimram: "I never understood what maybe_blocking was for" haha me too buddy.

This seems safe to merge. There were only generic queues by default before so the overall behavior will not change.

Will add a migration doc and generic scheduler/thread page to the doc with it as well.

@toots toots enabled auto-merge October 2, 2024 14:35
@toots toots added this pull request to the merge queue Oct 2, 2024
Merged via the queue into main with commit fcd8bf4 Oct 2, 2024
25 checks passed
@toots toots deleted the thread-queues branch October 2, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant