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

feat(taskworker): Remove task input queue #86151

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

evanh
Copy link
Member

@evanh evanh commented Feb 28, 2025

It was found that tasks were dying in the queue while waiting to be executed. One workaround for this is to have the input queue be only of size 1, but that means there is still one task that might die in the queue waiting to be executed.

Instead, have the children signal to the parent that they are ready for more tasks. This way the parent doesn't make any gRPC calls unless the children are idle.

Change the children to also pass one end of a pipe to the parent in the results queue. The parent is constantly draining the queue, and for every result in the queue, is fetching a new task and putting it on one of the channels.

This is also a first step towards making batched gRPC calls, since now the parent is aware of how many tasks it needs to update/fetch at a time.

The size of the queue can be set to the number of children, and when a child starts up it will put a signal into the queue (WaitResult) that is just a marker that the child is ready for a task.

In this scenario, in order to avoid constant polling of the broker, if the queue is full of idle workers, and no new tasks are fetched, then backoff.

It was found that tasks were dying in the queue while waiting to be executed. One workaround for
this is to have the input queue be only of size 1, but that means there is still one task that might
die in the queue waiting to be executed.

Instead, have the children signal to the parent that they are ready for more tasks. This way the
parent doesn't make any gRPC calls unless the children are idle.

Change the children to also pass one end of a pipe to the parent in the results queue. The parent is
constantly draining the queue, and for every result in the queue, is fetching a new task and putting
it on one of the channels.

This is also a first step towards making batched gRPC calls, since now the parent is aware of how
many tasks it needs to update/fetch at a time.

The size of the queue can be set to the number of children, and when a child starts up it will put a
signal into the queue (WaitResult) that is just a marker that the child is ready for a task.

In this scenario, in order to avoid constant polling of the broker, if the queue is full of idle
workers, and no new tasks are fetched, then backoff.
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 28, 2025
Copy link

codecov bot commented Feb 28, 2025

❌ 8 Tests Failed:

Tests completed Failed Passed Skipped
23765 8 23757 290
View the top 3 failed test(s) by shortest run time
tests.sentry.taskworker.test_worker::test_child_worker_unknown_task
Stack Traces | 0.035s run time
#x1B[1m#x1B[.../sentry/taskworker/test_worker.py#x1B[0m:200: in test_child_worker_unknown_task
    child_worker(todo, processed, shutdown, max_task_count=1)
#x1B[1m#x1B[31mE   TypeError: child_worker() got multiple values for argument 'max_task_count'#x1B[0m
tests.sentry.taskworker.test_worker::test_child_worker_failure_task
Stack Traces | 0.036s run time
#x1B[1m#x1B[.../sentry/taskworker/test_worker.py#x1B[0m:171: in test_child_worker_failure_task
    child_worker(todo, processed, shutdown, max_task_count=1)
#x1B[1m#x1B[31mE   TypeError: child_worker() got multiple values for argument 'max_task_count'#x1B[0m
tests.sentry.taskworker.test_worker::test_child_worker_shutdown
Stack Traces | 0.036s run time
#x1B[1m#x1B[.../sentry/taskworker/test_worker.py#x1B[0m:186: in test_child_worker_shutdown
    child_worker(todo, processed, shutdown, max_task_count=1)
#x1B[1m#x1B[31mE   TypeError: child_worker() got multiple values for argument 'max_task_count'#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant