Skip to content

fix: SQSUpkeepHandler + single worker activity handler#1161

Merged
dodamih merged 5 commits intomainfrom
dodam/single_worker_activity_tracker
Feb 9, 2026
Merged

fix: SQSUpkeepHandler + single worker activity handler#1161
dodamih merged 5 commits intomainfrom
dodam/single_worker_activity_tracker

Conversation

@dodamih
Copy link
Collaborator

@dodamih dodamih commented Feb 3, 2026

No description provided.

@dodamih dodamih force-pushed the dodam/single_worker_activity_tracker branch from c792108 to 3ed5d6f Compare February 3, 2026 06:33
@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.96%. Comparing base (a66a409) to head (864f21d).
⚠️ Report is 92 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1161      +/-   ##
==========================================
- Coverage   99.97%   99.96%   -0.02%     
==========================================
  Files         180      192      +12     
  Lines        8912    10199    +1287     
==========================================
+ Hits         8910    10195    +1285     
- Misses          2        4       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dodamih dodamih force-pushed the dodam/single_worker_activity_tracker branch from 3ed5d6f to 5745484 Compare February 3, 2026 17:40
Copy link
Collaborator

@nkemnitz nkemnitz left a comment

Choose a reason for hiding this comment

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

Just so I get the idea: Each worker now spawns its own new process, and the this process keeps extending the SQS lease for the task currently being processed in the main process? And this solves the issue that poorly behaving tasks which don't release the GIL were able to prevent the lease extension, causing SQS to re-release and eventually fill all other workers with the same neverending, GIL-blocking task?

But why do you need the Queue and the active_upkeeps tracking? A single worker can only process a single task at any time, I think? So the UpkeepHandler will only ever track a single tasks lease? Or was the intention to have a single SQSUpkeepHandlerManager process that coordinates the leases for all workers?

@dodamih
Copy link
Collaborator Author

dodamih commented Feb 5, 2026

@nkemnitz The Queue is just a way for the parent process to send a command to the upkeep process asynchronously. Also, the use case is one Upkeep proc per worker proc, but figured this was more general and we aren't losing anything by making it more futureproof.

@dodamih dodamih requested a review from nkemnitz February 5, 2026 01:14
Copy link
Collaborator

@nkemnitz nkemnitz left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me.

@dodamih dodamih merged commit 188a890 into main Feb 9, 2026
9 of 10 checks passed
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