quic-go icon indicating copy to clipboard operation
quic-go copied to clipboard

Implement graceful shutdown for http3 servers.

Open WeidiDeng opened this issue 1 year ago • 4 comments

Fix 6195 for caddy.

WeidiDeng avatar Apr 03 '24 03:04 WeidiDeng

@marten-seemann Do you have time to take a look at this draft?

WeidiDeng avatar Apr 05 '24 01:04 WeidiDeng

Codecov Report

Attention: Patch coverage is 43.13725% with 116 lines in your changes are missing coverage. Please review.

Project coverage is 84.59%. Comparing base (e2fbf3c) to head (312a3d0).

Files Patch % Lines
http3/client.go 35.71% 42 Missing and 3 partials :warning:
http3/server.go 61.17% 32 Missing and 8 partials :warning:
http3/frames.go 0.00% 28 Missing :warning:
http3/conn.go 0.00% 2 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4407      +/-   ##
==========================================
- Coverage   85.15%   84.59%   -0.56%     
==========================================
  Files         154      154              
  Lines       14794    14990     +196     
==========================================
+ Hits        12597    12680      +83     
- Misses       1690     1791     +101     
- Partials      507      519      +12     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 07 '24 02:04 codecov[bot]

@marten-seemann When do you think this patch is ready to be reviewed? I haven't updated it in a while, and several of those enhancements already landed in the latest release. If you think it's ok to add graceful shutdown, I'll update this patch against the latest release.

WeidiDeng avatar Aug 07 '24 03:08 WeidiDeng

Sorry, I dropped the ball on this PR. Let's get this into the next release! I'll review it later.

marten-seemann avatar Aug 07 '24 04:08 marten-seemann

I refactored the tests in https://github.com/quic-go/quic-go/commit/e933c80ada58eba408722e912f0a9cf094f27ecf. Also, it looks like we should probably merge #4689 first. Does this make sense @WeidiDeng?

marten-seemann avatar Oct 08 '24 15:10 marten-seemann