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

Enable HTTP/3 by default #845

Closed
ghassanmas opened this issue May 27, 2023 · 3 comments · Fixed by #864
Closed

Enable HTTP/3 by default #845

ghassanmas opened this issue May 27, 2023 · 3 comments · Fixed by #864
Assignees
Labels
enhancement Enhancements will be processed by decreasing priority

Comments

@ghassanmas
Copy link
Member

ghassanmas commented May 27, 2023

Is your feature request related to a problem? Please describe.

From caddy version 2.6 it allows/uses http3 by default, which was experimental.
To utilize http3, docker compose would need to bind to udp, given the http3/quic new protocal is built on top of UDP1.

Describe the solution you'd like
Expected code change to allow such feature

ports:
- "{{ CADDY_HTTP_PORT }}:80"
{% if ENABLE_HTTPS and ENABLE_WEB_PROXY %}- "443:443"{% endif %}

 ports: 
   - "{{ CADDY_HTTP_PORT }}:80" 
   {% if ENABLE_HTTPS and ENABLE_WEB_PROXY %}- "443:443"{% endif %} 
   {% if ENABLE_HTTPS and ENABLE_WEB_PROXY %}- "443:443/udp"{% endif %} 

Which shall be rendered to assuming https is enabeld:

 ports: 
   - "80:80" 
   -"443:443"
   -"443:443/udp"

Describe alternatives you've considered

  • Add a patch to be able to overide, caddy service prots section.

Additional context

Footnotes

  1. https://en.wikipedia.org/wiki/HTTP/3

@ghassanmas ghassanmas changed the title Enable http3 by default Enable HTTP/3 by default May 27, 2023
@regisb
Copy link
Contributor

regisb commented May 29, 2023

Hi @ghassanmas! From a theoretical perspective, I understand the point of supporting http3. But what I do not know is the actual performance benefit that it will bring to Open edX. Did you run some tests to benchmark the impact?

@regisb
Copy link
Contributor

regisb commented May 29, 2023

Initial tests using Google's PageSpeed Insights show significant improvements with http/3.

Without http/3 (current implementation):

Screenshot from 2023-05-29 12-52-18

With http/3:

Screenshot from 2023-05-29 12-53-00

In particular, total blocking time is reduced by 55% (510 -> 230 ms), so this is pretty big.

Based on these figures, I agree with you that we should support http/3. But we will need to add support to Kubernetes as well, and we will need to add instructions for end users to make the most use of it.

@regisb regisb added the enhancement Enhancements will be processed by decreasing priority label May 29, 2023
@ghassanmas
Copy link
Member Author

@regisb for your previous question, I think you were faster than me in doing plain tests ;), and for why we should enable HTTP/3, another point is that for IETF, HTTP/3 standard holds same status as for HTTP/2 which is proposed standrad.

Regarding meaning of proposed standard:

A Proposed Standard specification is generally stable, has resolved
known design choices, is believed to be well-understood, has received
significant community review, and appears to enjoy enough community
interest to be considered valuable. However, further experience
might result in a change or even retraction of the specification
before it advances. ref 1

So my thinking was if we support HTTP/2 by default, then logically we shall support HTTP/3 assuming we follow the IETF criteria.

But we will need to add support to Kubernetes

I think probably it would need to add extra port for caddy options, where the protocol is specified, to my qick investagation k8s as well use TCP protocol by deafult2, so propaly a new port entry for 443 while speciing the protocol is needed. Though I haven't tested this, nor do I have a matrue expirence in regard of tutor/k8s.

we will need to add instructions for end users to make the most use of it

Thinking about this, I think this would be relevant for people who won't be using the default deployment options for tutor, of which I can think of two main scinarios:

  1. Running behind web proxy ref https://docs.tutor.overhang.io/tutorials/proxy.html
  2. Using different k8s template for caddy than the default tutor/templates/k8s/deployments.yml

For the 1. I think it would depend which proxy server opreator would choose (caddy, nginx, apache..etc). i think http/3 is not supported using the deafult config for the latter as is the case of caddy. Nginx offcial doc recommend rebuilding the binary,3 Other than that, for nginx, there seems to be an nginx module built by cloudflare to support it 4.

For 2, I think I might not be in a good position to determine what/how, but I think if typically opreators use ingress for nginx, then probably the above note is relevant. For spefically ingress-nginx I found there is stalled issue here

Footnotes

  1. RFC 2026 The Internet Standards Process https://datatracker.ietf.org/doc/html/rfc2026#section-4.1.1

  2. Protocols for Services K8s docs https://kubernetes.io/docs/reference/networking/service-protocols/

  3. Nginx support of HTTP/3 https://www.nginx.com/blog/our-roadmap-quic-http-3-support-nginx/

  4. Cloudflare Nginx module for quic/http/3 https:/cloudflare/quiche/tree/master/nginx

regisb added a commit that referenced this issue Jun 23, 2023
It was observed that waiting time was cut in half after http/3 was
enabled. Plus, supporting http/3 is super easy :)

Close #845
@regisb regisb self-assigned this Jun 23, 2023
regisb added a commit that referenced this issue Aug 3, 2023
It was observed that waiting time was cut in half after http/3 was
enabled. Plus, supporting http/3 is super easy :)

Close #845
moonesque pushed a commit to edSPIRIT/tutor that referenced this issue Nov 20, 2023
It was observed that waiting time was cut in half after http/3 was
enabled. Plus, supporting http/3 is super easy :)

Close overhangio#845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancements will be processed by decreasing priority
Projects
Development

Successfully merging a pull request may close this issue.

2 participants