Concurrency option for warming up X instances of of a lambda.

[ejwilburn]

The default concurrency value is 1 and can be set in custom.warmup.concurrency for all lambdas and can be overridden by specifying a warmupConcurrency value at the function definition level. I'm not super knowledgeable on YAML, node or developing serverless plugins so I wasn't sure if using .warmup.concurrency would work since .warmup is already being used as a specific config value (true/false/stage) rather than a config section.

Also, another value is passed along with the source to both the context and payload of the call, concurrencyIndex. That's added in case someone wants special handling for the first X concurrent instances.

I've tested that this is working and is calling my lambda functions but using the AWS SDK to invoke the lambda instead of invoking through the API gateway really isn't helping for my use case which is a public REST API that's also in a VPC. Apparently this method of warming isn't warming everything up completely.

I thought about updating this further to check if the function uses the API gateway then call the external endpoint in order to warm it (or maybe use APIGateway.testInvokeMethod()) but I'm not sure I want to tackle that right now.

[ejwilburn]

FYI I confirmed that using APIGateway.testInvokeMethod() does properly warm up relevant lambdas and related resources. Unfortuantely it requires the API Gateway resource Id of the lambda function (as well as the API Gateway Rest API ID) and I don't see an easy way to get that resource id from serverless. You can use the AWS SDK to get it by looping through all resources for a given REST API though, so I'll probably work on adding that to this as well.

[marcfielding1]

Nice I never got time to jump on, good work!

[ejwilburn]

Turns out the issue I had with lambdas not being warmed up fully had nothing to do with invocation of the lambdas directly vs. through the API gateway. I had some initialization code that was below the check to see if this was a warmup and then exiting. I moved that code up above the warmup check and that fixed the issue.

That was of course after I spent the entire day adding code to invoke the lambdas through the API gateway and spent hours trying to figure out why it warmed up if my warmup detection failed but didn't warm up if my warm-up detection code worked. Grrr...

FYI, using AWS Secrets Manager in a Java lambda adds several seconds to the initial warming up process. With a 1GB memory lambda it takes about 4 seconds just to get a secret. Going up to 1.5GB takes it down to ~2 and 3GB gets it down to ~1 or less. Not sure what's going on with that library but it's not good. Another dev reporting it here: https://forums.aws.amazon.com/thread.jspa?threadID=288501&tstart=0

[ejwilburn]

So, in summary, this code should be good to go unless you find issues.

[goncaloneves]

Awesome contribution. Will have a look this week. 😎👍

…

On Sun, 25 Nov 2018, 01:18 E.J. Wilburn ***@***.*** wrote: So, in summary, this code should be good to go unless you find issues. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#81 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGO-8ZMAsKFFb8cAvc06bTuEB84GYjqWks5uyhotgaJpZM4YxPT7> .

[juanjoDiaz]

How is this working exactly? Simply a calling a lambda 5 times won't give you 5 lambdas. AWS reuses containers. There was quite a bit of discussion about that in the relevant issue.

• Code structure and quality should be improved.
• Source being a string but treated as object should be reviewed
• There is no documentation on how to actually use this feature.

[ejwilburn]

How this works is that all concurrent lambda calls for a specific function are initiated simultaneously. If a call is made to a lambda function while that function is still executing it spawns a new instance of that function. In your lambda handler you should have a check to see if the function was invoked by this plugin, then delay the return a small amount. In my testing just doing a Thread.sleep() for 25ms was plenty. The concurrencyIndex that's passed to the invocation of the lambda is there in case you want to have a longer or shorter delay or different processing based on the instance, say quicker return for the first instance.

I've tested and confirmed that this is spawning the correct number of lambda function instances.

I did add some basic documentation to the readme but I agree more needs to be added.

[ejwilburn]

Probably be a couple of days before I have time.

[ejwilburn]

Cleaned things up as requested, added more documentation and removed concurrencyIndex because I don't think it's really necessary right now.

[juanjoDiaz]

Thanks for keeping up!

Just few more comments 🙂

[goncaloneves]

@ejwilburn @juanjoDiaz Are we happy about the changes? Did we perform tests on the logic?

[juanjoDiaz]

Sorry about the delay answering. A bit swamped with other stuff lately.

