Skip to content

ref(workerchild): move checking future completion into bg thread#727

Merged
bmckerry merged 1 commit into
mainfrom
ben/check-futures-bg-thread
Jun 23, 2026
Merged

ref(workerchild): move checking future completion into bg thread#727
bmckerry merged 1 commit into
mainfrom
ben/check-futures-bg-thread

Conversation

@bmckerry

Copy link
Copy Markdown
Member

Previously, future completion was only checked at the start of the worker child loop. A common pattern we saw was:

  • a task executes, produces futures, next loop iteration runs immediately and those futures aren't complete yet
  • next task executes, and since some tasks can take 5-60s to complete, we're stuck 5-60s until we check future completion again
    • this meant that TaskProducer can increase the latency between a worker receiving a task and marking the task as complete from N to (N+M), where N is the execution time of the original task and M is the execution time of the next task

This didn't affect task throughput (since checking doneness is non-blocking), but did mean tasks would sometime exceed their processing deadline which is bad.

This PR tries to deal with that by moving this logic into a separate thread in the worker child.

@bmckerry bmckerry requested a review from a team as a code owner June 23, 2026 15:28
Comment thread clients/python/src/taskbroker_client/worker/workerchild.py

@untitaker untitaker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cursor comment seems legit, but rubberstamping otherwise

Comment thread clients/python/src/taskbroker_client/worker/workerchild.py
@bmckerry bmckerry force-pushed the ben/check-futures-bg-thread branch from f180a08 to 04b0503 Compare June 23, 2026 16:02
Comment thread clients/python/src/taskbroker_client/worker/workerchild.py Outdated
Comment thread clients/python/src/taskbroker_client/worker/workerchild.py Outdated
@bmckerry bmckerry force-pushed the ben/check-futures-bg-thread branch from 04b0503 to b7e2223 Compare June 23, 2026 16:18

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b7e2223. Configure here.

Comment thread clients/python/src/taskbroker_client/worker/workerchild.py
Comment thread clients/python/src/taskbroker_client/worker/workerchild.py
Comment thread clients/python/src/taskbroker_client/worker/workerchild.py
@bmckerry bmckerry force-pushed the ben/check-futures-bg-thread branch from b7e2223 to b377335 Compare June 23, 2026 16:30
@bmckerry bmckerry force-pushed the ben/check-futures-bg-thread branch from b377335 to 3692b7d Compare June 23, 2026 16:32
@bmckerry bmckerry merged commit 6ed63c5 into main Jun 23, 2026
27 checks passed
@bmckerry bmckerry deleted the ben/check-futures-bg-thread branch June 23, 2026 17:24
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.

3 participants