-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Experimental IETF-standard HTTP/3 support #2727
Conversation
Also use latest quic-go for ALPN fix
@marten-seemann This is almost working all the way. Last thing to do is figure out how to handle config reloads gracefully. I need a couple things for that to happen:
There is one more problem, too, which I'm hoping you'll have insights on. It only arises when Can you replicate the error? First, I did use a
Then I used this Caddy config in a file called
Then from this branch I run Caddy 2:
Then in another terminal I run the HTTP/3 client:
It should reply with "Hello, world!" But then reload the config with (this command forces a reload with the Cache-Control header, which is necessary unless you've changed the config):
(don't stop the running Caddy process, make sure this reload happens while it's still running) Then try the client again. It should hang and panic:
Now, stop Caddy and run it again but set In other words, I can only replicate the hanging if Unfortunately, the only way I can get this PR finished and merged, is if we can gracefully apply new configuration. So any help would be appreciated! :) |
Hi @mholt, I just tried to reproduce this, but I'm failing at the very first step. The output is
Caddy is listening on localhost TCP 2443, but apparently not on the corresponding UDP port. Is there anything else I need to do to enable QUIC? |
@marten-seemann Thanks for taking a look! I'm not sure what happened, but somehow this part of the config got left out of my paste:
This enables the default HTTPS configuration for that server. I've updated my post to include the proper config which I have tested will enable the HTTP/3 listener. Sorry for the mishap! Let me know how that works. |
Hi @mholt, I finally had the time to dig into this. The problem seems to be the Apparently, after applying the new config, you're passing a new |
Looks like this is not Caddy's fault after all. The problem seems to be that we're not correctly recognizing the same packet conn being passed to quic-go. |
Phew, thanks for all the investigative work @marten-seemann ! I think some good improvements have been made. Unfortunately, though, after pulling the latest master of quic-go and building Caddy with it, I still see the hanging / panic from the client after shutting down the old server and starting a new one with the same underlying PacketConn. Also, even though I thought that not Close()'ing the old server worked, it seems to cause problems; with more trials I found that the old servers were responding to some requests with the old configuration (which makes sense). So I can't use that as a workaround and thus will be Close()'ing the http3.Servers. So, with that said, I've found out something else which may be helpful to you: With debug mode enabled on the server, sometimes requests do work after doing a config reload. However, the requests often take much longer at least for the first several seconds. BEFORE a config reload, when things work peachy, the server's output is about 150 lines long:
AFTER the config reload, the debug output is over 100 lines longer, indicating that the server (and client) have to do a lot more work to make the connection succeed:
You'll notice that in the second case, first the server reports an unexpected connection ID, and does considerably more work than the first requests before the reload. (The second request, after the reload, takes several seconds -- noticeably slower than the first.) I hope the difference between these logs will be helpful. It is still difficult to reproduce the bug with debug mode enabled, but it happens pretty much every time for me with the server's debug mode off. |
WAIT A SECOND. I forgot to apply my git stash, which had a crucial Considering how easily and often I was able to reproduce the bug, it seems that I can't reproduce it anymore. Nice work, yo. 😎 Whatever you committed to master last night must have made things better. Thank you!! UPDATE: Okay actually I've been able to reproduce it twice. 🤔But it took a lot more trials. I've updated this PR/branch with my latest code. The current commit does not close the servers, for better or for worse. |
# Conflicts: # go.mod # go.sum
@marten-seemann To summarize: the issue persists, but by not closing the http3.Server, the issue is either resolved or it hides itself. I will make HTTP/3 off by default but configurable in a commit later this morning, and then merge this in, however, before HTTP/3 can be on by default we'll need to get to the bottom of what is causing the issue that I think we've made some progress on but is still not totally resolved. Thanks for your hard work! Let me know how I can help. |
1. What does this change do, exactly?
Adds IETF HTTP/3 functionality to Caddy.
2. Please link to the relevant issues.
Closes #2450
3. Which documentation changes (if any) need to be made because of this PR?
Add HTTP/3 to our ad-hoc docs for Caddy 2.
4. Checklist