Repository navigation
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b25962b97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| conn, err := ln.Accept(ctx) | ||
| if err != nil { | ||
| if ctx.Err() != nil || errors.Is(err, quic.ErrServerClosed) { | ||
| return nil |
There was a problem hiding this comment.
Wait for HTTP/3 shutdown before returning
When ctx is canceled during an active HTTP/3 request, this Accept returns immediately and Serve runs its deferred tr.Close() without waiting for the concurrently running d.h3.Shutdown. In quic-go v0.63.0, Transport.Close abruptly terminates existing connections, so deploys or SIGTERM can cut off in-flight /v1/app/once requests instead of allowing the advertised five-second grace period. Synchronize with the shutdown goroutine before returning from Serve.
Useful? React with 👍 / 👎.
| if err := door.Serve(ctx, udp, tlsConfig); err != nil { | ||
| log.Error("HTTP/3 stopped; TCP serves on", "err", err) | ||
| } |
There was a problem hiding this comment.
Stop the service when the HTTP/3 listener fails
If Door.Serve exits with a fatal UDP or QUIC listener error, this path only logs it while the TCP server keeps emitting the unchanged Alt-Svc header. The inspected deploy/compose.yml health check can mark the container unhealthy, but the deployment documentation explicitly says that nothing restarts on that state, so clients can keep racing a dead HTTP/3 endpoint indefinitely. Propagate this error to terminate the process, or otherwise stop advertising HTTP/3.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
quicdoor serves h3 with the TCP handler and certificate, Alt-Svc points clients at it, ftwcloudd health checks both, and compose maps 443/udp. A stateless reset key in the state directory lets the next process reset an old connection after a restart. Past 200 new connections a second the door asks for a Retry, a handshake gets 5 s, and a UDP port that will not bind leaves TCP serving alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QUIC, step 1: the plain HTTPS routes answer over HTTP/3 on UDP 443 too. The watch's one-shot read gets one round trip shorter, and much shorter under loss. The sockets stay on TCP; QUIC for them is step 2, in paired PRs.
What changes
internal/quicdoorlistens for QUIC on the same address over UDP and hands each connection to the protocol its ALPN named. Today that is onlyh3, served by quic-go's HTTP/3 server with the same handler and certificate as TCP. It serves/v1/app/once,/v1/now,/healthzand/source. 0-RTT is off. Extended CONNECT, which quic-go always offers, never reaches a route. quic-go logs nothing.Alt-Svc: h3=":443"; ma=86400goes on TCP responses, but not on a WebSocket upgrade's.r.TLSis the service's own TLS 1.3 session. Its exporter equals the one the client computes on the same QUIC connection (TestTheExporterOverHTTP3IsTheClientsOwn). That is what the box's hello will bind to over QUIC in step 2.ftwcloudd health URLchecks/healthzover TCP and HTTP/3.compose.ymlmaps443/udpand runs it as the container's health check.ftwclouddkeeps a stateless reset key inquic-reset.keyin the state directory, so the next process resets an old connection's next packet (TestARestartResetsTheOldProcesssConnections: about 10 ms with the key; without it the connection still stands after 3 s).deploy/http3.mdcovers the security group's UDP 443, the order, how to check it and how to turn it off without a release (FTW_CLOUD_H3=false, whichftw-cloud-deploykeeps).bootstrap.shraisesnet.core.rmem_max/wmem_maxto the 7.5 MB quic-go asks for. I touched nothing in AWS.Numbers
The watch's one-shot read (
TestOnceReadTimes), from the first byte sent to the decrypted answer, in the real handler with a box on its uplink.scripts/netem-once.shruns it in a Linux container whose loopback carries netem: 20 ms each way, so a 40 ms round trip, and the loss applies to each packet. 200 reads per row. Cold means a new connection, as after the watch slept; warm means a reused one. TCP today offers only HTTP/1.1; HTTP/2 is there for comparison only.No read failed in any row. A cold read is three round trips over TCP (TCP, TLS, request) and two over QUIC. Under loss, TCP's tail is a lost SYN or handshake segment waiting out a one-second timer.
Apple's stack against this server (macOS 27, URLSession, loopback)
assumesHTTP3Capable, the first request goes over HTTP/3. With UDP refused, it fell back to HTTP/1.1 in the same time as without the flag. I did not measure a network that drops UDP silently.Alt-Svccannot pull the app's socket onto HTTP/3.Alt-Svcalone did not move URLSession to HTTP/3 on a port other than 443, even after the TCP connection was gone. I could not bind UDP 443 locally to try the real port. So the watch's direct path (one-door step 3) should setassumesHTTP3Capableand keep one URLSession. That needs no change here.Deploy order (Fredrik)
sg-098992ad382ecf94a(command indeploy/http3.md).bootstrap.shagain for the buffer sysctl.go run ./cmd/ftwcloudd health https://cloud.ftw.energy/healthzshould printHTTP/1.1 okandHTTP/3.0 ok.Opening the port first matters. A client that saw
Alt-Svcraces QUIC against TCP for up to a day, and with UDP shut QUIC always loses that race.Checks
go vet ./...,go test -race ./...,go test -tags ftwclouddev ./...,scripts/check-imports.sh: pass.TestTheSnapshotArrivesFast(appsession, unchanged) once went over its 100 ms on a laptop at load average 50; alone it passes.TestAWatchReadsItsHomeOverHTTP3: a read,token.getand/v1/nowover HTTP/3, with the box's bytes graded live. The sockets answer 4xx over HTTP/3.TestAltSvcIsOnPlainResponsesOnly,TestExtendedConnectNeverReachesARoute,TestAnUnknownProtocolIsRefused,TestNoEarlyData.443:443/udp).ftwcloudd healthpassed inside the container and from the host through the UDP mapping.Not here
/v1/boxand/v1/app, and 0-RTT: The uplink and the app session over QUIC, with 0-RTT a replay cannot use (QUIC steps 2–3) #22, stacked on this one and paired with FTW Cloud over QUIC: the protocol, and the box's uplink racing the WebSocket with 0-RTT (QUIC steps 2–3) ftw#1653 and iPhone: FTW Cloud over QUIC, racing the WebSocket (QUIC step 2) ftw-app#74.HTTPSrecord. Leave it out: it would tell the sockets' clients to try QUIC as well.About 230 lines of service Go (
quicdoor,health.go, the flag).🤖 Generated with Claude Code