Repository navigation
Support Lambda Managed Instances #1065
Description
Activity
Re:
we can spawn a new async task for each invoke
(This is assuming this feature is only going to be implemented for tokio, initially).
Just as an implementation suggestion, we probably should make sure that we aren't dispatching work from the tokio
block_onthread (aka non-worker thread), but rather from a worker thread (ie inside a task).We don't control whether the caller awaits the lambda_runtime future on a worker thread or not, so it seems prudent to always spawn a task to drive the receive loop.
This is a performance optimization to avoid ping ponging the socket between the I/O driver on a worker thread, and the non-worker thread.
Another thing to think about would be, how does this fit into turnkey graceful shutdown handling. The simple answer is, just fire the graceful shutdown once, globally, but I imagine the nicer thing might be some signaling to allow each accept loop to clean up its own state.
new async task for each invoke
Correction (discussed offline): new task per event loop based on max concurrency, not per invoke
Reacted by Harold SunThanks for opening this; now I understand why my containerized function that uses the Lambda Rust Runtime breaks as soon as I attach a Capacity Provider to it.
I also noticed that AWS recently updated the Lambda docs with a section on building custom runtimes for Lambda Managed Instances, which helps clarify how custom runtimes need to be implemented in that setup..
Reacted by Alessandro BolognaReacted by Luciano MamminoI have pushed a (very) draft PR for review to implement concurrent requests and published a super simple demo/benchmark comparing a tiny 128MB rust function with a 2GB one on managed instances. Currently the lambda is just a hello world so it's not very useful as a real world example, but it's a start...
Reacted by Gary Sassano and Nick Angelou@alessandrobologna Thanks for the PR. I took a very brief look. One thing I would recommend to change: don't change the existing
runmethods in the runtimes, create additionalrun_concurrent()methods instead. This is because addingClone + Send + 'staticto run methods is a big breaking change. Most of the existing functions won't even compile without adding those traits.Reacted by Jess Izen and Alessandro BolognaThanks @bnusunny, that makes sense. I did (just briefly) consider either separate crates (maybe lambda_runtime_concurrent and lambda_http_concurrent) or a concurrent::run namespace, but I agree that your suggestion is the most practical.
I've updated the PR to add new *_concurrent entry points (lambda_runtime::run_concurrent, lambda_http::run_concurrent, and lambda_http::run_with_streaming_response_concurrent) and left the existing run functions (and their signatures/behavior) untouched. Concurrency is now opt‑in via the new APIs.Hi, (I guess the user has been deleted?), I couldn't really think of a way to make the current approach work without
Clone + Send + 'static.
In the concurrent code we spawn tasks per invocation, withtokio::spawn(async move { ... }), so the handler (and anything it captures) needs to beSend + 'staticon Tokio’s multithread runtime.
And it also needs to beClonebecause TowerService::calltakes&mut self. In concurrent mode, once you have multiple invocations in flight, a single service instance can’t be called concurrently from multiple tasks.But it's also very possible that there are other possible approaches that I am not thinking of.
@alessandrobologna Thanks for the updates. Two more comments from me:
- Ideally we should create
AWS_LAMBDA_MAX_CONCURRENCYnumbers of tasks and within each task to run an event loop pulling from Runtime API. The event loop could reuse the existing one with small updates. We don't really need two event loop implementations in the runtime. - Regarding the graceful shutdown handler, Lambda will only shutdown the sandboxes when they are idle. The existing graceful shutdown handler should work for LMI as well.
Reacted by Alessandro Bologna- Ideally we should create
@bnusunny very good points, thank you.
-
You are right, in the implementation as it is now, the concurrent code is reimplementing some of the same logic that already exists in the sequential loop, so it's a bit duplicative. Spawning N (the max concurrency) worker tasks and running one /next loop per worker is a cleaner design, and should let us reuse more of the existing event-loop code. I'm going to refactor in that direction.
-
I was thinking that in case of running with RIE or other local emulator we could have situations where there are in flight invocations, but you are right, it's probably over-engineered to handle scenarios that are unlikely to happen in the real world.
-
Hi,
I’ve updated the draft PR #1067, now it should be better aligned with the suggested new design:- The concurrent runtime now follows the recommended LMI model: it spawns AWS_LAMBDA_MAX_CONCURRENCY worker tasks, and each worker runs its own /next loop (no task-per-invocation approach).
- I refactored the runtime so sequential and concurrent paths share the same invocation processing logic, rather than maintaining two separate event loop implementations.
- I kept the graceful shutdown handler aligned with upstream (run the hook then exit), and removed the extra "drain in-flight invocations" coordination. In concurrent mode, the runtime still broadcasts shutdown on first worker infra error/panic so other workers stop polling and we join them before returning.
- Added a small basic-lambda-concurrent example and the
test-rie-lmimake target to make it easier to test locally.
Overall there are less changes now compared to main, which is good I guess. If you have a chance to give it another look, please let me know if it's going in the right direction.
@alessandrobologna Thanks for the update! It looks great. Only one problem, LMI handles worker failures differently: If a runtime worker crashes, the execution environment continues operating with the remaining healthy workers.
We don't really need the shutdown channel to broadcast the shutdown signal. With It, a single worker failure shuts down the whole runtime. We should just remove that to maintain the error isolation of workers.
We probably need a few tests to cover this behavior.
Reacted by Alessandro BolognaOk, good to know. :)
So we should:- remove the shutdown channel (it's currently only used to "kill every task on first failure").
- change the supervisor (run_concurrent_inner) so that a worker exiting is not fatal to the whole runtime.
Question for you... should we:
- just log the worker failure and keep running with fewer workers or
- log it and spawn a replacement worker (maybe with a small backoff to avoid tight crash loops)
And, in both cases, stil terminate if we can’t keep at least 1 worker alive (meaning we are crash-looping after retry/backoff threshold)?
Let's keep it simple and start with 1. 2 is better if the implementation is reliable and not complicated.
Yes, the runtime should shutdown if we can't keep at least 1 worker alive.
Reacted by Alessandro Bologna7 remaining items
This is released in release
1.1.0-rc1.This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.Ah, yes.
Indeed, we should keep this discussion open for feedback, and close once done.
Remaining work:
- Verify edge-cases
- Align on the implementation with relevant internal parties
There's been some offline discussion about possible improvements to the API for concurrent worker execution. Sharing to solicit community feedback - do you prefer the following option 1 or option 2 (or something else?)
For context, our concurrency implementation uses tokio task spawning. This requires
Send + Clonebounds. We don't want to narrow the current bounds ofrun().Option 1 (current rc):
- non-default concurrency feature flag adds
run_concurrent()api run_concurrent()api will still run serially ifAWS_LAMBDA_MAX_CONCURRENCYis not set- if
run()is used withAWS_LAMBDA_MAX_CONCURRENCY, logs a warning but runs serially
Option 2:
- non-default concurrency feature flag changes bounds of
run()to includeSend+Clone run()spawns multiple workers if both the concurrency feature flag andAWS_LAMBDA_MAX_CONCURRENCYare setrun()runs sequentially ifAWS_LAMBDA_MAX_CONCURRENCYis not set- if
run()is used withAWS_LAMBDA_MAX_CONCURRENCYbut not the concurrency feature flag, log a warning and run serially - (optional) add a builder that exposes a
sequential_handler()API that preserves the looser bounds
For either, we would like to see a
Runtime::builder()API. That allows unambiguously specifying a handler as sequential or possibly concurrent. That same API could also allow configuring something like whether to restart a worker task if it crashes. But, that would be a future enhancement that is semver-compatible.As a side note, we are thinking to rename the feature flag to
concurrent_tokioto leave open the door for alternative executors down the road. That relates to #984 - we would like to make thetokio::rtdependency opt-in since the core runtime API does not require it. Ultimately we want to support "bring your own executor" such as a thread per core async executor, or something backed by a synchronous threadpool like rayon.- non-default concurrency feature flag adds
Remaining work to move out of
-rcstatus (as I understand it):- Finalize preferred API for
run/run_concurrent(See previous message) - Log timeouts when the lambda data plane returns a timeout - in progress (Log timeouts when DP responds with timedout #1091)
(@FullyTyped are there other edge cases needing verification still?)
Reacted by Astraea- Finalize preferred API for
Nothing else from what I know.
Looking to finish the PR for timeouts today or tomorrow.
Reacted by Jess IzenI like the simplicity of option 2, my actual handler syntax stays the same. I just enable a crate feature and switch an environment variable. And enabling the crate feature would cause the compilation errors to force me to go and update my code based on the trait bounds.
I do like the explicit nature of option 1, but it just does like an extra step. In that, I'd have to do the same as I've described above, as well as also changing my handler code (albeit a single line code change).
The counter argument, is that option 1 is incredibly explicit. If you had a project that has some multi concurrent and some single concurrency Lambda functions having that explicit
run_concurrentAPI makes it really clear that as a developer you need to think about concurrency.I realize that isn't a firm opinion either way, I think I'm swaying towards the more explicit nature of Option 1.
+1 to the idea of supporting multiple async runtimes as well.
Reacted by Jess Izen, Harold Sun, Alessandro Bologna and RYOTARO HORIUCHIThis issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
AWS just launched Lambda Managed Instances What's New. It supports multiple concurrent requests on a single execution environment. We should support it to get the benefit to the Rust Runtime users.
To support multi-concurrency, we can spawn a new async task for each invoke. Both
lambda-runtimeandlambda-httpcrates need new run methods because the tower service instances need implementClonetrait. AddingClonetrait to the exiting run methods will be a breaking change.X-ray trace header propagation should not use the environment variable. We need use a thread-safe way to store the x-tray trace header for each request.