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 ability to limit how many handlers are processed #114

Merged
merged 11 commits into from
Mar 14, 2025

Conversation

jaredlewis
Copy link
Contributor

@jaredlewis jaredlewis commented Mar 12, 2025

Since we have so many instances using rrule now, the single task handler is timing out trying to process all handlers within the same transaction. This will give us the ability to limit how many instances are being processed at once and give us the ability to create tasks that will process much smaller bathes of recurrences.

This is not an ideal fix and would like to revisit this to propose a better more robust solution, but this is necessary given our current state.

@wesleykendall wesleykendall self-requested a review March 12, 2025 01:39
Copy link

@wesleykendall wesleykendall left a comment

Choose a reason for hiding this comment

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

I think I see your general strategy here and it lgtm.

Do we actually use this rrule module anywhere outside Ambition? I dont see it in Houston or Data. There are already docs in Ambitions repo about this module too.

probably best to eventually consolidate this one too

@jaredlewis
Copy link
Contributor Author

i think its used in houston but only for the lock which can be replaced with pglock very easily

order_by=F('next_occurrence').asc()
)
).filter(
row_num=1
Copy link
Contributor

Choose a reason for hiding this comment

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

so this is the part that restricts the rrule handler from processing more than one recurrence type in a single tick?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this just ensures proper ordering in the event we are running with a limit, we want to prioritize a handler that has the most out of date occurrence

@jaredlewis jaredlewis merged commit 4db809c into ambitioninc:develop Mar 14, 2025
4 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