-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[SsrSite]: Add function warming to all SsrSite constructs #2988
Comments
I have test deployed a RemixSite with 2 warmer functions and it has been working well, this is mostly just taking what @fwang did with the NextjsSite for function warming and applying it generally to all SsrSite constructs. One important note, when deploying NextjsSite, the |
Good news for anyone who wants function warming right away! I created a package that is very similar to this Issue+PR , you can now use this package as a drop-in replacement for your existing RemixSite, SvelteKitSite, AstroSite, or SolidStartSite. This package exists only to bridge the gap until function warming is added to SST proper, but in the meantime I hope this is useful. I have successfully deploy my personal site with function warming (which is a RemixSite) with this new package, and the code for each site is identical: https://github.com/SpencerDuball/sst-warmer |
Thanks @SpencerDuball! Fixed in #3301 |
Feature Request
I want to have the option to warm functions with all
SsrSite
constructs similar to how we can warm function withNextjsSite
construct via open-next. The specific ask is to have theSsrSite
updated to acceptwarm: number
as an option keeping the same API asNextjsSite
- this would allow all other constructs such asRemixSite
,SvelteKitSite
,SolidStartSite
,AstroSite
to keep instances warm.Overview
I was having a look at the implementation for the open-next to mimic this for
RemixSite
and try to get a PR pushed but there were two larger design decisions that I think warrant some discussion before pushing something.warm
feature should be part of theSsrSite
APIsst
, not as an implementation detail of the specific site you are deploying.1.
warm
part ofSsrSite
I was having a look at the
NextjsSite
to mimic this behavior and see that this warming feature is really two parts. There is acreateWarmer
function on theNextjsSite
construct that will deploy a warmer lambda and a cron job to invoke this lambda. Then there is the actual warmer lambda (which is part of the open-next package) here.The reason this could be generalized for all
SsrSite
constructs is because there is only ever one lambda for these regional deploys (maybe many instances, but only one lambda arn) which exists asthis.serverLambdaForRegional
. There is nothing unique about theNextjsSite
that would preclude the others from using this exact samethis.createWarmer
function.2. Warmer Handler in SST, not open-next
The next thought would be to have the actual warmer lambda be defined as part of SST not open-next (of any other type of site). Currently, the
NextjsSite
statically defines this location for the warmer lambda as.open-next/warmer-function.ts
but there is also nothing unique toNextjsSite
that would preclude us from making this function generic to all constructs. It should be possible to have this pre-bundled lambda be included as part of SST itself.I would be happy to create a PR for this, the only issue I could see if that open-next is not specific to SST. Would moving this handler lambda to SST be a deal breaker for compatibility with other deploy options that open-next supports? For example, do we need to have the warmer lambda be
.open-next/warmer-function.ts
for any reason?The text was updated successfully, but these errors were encountered: