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_flow_in_subprocess utility #16802

Merged
merged 12 commits into from
Jan 28, 2025
Merged

Conversation

desertaxle
Copy link
Member

@desertaxle desertaxle commented Jan 21, 2025

This PR adds a run_flow_in_subprocess utility, which encapsulates the logic necessary to take an in-memory Flow object and run it in a separate process. This utility will be useful when used within the Runner, which can hold onto in-memory Flow objects when started via flow.server and simplify code retrieval when executing flow runs for remotely stored flows.

run_flow_in_subprocess uses cloudpickle to serialize the provided flow, parameters, and other kwargs when delegating execution to a subprocess. Subprocesses are created via spawn rather than fork because the flow engine deadlocks when started in a forked process. I haven't yet discovered why it deadlocks in a forked process, but it'd be worthwhile to understand and potentially resolve that behavior.

Copy link

codspeed-hq bot commented Jan 21, 2025

CodSpeed Performance Report

Merging #16802 will not alter performance

Comparing refactor-runner-process-handling (17e2337) with main (f481fc0)

Summary

✅ 2 untouched benchmarks

@desertaxle desertaxle force-pushed the refactor-runner-process-handling branch 2 times, most recently from 25485cf to 9239c3b Compare January 23, 2025 22:27
@desertaxle desertaxle force-pushed the refactor-runner-process-handling branch from 36ded36 to 577b457 Compare January 24, 2025 14:56
@desertaxle desertaxle changed the title Refactor runner process handling Add run_flow_in_subprocess utility Jan 28, 2025
@desertaxle desertaxle marked this pull request as ready for review January 28, 2025 18:11
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

lgtm

@desertaxle desertaxle merged commit 10d2c75 into main Jan 28, 2025
49 checks passed
@desertaxle desertaxle deleted the refactor-runner-process-handling branch January 28, 2025 19:14
flow: "Flow[..., Any]",
flow_run: "FlowRun | None" = None,
parameters: dict[str, Any] | None = None,
wait_for: Iterable[PrefectFuture[R]] | None = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry, i was late to review, but this shouldn't be R should it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yeah you're right

@desertaxle desertaxle added the enhancement An improvement of an existing feature label Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants