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

Prevent workers from running flow runs scheduled for in process retry #15482

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

desertaxle
Copy link
Member

@desertaxle desertaxle commented Sep 24, 2024

This PR adds the retry_type field to FlowRunPolicy, so the server can determine if a flow will be retried via an already running process or needs to be rescheduled. Automatic retries will have retry_type set to in_process via the existing RetryFailedFlows orchestration policy. The query to get scheduled flow runs has been updated to filter out flow runs that are in AwaitingRetry but will be retried via an existing process. When transition out of a terminal state into a SCHEDULED state (i.e. manual retries) the retry_type will be set to reschedule and will be given to workers for execution by the server.

Closes #15458

@github-actions github-actions bot added the bug Something isn't working label Sep 24, 2024
@desertaxle
Copy link
Member Author

desertaxle commented Sep 24, 2024

@zzstoatzz has pointed out that this change will break flow retries from the UI since it uses the AwaitingRetry state. Closing this PR and exploring other solutions.

Edit: This was for the initial implementation that used run_count. The updated implementation doesn't have this problem.

Allows server to distinguish which `AwaitingRetry` flows should be returned when retrieving scheduled flow runs
@desertaxle desertaxle reopened this Sep 25, 2024
@github-actions github-actions bot added the docs label Sep 25, 2024
Copy link

codspeed-hq bot commented Sep 25, 2024

CodSpeed Performance Report

Merging #15482 will not alter performance

Comparing fix-duplicate-runs-on-awaiting-retry (8bff9da) with main (f168d96)

Summary

✅ 3 untouched benchmarks

@desertaxle desertaxle changed the title Prevent workers from running flow runs in AwaitingRetry Prevent workers from running flow runs scheduled for in process retry Sep 25, 2024
@desertaxle desertaxle marked this pull request as ready for review September 25, 2024 15:02
Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

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

🥇 lgtm

@jashbycu
Copy link

Hi all. Is there a plan to get this into a release soon? This issue has been a major pain for us at CU Boulder and we would love to see this fix deployed. Thank you!

Copy link
Contributor

This pull request is stale because it has been open 14 days with no activity. To keep this pull request open remove stale label or comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs
Projects
None yet
3 participants