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 run_deployment timeout #2

Merged
merged 3 commits into from
Mar 8, 2024

Conversation

AbbyGi
Copy link
Contributor

@AbbyGi AbbyGi commented Aug 1, 2023

This is to fix the issue where the prefect kafka consumer leaves the kafka group after timing out waiting for a deployment to complete.

@jklynch
Copy link
Contributor

jklynch commented Aug 1, 2023

I just read the documentation for the timeout parameter in run_deployment(...). I would be happy with timeout=0 and letting prefect start the runs as they come. Maybe the problem you are trying to deal with is one beamline sending a large number of jobs and other beamlines' jobs ending up at the end of the line? I wish prefect could deal with that.

@AbbyGi
Copy link
Contributor Author

AbbyGi commented Aug 1, 2023

Each beamline should have their own group, consumer, and agent so I don't think that's an issue. In my testing, I found that CMS runs a lot of short scans pretty quickly and the one CMS prefect agent we have running can't really keep up with the number of jobs trying to run. A better solution is probably to get more agents running per beamline if we want to have timeout be 0. Totally possible, just haven't gotten around to it yet.

@AbbyGi AbbyGi force-pushed the prefect-consumer-fix branch from b9aa65d to 76081a2 Compare March 8, 2024 15:16
@AbbyGi AbbyGi requested a review from padraic-shafer March 8, 2024 15:21
Copy link
Contributor

@padraic-shafer padraic-shafer left a comment

Choose a reason for hiding this comment

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

Looks good.

@padraic-shafer padraic-shafer merged commit 49f1818 into NSLS-II:main Mar 8, 2024
5 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