I'm happy with the functionality (although I haven't tested it).

However, I don't like the warmupConcurrency field. I'm planning to do a PR, hopefully before the end of the week that adds the possibility to use functionObject.warmup.concurrency.

[goncaloneves]

I love the fact @ejwilburn actually implemented the logic we spoke about but without test results, solution may be good only on paper and there is no reason to push it forward.

@ejwilburn can you shed some light on results on the first warming vs sustained warming?

@juanjoDiaz You're right, prop name should be named the same between service and function, otherwise it looks like it's another option.
SLS reserved concurrency it's called reservedConcurrency. Following the same logic we could go with warmupConcurrency on both, even though concurrency is available too.

[juanjoDiaz]

My concern is not so much about the name. warmupConcurrency or concurrency is not so relevant.

My main concern is warmupConcurrency being out of the warmup object.
So instead of

functions:
  foobar:
    warmup:
      default: true
      concurrency: 2

you have to do:

functions:
  foobar:
    warmup:
      default: true
    warmupConcurrency: 2

[goncaloneves]

I didn't even see that. Yes, inside like all other props.

[ejwilburn]

I didn't even see that. Yes, inside like all other props.

I agree with you but I've explained why that currently won't work without refactoring how functionObject.warmup is used. See my comments here: #81 (comment)

[ejwilburn]

@ejwilburn can you shed some light on results on the first warming vs sustained warming?

I've tested both manually and confirmed the correct number of lambda containers were instantiated (5 in this case) by checking the number of log streams created for that lambda function and confirmed that successive warming calls to that lambda split the calls to each concurrent container, 1 per instance by verifying the in the log stream.

I've also tested raising the concurrency level by editing the generated warmup function and modifying the concurrency value direct, re-running it and validating that the additional concurrent instances were spawned and created their own log streams.

[juanjoDiaz]

The issue with warmupConcurrency (#81 (comment)) should be easily fixed as soon as #82 is merged.

[ejwilburn]

Ah, good, when the other PR gets merged I'll merge and update my code.

[goncaloneves]

@juanjoDiaz @ejwilburn merged #82.

[ejwilburn]

Updated for the function level config changes.

[goncaloneves]

@ejwilburn can you shed some light on results on the first warming vs sustained warming?

I've tested both manually and confirmed the correct number of lambda containers were instantiated (5 in this case) by checking the number of log streams created for that lambda function and confirmed that successive warming calls to that lambda split the calls to each concurrent container, 1 per instance by verifying the in the log stream.

I've also tested raising the concurrency level by editing the generated warmup function and modifying the concurrency value direct, re-running it and validating that the additional concurrent instances were spawned and created their own log streams.

@ejwilburn Thanks for clarifying and updating code.
I am curious, holding event loop for 25ms is enough to cold start a new container?

I appreciate your great help on this matter.

[juanjoDiaz]

Looks great now!

[ejwilburn]

@ejwilburn Thanks for clarifying and updating code.
I am curious, holding event loop for 25ms is enough to cold start a new container?

I appreciate your great help on this matter.

@goncaloneves In my testing it was, yes.

[marcfielding1]

You like to merge it merge it, you like to merge it merge it, we like to merge it! Common choppy chop I have a customer that really needs this!

[ejwilburn]

👍

Gotta be merged by @juanjoDiaz or someone with write access.

[marcfielding1]

Hehe, yeah no worries - I'm quite excited about this as I can actually test it in a production/real environment for ya on a pretty big project.

[goncaloneves]

Thanks for this extraordinary effort @ejwilburn @juanjoDiaz 🙏

I will be merging now and will try to execute tests this week for the latest refactor and multiple warmup so we can release this asap.

[marcfielding1]

Sorry to bump a closed thread(hopefully it won't reopen, sorry if it does) - I'm adding this as a dep to a professional PoC tomorrow and will let you know if we hit any issues.

[juanjoDiaz]

Thanks!

Any feedback is more than welcome!

[goncaloneves]

@marcfielding1 Actually makes sense to reopen this until it's released to continue open conversation.

I am curious how did it work the PoC?

[goncaloneves]

@marcfielding1 let's continue conversation in #24 👍