-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
Optimize reverse proxy #40
Comments
I've been using |
need to do some better measuring but it looks liek latency is already < 1ms so it might not be a big deal, especially with each Lambda container having a concurrency of 1 haha. Might as well check for some quick wins though :D |
FWIW The product I'm launching on |
would it be reasonable to add opting out of the reverse proxy as an option? |
@StoneCypher It's mandatory for now, since it relays events to the app you deploy. Honestly though it only introduces ~500μ, so it's nothing to worry about |
Looks like this has been deprioritized, but wanted to add some thoughts in case anyone revisits. Careful with unix domain sockets, you need to always spawn a random one to use - since containers are reused (and frozen), you have to always choose a different unix domain socket to prevent reuse errors. I think switching to TCP / Unix domain sockets is a huge win over stdin / stdout simply because it eliminates oddness with accidentally logging to stdout. Eliminating that gotcha is worth way more valuable than speed IMO - plus TCP stacks are pretty dang fast these days. Last point - if you're looking to tighten up speed, my spidey-sense tells me that the big win will be in eliminating the extra marshal / unmarshal pass that happens in the node.js shim (over stdout). Writing the reverse proxy in a language that is natively supported by lambda is going to have the biggest win is my intuition. (unless of course you're just banking on official go support). And this might be sad, but I believe aws-lambda-go-shim uses a shared memory technique which is going to blow all of them out of the water. |
@skabbes FWIW that stdout weirdness only really applies to the up proxy, at least no one else has to be aware of it (thank god haha). Latency seems fine as-is but I wouldn't be too surprised if the serialization for big requests would be worth getting rid of, seems negligible for average stuff. Cold start is probably another story, it's ok but nothing fantastic, I read the post with their Python bindings and it seems cool but definitely more involved than the simple shim. Reinvent is soon too, I'm banking on a whole bunch of new features. Curious to see what Go support would look like, installing pkgs takes longer than uploading a binary, so official support mightttt not help much. |
@tj Ahh, ya I was confusing with go-apex, since I think you do have to be aware of stdout there. FWIW, as a user of up in production, I love that I have the ability to read all the code and understand exactly what is going on. External reverse proxies, or things getting too magic would turn me off a bit.
I'd imagine that they'll just load your compiled code as a plugin, and put the compile requirements on you, so you get the same kind of speed. Pretty much, I think they'd just eliminate the need for the shim entirely. I'd even more love for them to add streaming upload and download support to APIGateway. |
Oh man yeah, I'm sure that'll be comforting for a lot of people, and not relying on some little PaaS startup is a nice benefit too. Trueee good point I forgot about plugins, that's a great use-case for it, seems like it'd slim down quite a bit:
|
Do some profiling and get the latency as low as possible, possible replace Go's reverse proxy with https://github.com/valyala/fasthttp, test unix domain sockets in Lambda as well, check heap and see what can be pooled. It's also making some assumptions about having concurrency of 1 at the moment, since that's the case with Lambda.
Make sure it's not decompressing gzip and recompressing either, not sure what the proxy does but I know net/http's client does this transparently, test with some larger responses.
The text was updated successfully, but these errors were encountered: