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

chore: Delete persons workflow retries forever #29255

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomasfarias
Copy link
Contributor

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?

start_to_close_timeout=dt.timedelta(hours=2),
start_to_close_timeout=dt.timedelta(minutes=5),
Copy link
Contributor Author

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.

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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 for delete_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),
Copy link
Contributor Author

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.

@tomasfarias tomasfarias requested a review from rossgray February 26, 2025 15:17
@tomasfarias tomasfarias enabled auto-merge (squash) February 26, 2025 16:45
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.

1 participant