-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Reduces the memory requirement of a standalone Prefect server again #16651
Conversation
In #16644, we were able to get the standalone Prefect server we run in `prefect server start` from 400MB to 343MB. By running `uvicorn` directly in the parent process, we further reduce that to *266MB*. I measured this using the same process as before, using a `cgroup` with no swap to constrain the memory until I found the minimum. There are two main changes here: 1. Making `prefect server start` a synchronous command in both the background and the foreground cases. This helps to simplify the handling of signals and removes the need for `anyio` or piping the streams of a child process manually. 2. Splitting the paths between background and foreground servers. In the case of the background server, we run the subprocess as usual and then exit leaving it running. In the case of the foreground server, we run it with `uvicorn.run` as the final call, letting `uvicorn` handle signals. `uvicorn` already handles `SIGINT` and `SIGTERM` to cleanly shutdown, so we don't need to do anything additional here. I'm removing the two tests that were testing what happens when the signals are sent in escalating succession, as they aren't materially different now. In both cases, `uvicorn` will attempt to shutdown cleanly. I also updated the tests to reflect that `SIGINT` will lead to a successful exit, while `SIGTERM` will lead to a -15 exit (pretty standard).
CodSpeed Performance ReportMerging #16651 will not alter performanceComparing Summary
|
from prefect.server.api.server import create_app | ||
|
||
with temporary_settings( | ||
{getattr(prefect.settings, k): v for k, v in server_settings.items()} |
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.
Is this cheeseball? Should I just explicitly set the Prefect settings here? I couldn't decide...
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.
This seems reasonable to me, considering the current state of settings. We need to update temporary_settings
to be more ergonomic now we're using pydantic settings, but this is the right way to do it now.
server_env["PREFECT_API_SERVICES_SCHEDULER_ENABLED"] = str(scheduler) | ||
server_env["PREFECT_SERVER_ANALYTICS_ENABLED"] = str(analytics) | ||
server_env["PREFECT_API_SERVICES_LATE_RUNS_ENABLED"] = str(late_runs) | ||
server_env["PREFECT_API_SERVICES_UI"] = str(ui) |
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.
While doing this, I discovered that PREFECT_API_SERVICES_UI
isn't a thing, so I removed it. Not sure if was supposed to be another setting or just an old variable?
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.
Yeah, PREFECT_UI_ENABLED
is the correct one, which also there, so this is good to remove.
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.
Awesome!
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.
rockstar!
In #16644, we were able to get the standalone Prefect server we run in
prefect server start
from 400MB to 343MB. By runninguvicorn
directly in the parent process, we further reduce that to 266MB. I
measured this using the same process as before, using a
cgroup
withno swap to constrain the memory until I found the minimum.
There are two main changes here:
Making
prefect server start
a synchronous command in both thebackground and the foreground cases. This helps to simplify the
handling of signals and removes the need for
anyio
or piping thestreams of a child process manually.
Splitting the paths between background and foreground servers. In
the case of the background server, we run the subprocess as usual and
then exit leaving it running. In the case of the foreground server,
we run it with
uvicorn.run
as the final call, lettinguvicorn
handle signals.
uvicorn
already handlesSIGINT
andSIGTERM
tocleanly shutdown, so we don't need to do anything additional here.
I'm removing the two tests that were testing what happens when the
signals are sent in escalating succession, as they aren't materially
different now. In both cases,
uvicorn
will attempt to shutdowncleanly. I also updated the tests to reflect that
SIGINT
will lead toa successful exit, while
SIGTERM
will lead to a -15 exit (prettystandard).
Checklist
<link to issue>
"mint.json
.