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

fix(sdk): inflight clients are not cached #5478

Merged
merged 17 commits into from
Jan 22, 2024
Merged

fix(sdk): inflight clients are not cached #5478

merged 17 commits into from
Jan 22, 2024

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Jan 20, 2024

Reuse the handler's inflight client across cloud.Function invocations, as long as they are running in the same sandbox.

This enables common FaaS optimizations such as caching database connections, etc.

To test this and improve the fidelity of our simulator, I've changed the behavior of cloud.Function in the simulator to reuse the function sandbox across invocations and just call handle() every time.

Checklist

  • Title matches Winglang's style guide
  • Description explains motivation and solution
  • Tests added (always)
  • Docs updated (only required for features)
  • Added pr/e2e-full label if this feature requires end-to-end testing

By submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.

Reuse the root inflight client across `cloud.Function` handler invocations.
@eladb eladb requested a review from a team as a code owner January 20, 2024 12:46
@monadabot
Copy link
Contributor

Thanks for opening this pull request! 🎉
Please consult the contributing guidelines for details on how to contribute to this project.
If you need any assistence, don't hesitate to ping the relevant owner over Slack.

Topic Owner
Wing SDK and utility APIs @chriscbr
Wing Console @ainvoner, @skyrpex, @polamoros
JSON, structs, primitives and collections @hasanaburayyan
Platforms and plugins @hasanaburayyan
Frontend resources (website, react, etc) @tsuf239
Language design @eladb
VSCode extension and language server @markmcculloh
Compiler architecture, inflights, lifting @yoav-steinberg
Wing Testing Framework @tsuf239
Wing CLI @markmcculloh
Build system, dev environment, releases @markmcculloh
Library Ecosystem @chriscbr
Documentation @hasanaburayyan
SDK test suite @tsuf239
Examples @skorfmann
Wing Playground @eladcon

@monadabot
Copy link
Contributor

monadabot commented Jan 20, 2024

Benchmarks

Comparison to Baseline ⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜
Benchmark Before After Change
version 75ms±1.43 73ms±0.29 -1ms (-1.99%)⬜
jsii_big.test.w -t sim 3391ms±25.93 3380ms±13.6 -11ms (-0.33%)⬜
jsii_big.test.w -t tf-aws 3482ms±9.27 3476ms±16.11 -6ms (-0.16%)⬜
functions_1.test.w -t sim 619ms±6.52 622ms±3.35 +3ms (+0.46%)⬜
functions_1.test.w -t tf-aws 1842ms±103.39 1825ms±89.89 -17ms (-0.9%)⬜
hello_world.test.w -t sim 625ms±8.76 626ms±5.2 +0ms (+0.05%)⬜
hello_world.test.w -t tf-aws 4819ms±33.13 4831ms±21.36 +12ms (+0.25%)⬜
functions_10.test.w -t sim 685ms±7.25 682ms±3.67 -3ms (-0.5%)⬜
functions_10.test.w -t tf-aws 4144ms±20.38 4176ms±25.45 +33ms (+0.79%)⬜
jsii_small.test.w -t sim 596ms±4.38 593ms±4.83 -3ms (-0.48%)⬜
jsii_small.test.w -t tf-aws 705ms±3.16 707ms±2.26 +2ms (+0.32%)⬜
empty.test.w -t sim 592ms±4.01 594ms±4.08 +2ms (+0.34%)⬜
empty.test.w -t tf-aws 706ms±7.54 702ms±4.43 -4ms (-0.52%)⬜

⬜ Within 1.5 standard deviations
🟩 Faster, Above 1.5 standard deviations
🟥 Slower, Above 1.5 standard deviations

Benchmarks may vary outside of normal expectations, especially when running in GitHub Actions CI.

Results
name mean min max moe sd
version 73ms 72ms 74ms 0ms 0ms
jsii_big.test.w -t sim 3380ms 3355ms 3415ms 14ms 19ms
jsii_big.test.w -t tf-aws 3476ms 3438ms 3517ms 16ms 23ms
functions_1.test.w -t sim 622ms 617ms 632ms 3ms 5ms
functions_1.test.w -t tf-aws 1825ms 1654ms 2068ms 90ms 126ms
hello_world.test.w -t sim 626ms 616ms 642ms 5ms 7ms
hello_world.test.w -t tf-aws 4831ms 4791ms 4880ms 21ms 30ms
functions_10.test.w -t sim 682ms 672ms 693ms 4ms 5ms
functions_10.test.w -t tf-aws 4176ms 4114ms 4237ms 25ms 36ms
jsii_small.test.w -t sim 593ms 582ms 609ms 5ms 7ms
jsii_small.test.w -t tf-aws 707ms 701ms 712ms 2ms 3ms
empty.test.w -t sim 594ms 587ms 604ms 4ms 6ms
empty.test.w -t tf-aws 702ms 694ms 715ms 4ms 6ms
Last Updated (UTC) 2024-01-22 07:00

monadabot and others added 2 commits January 20, 2024 15:04
Signed-off-by: monada-bot[bot] <monabot@monada.co>
@monadabot monadabot added the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 20, 2024
@eladb eladb removed the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 20, 2024
eladb and others added 4 commits January 20, 2024 21:13
@monadabot monadabot added the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 20, 2024
@eladb eladb requested a review from Chriscbr January 20, 2024 19:32
@eladb eladb removed the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 20, 2024
@eladb eladb requested a review from yoav-steinberg January 21, 2024 06:24
@yoav-steinberg
Copy link
Contributor

yoav-steinberg commented Jan 21, 2024

Haven't reviewed yet, but one thing to keep in mind is that cold/warm lambda invocation is an implementation detail.
To correctly simulate this we might want to randomly decide if we want to reuse the sandbox or create a new one on each invocation.
Or add a force-cold argument to invoke.

Copy link
Contributor

@yoav-steinberg yoav-steinberg left a comment

Choose a reason for hiding this comment

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

I think this is pretty elegant.
I'd like to better doc the behavior an perhaps consider modeling the cold invocation into the could.Function API.

libs/wingsdk/src/cloud/function.ts Show resolved Hide resolved
@monadabot monadabot added the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 21, 2024
@eladb eladb removed the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 21, 2024
Co-authored-by: yoav-steinberg <yoavs@monada.co>
monadabot and others added 2 commits January 22, 2024 08:48
Signed-off-by: monada-bot[bot] <monabot@monada.co>
@monadabot monadabot added the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 22, 2024
@eladb eladb removed the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Jan 22, 2024
Copy link
Contributor

mergify bot commented Jan 22, 2024

Thanks for contributing, @eladb! This PR will now be added to the merge queue, or immediately merged if eladb-patch-6 is up-to-date with main and the queue is empty.

@mergify mergify bot merged commit 132f90b into main Jan 22, 2024
15 checks passed
@mergify mergify bot deleted the eladb-patch-6 branch January 22, 2024 13:08
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.54.50.

eladb added a commit that referenced this pull request Jan 23, 2024
Following up on #5478 and [@Chriscbr's comment](https://github.com/winglang/wing/pull/5478/files#r1462456582),
verify that inflight clients are not shared across two separate instances of a cloud function.
mergify bot pushed a commit that referenced this pull request Jan 23, 2024
)

Following up on #5478 and [@Chriscbr's comment](https://github.com/winglang/wing/pull/5478/files#r1462456582), verify that inflight clients are not shared across two separate instances of a cloud function.

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
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.

4 participants