Skip to content

Remove tokio rt-multi-thread feature from lambda_runtime and lambda_extension, and possibly rt as well #984

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

Open
jlizen opened this issue May 6, 2025 · 4 comments

Comments

@jlizen
Copy link
Contributor

jlizen commented May 6, 2025

Currently the lambda_runtime crate has an unconditional dependency on tokio withe rt-multi-thread feature enabled:

I'm struggling to see anywhere this dependency is actually used. Meanwhile, given that lambdas only receive one request at a time, it will frequently make sense for callers to only want to use the current thread runtime anyway (ref: #985 ).

I don't think we even need the rt (not multithreaded) feature for lambda-runtime, not seeing anywhere we are spawning or interacting with feature gated APIs. Though, for #983 we would need it, so probably ok to leave in?

Anyway, is there a reason we need to depend on the multithreaded runtime feature? Am I missing something?

@jlizen jlizen changed the title Consider removing implicit enablement of tokio rt-multi-thread feature in lambda_runtime and lambda_extension Consider removing implicit enablement of tokio rt-multi-thread feature in lambda_runtime and lambda_extension, and possibly rt May 6, 2025
@jlizen
Copy link
Contributor Author

jlizen commented May 6, 2025

On thinking more about it, I don't think I actually need a task::spawn for #983 , I would rather just return a future and let the caller decide.

That leaves no usages of tokio::rt/task whatsoever in aws-lambda-rust-runtime, we could completely ditch the feature. And then for lambda_extension, we are implicitly spawning some background loops for our telemetry and log processor, which is a bit awkward. We could return a future but definitely would be semver breakage.

Anyway probably ripping out any of these features in semver breakage, since if somebody was implicitly getting them enabled by aws-lambda-rust-runtime, their code could break.

So presumably this would all require a new minor version (since we are pre-1.0)

@jlizen jlizen changed the title Consider removing implicit enablement of tokio rt-multi-thread feature in lambda_runtime and lambda_extension, and possibly rt Remove tokio rt-multi-thread feature from lambda_runtime and lambda_extension, and possibly rt as well May 6, 2025
@jlizen
Copy link
Contributor Author

jlizen commented May 6, 2025

Ok, I do need tokio::rt for graceful shutdown handling, at least the easy way. The listener panics if used outside of rt - ref. But anyway I can just put the whole thing behind a feature flag.

So I think that the larger point remains wrt to removing rt dependency from base crate.

It might actually simplify the migration path to have the feature flag for tokio/tokio-rt/graceful-shutdown/etc. Since we could make it a default feature. Makes people less likely to break if they don't change their tokio dependency.

@bnusunny
Copy link
Contributor

bnusunny commented May 8, 2025

Even through one Lambda sandbox only receives one request at time, nothing prevents people starting multiple async tasks to handle that request. I think you also reached the same conculsion at #985.

@jlizen
Copy link
Contributor Author

jlizen commented May 16, 2025

Even through one Lambda sandbox only receives one request at time, nothing prevents people starting multiple async tasks to handle that request. I think you also reached the same conclusion

That may be, but it still seems awfully infectious to enable rt-multi-thread when we aren't actually using it for anything? It is the caller that needs to instantiate the runtime anyway. Most of the time, they should be using rt-multi-thread, yes, but they could also be using non-tokio runtimes or a more esoteric current-thread runtime where they are careful about managing concurrency.

For the purposes of feature hygiene, shouldn't we avoid enabling that flag that we aren't directly using?

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

No branches or pull requests

2 participants