-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
chore: Delete persons workflow retries forever #29255
base: master
Are you sure you want to change the base?
Conversation
start_to_close_timeout=dt.timedelta(hours=2), | ||
start_to_close_timeout=dt.timedelta(minutes=5), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reduced this as it seemed too much just to mogrify a few queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR modifies the delete persons workflow to retry indefinitely on failures, eliminating the need for manual intervention after worker restarts.
- Changed
mogrify_delete_queries_activity
timeout from 2 hours to 5 minutes to better match this preparatory activity's typical duration - Set
maximum_attempts=0
(infinite retries) in retry policies for both activities, replacing the previous limit of 1 attempt - Increased
maximum_interval
fordelete_persons_activity
retry policy from 60 to 360 seconds, allowing longer backoff periods between retries - These changes ensure the workflow can automatically recover from transient failures without manual intervention
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
@@ -235,8 +235,8 @@ async def run(self, inputs: DeletePersonsWorkflowInputs): | |||
start_to_close_timeout=dt.timedelta(hours=2), | |||
retry_policy=temporalio.common.RetryPolicy( | |||
initial_interval=dt.timedelta(seconds=10), | |||
maximum_interval=dt.timedelta(seconds=60), | |||
maximum_attempts=1, | |||
maximum_interval=dt.timedelta(seconds=360), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped this in case we enter a retry loop, to have more time in between and not contribute to whatever is causing the retry loop.
Problem
Delete persons workflow runs without problems, but worker restarts can cause it to fail and have to be manually retried.
Changes
Let Temporal handle retrying by setting a RetryPolicy without retry limit.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?