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

Coldstart can be drastically reduced by calling LambdaHandler externally #982

Merged
merged 5 commits into from
Dec 9, 2021

Conversation

xncbf
Copy link
Contributor

@xncbf xncbf commented May 24, 2021

Description

Coldstart can be drastically reduced by calling LambdaHandler externally.

Before

image
image

After

image
image
image

GitHub Issues

Miserlou/Zappa#1974
#810

@xncbf xncbf changed the title Update handler.py Coldstart can be drastically reduced by calling LambdaHandler externally May 24, 2021
@xncbf
Copy link
Contributor Author

xncbf commented May 24, 2021

Test code is failing because settings_name does not exist. Is there any way to operate without settings_name?

@Yaronn44
Copy link

I'm really interesting in this feature.
How many invocations did you do in the Before part ?

@xncbf
Copy link
Contributor Author

xncbf commented May 24, 2021

@Yaronn44 I'm not sure because I didn't capture it, but it's definitely less than 1k

@xncbf
Copy link
Contributor Author

xncbf commented May 24, 2021

The invocation number is meaningless btw

@Yaronn44
Copy link

How does it work under the hood ? Why does calling the class like that improve the perfs ?

@xncbf
Copy link
Contributor Author

xncbf commented May 25, 2021

I don't know the detailed principle, but there seems to be some delay in the first invocation after the cold start.

@xncbf
Copy link
Contributor Author

xncbf commented May 26, 2021

I need help to pass the test.

@flanaman
Copy link

@Yaronn44 The "execution context" is kept around for subsequent lambda invocations:
https://medium.com/api-api-joy-joy/unleashed-lambdas-outside-the-handler-4e5fc27cb7b3

See "Take advantage of execution environment reuse to improve the performance of your function":
https://docs.aws.amazon.com/lambda/latest/dg/best-practices.html

I've had an issue with zappa opening too many simultaneous database connections so I'm hoping that this feature will solve that problem

@flanaman
Copy link

flanaman commented May 27, 2021

@Yaronn44 The "execution context" is kept around for subsequent lambda invocations:
https://medium.com/api-api-joy-joy/unleashed-lambdas-outside-the-handler-4e5fc27cb7b3

See "Take advantage of execution environment reuse to improve the performance of your function":
https://docs.aws.amazon.com/lambda/latest/dg/best-practices.html

I've had an issue with zappa opening too many simultaneous database connections so I'm hoping that this feature will solve that problem

Oh, I didn't see that singleton stuff at the top of the LambdaHandler class. I guess there might be a slight time gap between module import time and the handler invocation?

@MunsuPark
Copy link

@xncbf If you check x-ray in api gateway, you can see this commit is not working. If you are using slim_handler option, you could found that load_remote_project_archive takes 7-10 seconds. This function called when AWS lambda's concurrency is increased.

@xncbf
Copy link
Contributor Author

xncbf commented Jun 2, 2021

@MunsuPark I've tried several things.
I tried reducing the size of the package to less than 50MB and slim_handler=false. but still got the same symptoms.
I'm still using slim_handler=true and it reduces coldstart regardless of this.

@xncbf
Copy link
Contributor Author

xncbf commented Jun 2, 2021

In addition, I forked this branch and applied it to the production environment and have been using it for a week. There are a few other minor issues, but the cold start has definitely been reduced.

The following is the duration graph for the last 4 weeks.
The cursor points to the point at which you switched to this commit.

image

@cometurrata
Copy link

I'd really love to see that feature out 🙂

I need help to pass the test.

You could do something like:

if os.environ.get("INSTANTIATE_LAMBDA_HANDLER_ON_IMPORT"):
    LambdaHandler()

This would become something optional through the ENV variables 🤷 WDYT ?

@xncbf
Copy link
Contributor Author

xncbf commented Jun 22, 2021

I received a personal email from @MunsuPark on June 3rd stating that my commit was wrong.
The coldstart I measured was monitoring only the lambda, but when I looked at the overall flow (api-gateway > lambda), it still looked like the coldstart remained.
Clearly, the cold start still seems to exist. However, in the existing method, lambda initailize seems to be executed when the user calls the api.
When LambdaHandler() is called externally, the keep_warm function appears to take over.
This is pretty useful.
By calling the keep_warm function periodically, the system can experience the cold start that the user experiences instead.

This is my 6 hour x-ray
-api-gateway (Most users experience latency within 1.2 seconds)
image

-lambda-function (still have coldstart.)
image

This is the log of api-gateway before commit is applied.
There were users who experienced intermittent but 4-8 second latencies.
image

This is the api-gateway log up to now after commit is applied.
image

@cspollar
Copy link

cspollar commented Jul 6, 2021

I've been running a custom fork with aLambdaHandler() for 14+ months and Zappa feels faster.

Perhaps adding a configurable setting is the most conservative approach for now? The default behavior could be off, but allow users to enabled it if desired. Over time we could do more testing and change the default behavior.

This PR would need a quick documentation update - perhaps something like the following:

Cold Starts (Experimental)

Lambda may provide additional resources than provisioned during cold start initialization. Set INSTANTIATE_LAMBDA_HANDLER_ON_IMPORT=True to instantiate the lambda handler on import. This is an experimental feature - if startup time is critical, look into using Provisioned Concurrency.

@gitumarkk
Copy link

This change greatly reduces the Lambda cold start time in a project I am working on as well, by about 6 seconds. A configurable setting should work.

@Sheile
Copy link

Sheile commented Oct 26, 2021

Is there any update on this issue?

Our project has reached own patch like same solution and drastically reduced duration for lambda with provisioned concurrency.

Deploy this fix at about 18:20 and disappear spike by cold start with initialization.

Lambda duration

image

API Gateway latency

image

IMO, This PR is very useful if target lambda is configured with provisioned concurrency.
When configure provisioned concurrency, Specified number of instances are prepared regardless of invocation.
But not patched handler isn't initialize until invocation.

As a result, first invocation is slow even with provisioned concurrency.
This PR has resolved this issue in our product.

jcrafford added a commit to jcrafford/Zappa that referenced this pull request Oct 26, 2021
@xncbf
Copy link
Contributor Author

xncbf commented Dec 7, 2021

I just edited code by request. plz check
@hellno @wrboyce @javulticat

Copy link
Member

@wrboyce wrboyce left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@coveralls
Copy link

coveralls commented Dec 9, 2021

Coverage Status

Coverage decreased (-0.01%) to 73.521% when pulling 9828acd on xncbf:patch-1 into d359e22 on zappa:master.

@chiangf
Copy link
Contributor

chiangf commented Dec 24, 2021

@wrboyce I noticed an UnboundLocalError when running with INSTANTIATE_LAMBDA_HANDLER_ON_IMPORT=True, so put up #1096 as a potential fix

jcrafford added a commit to jcrafford/Zappa that referenced this pull request Jan 29, 2022
@xncbf xncbf deleted the patch-1 branch February 6, 2022 05:08
Ian288 pushed a commit to tackle-io/Zappa that referenced this pull request Jul 11, 2023
…ly (zappa#982)

* ADD: LambdaHandler outside
* UPDATE: apply to INSTANTIATE_LAMBDA_HANDLER_ON_IMPORT variable

Co-authored-by: Will Boyce <[email protected]>
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.