[go: up one dir, main page]

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

[Bug]: Http Keep-alive not enabled #1494

Open
1 task done
hamiltop opened this issue Dec 25, 2023 · 4 comments
Open
1 task done

[Bug]: Http Keep-alive not enabled #1494

hamiltop opened this issue Dec 25, 2023 · 4 comments
Labels
S-Investigation This issue needs further investigation or design to figure out a solution T-Bug Something isn't working

Comments

@hamiltop
Copy link

What happened?

Http Keep-alive seems to not be enabled in deployment. Running locally it handles keep-alive just fine, but in deployment it does not.

Version

v0.35.2

Which operating system(s) are you seeing the problem on?

In deployment

Which CPU architectures are you seeing the problem on?

In deployment

Relevant log output

No response

Duplicate declaration

  • I have searched the issues and there are none like this.
@hamiltop hamiltop added S-Triage Awaiting decision for what to do T-Bug Something isn't working labels Dec 25, 2023
@oddgrd
Copy link
Contributor
oddgrd commented Jan 4, 2024

Hey @hamiltop! To clarify, enabling keep-alive on your server does not work in deployment, but it works locally? I suspect this is due to your application being behind our proxies in deployment, and the proxies don't have keep_alive enabled. Whether they should warrant further investigation, though.

@hamiltop
Copy link
Author
hamiltop commented Jan 4, 2024

That would make sense. I would strongly suggest enabling keep-alive on the proxies, both from client->proxy and proxy->server. Throughput is much better due to reduced tcp connection overhead (and may be even more important for HTTPS efficiency).

@oddgrd oddgrd added S-Investigation This issue needs further investigation or design to figure out a solution and removed S-Triage Awaiting decision for what to do labels Jan 5, 2024
@oddgrd
Copy link
Contributor
oddgrd commented Jan 5, 2024

Thanks for the suggestion @hamiltop! We'll need to do some investigation to explore potential drawbacks, as well as what to set it to and where to set it, but it makes sense! If you have thoughts on that, it would be great if you could expand on it a bit, if you have the time.

For context, requests to your service first go through an AWS LB, then to a proxy in shuttle-gateway, then to a proxy in shuttle-deployer (your project container) and on to your application. You can find the code for our proxies in gateway/src/proxy.rs and deployer/src/proxy.rs if that interests you.

@hamiltop
Copy link
Author
hamiltop commented Jan 5, 2024

I dug in a bit and found that hyper-reverse-proxy removes the connection header which is valid for http 1.1. That does break keep-alive for http 1.0. Maybe http 1.0 isn't that big of a deal. It only showed up on my radar because I tried using apache bench (ab) to benchmark things and it's an http 1.0 client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Investigation This issue needs further investigation or design to figure out a solution T-Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants