@marcel91200 / caddy Mirror · read-only / commits / da8b7fe5
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 13, 11:45 UTC

caddyhttp: Honor grace period in background (#5043)

* caddyhttp: Honor grace period in background This avoids blocking during config reloads. * Don't quit process until servers shut down * Make tests more likely to pass on fast CI (#5045) * caddyhttp: Even faster shutdowns Simultaneously shut down all HTTP servers, rather than one at a time. In practice there usually won't be more than 1 that lingers. But this code ensures that they all Shutdown() in their own goroutine and then we wait for them at the end (if exiting). We also wait for them to start up so we can be fairly confident the shutdowns have begun; i.e. old servers no longer accepting new connections. * Fix comment typo * Pull functions out of loop, for readability

Matt Holt committed Sep 19, 2022 at 21:54 UTC da8b7fe58f83012d9a6c6e15cb249ca5f476597c
9 files changed +78 -10
caddy.go
+11
@@ -31,6 +31,7 @@ import (
31 "strconv"
32 "strings"
33 "sync"
34 + "sync/atomic"
35 "time"
36
37 "github.com/caddyserver/caddy/v2/notify"
@@ -676,6 +677,10 @@ func Validate(cfg *Config) error {
677 // Errors are logged along the way, and an appropriate exit
678 // code is emitted.
679 func exitProcess(ctx context.Context, logger *zap.Logger) {
680 + // let the rest of the program know we're quitting
681 + atomic.StoreInt32(exiting, 1)
682 +
683 + // give the OS or service/process manager our 2 weeks' notice: we quit
684 if err := notify.Stopping(); err != nil {
685 Log().Error("unable to notify service manager of stopping state", zap.Error(err))
686 }
@@ -739,6 +744,12 @@ func exitProcess(ctx context.Context, logger *zap.Logger) {
744 }()
745 }
746
747 +var exiting = new(int32) // accessed atomically
748 +
749 +// Exiting returns true if the process is exiting.
750 +// EXPERIMENTAL API: subject to change or removal.
751 +func Exiting() bool { return atomic.LoadInt32(exiting) == 1 }
752 +
753 // Duration can be an integer or a string. An integer is
754 // interpreted as nanoseconds. If a string, it is a Go
755 // time.Duration value such as `300ms`, `1.5h`, or `2h45m`;
caddytest/caddytest.go
+1 -1
@@ -43,7 +43,7 @@ type Defaults struct {
43
44 // Default testing values
45 var Default = Defaults{
46 - AdminPort: 2019,
46 + AdminPort: 2999, // different from what a real server also running on a developer's machine might be
47 Certifcates: []string{"/caddy.localhost.crt", "/caddy.localhost.key"},
48 TestRequestTimeout: 5 * time.Second,
49 LoadRequestTimeout: 5 * time.Second,
caddytest/integration/caddyfile_test.go
+4
@@ -16,6 +16,7 @@ func TestRespond(t *testing.T) {
16 {
17 http_port 9080
18 https_port 9443
19 + grace_period 1
20 }
21
22 localhost:9080 {
@@ -37,6 +38,7 @@ func TestRedirect(t *testing.T) {
38 {
39 http_port 9080
40 https_port 9443
41 + grace_period 1
42 }
43
44 localhost:9080 {
@@ -86,6 +88,7 @@ func TestReadCookie(t *testing.T) {
88 {
89 http_port 9080
90 https_port 9443
91 + grace_period 1
92 }
93
94 localhost:9080 {
@@ -109,6 +112,7 @@ func TestReplIndex(t *testing.T) {
112 {
113 http_port 9080
114 https_port 9443
115 + grace_period 1
116 }
117
118 localhost:9080 {
caddytest/integration/handler_test.go
+1
@@ -13,6 +13,7 @@ func TestBrowse(t *testing.T) {
13 {
14 http_port 9080
15 https_port 9443
16 + grace_period 1
17 }
18 http://localhost:9080 {
19 file_server browse
caddytest/integration/map_test.go
+1
@@ -13,6 +13,7 @@ func TestMap(t *testing.T) {
13 tester.InitServer(`{
14 http_port 9080
15 https_port 9443
16 + grace_period 1
17 }
18
19 localhost:9080 {
caddytest/integration/reverseproxy_test.go
+6
@@ -18,6 +18,7 @@ func TestSRVReverseProxy(t *testing.T) {
18 {
19 "apps": {
20 "http": {
21 + "grace_period": 1,
22 "servers": {
23 "srv0": {
24 "listen": [
@@ -50,6 +51,7 @@ func TestSRVWithDial(t *testing.T) {
51 {
52 "apps": {
53 "http": {
54 + "grace_period": 1,
55 "servers": {
56 "srv0": {
57 "listen": [
@@ -115,6 +117,7 @@ func TestDialWithPlaceholderUnix(t *testing.T) {
117 {
118 "apps": {
119 "http": {
120 + "grace_period": 1,
121 "servers": {
122 "srv0": {
123 "listen": [
@@ -156,6 +159,7 @@ func TestReverseProxyWithPlaceholderDialAddress(t *testing.T) {
159 {
160 "apps": {
161 "http": {
162 + "grace_period": 1,
163 "servers": {
164 "srv0": {
165 "listen": [
@@ -239,6 +243,7 @@ func TestReverseProxyWithPlaceholderTCPDialAddress(t *testing.T) {
243 {
244 "apps": {
245 "http": {
246 + "grace_period": 1,
247 "servers": {
248 "srv0": {
249 "listen": [
@@ -321,6 +326,7 @@ func TestSRVWithActiveHealthcheck(t *testing.T) {
326 {
327 "apps": {
328 "http": {
329 + "grace_period": 1,
330 "servers": {
331 "srv0": {
332 "listen": [
caddytest/integration/sni_test.go
+3
@@ -15,6 +15,7 @@ func TestDefaultSNI(t *testing.T) {
15 "http": {
16 "http_port": 9080,
17 "https_port": 9443,
18 + "grace_period": 1,
19 "servers": {
20 "srv0": {
21 "listen": [
@@ -112,6 +113,7 @@ func TestDefaultSNIWithNamedHostAndExplicitIP(t *testing.T) {
113 "http": {
114 "http_port": 9080,
115 "https_port": 9443,
116 + "grace_period": 1,
117 "servers": {
118 "srv0": {
119 "listen": [
@@ -212,6 +214,7 @@ func TestDefaultSNIWithPortMappingOnly(t *testing.T) {
214 "http": {
215 "http_port": 9080,
216 "https_port": 9443,
217 + "grace_period": 1,
218 "servers": {
219 "srv0": {
220 "listen": [
caddytest/integration/stream_test.go
+2
@@ -27,6 +27,7 @@ func TestH2ToH2CStream(t *testing.T) {
27 "http": {
28 "http_port": 9080,
29 "https_port": 9443,
30 + "grace_period": 1,
31 "servers": {
32 "srv0": {
33 "listen": [
@@ -216,6 +217,7 @@ func TestH2ToH1ChunkedResponse(t *testing.T) {
217 "http": {
218 "http_port": 9080,
219 "https_port": 9443,
220 + "grace_period": 1,
221 "servers": {
222 "srv0": {
223 "listen": [
modules/caddyhttp/app.go
+49 -9
@@ -505,24 +505,64 @@ func (app *App) Stop() error {
505 app.logger.Debug("servers shutting down with eternal grace period")
506 }
507
508 - // shut down servers
509 - for _, server := range app.Servers {
508 + // goroutines aren't guaranteed to be scheduled right away,
509 + // so we'll use one WaitGroup to wait for all the goroutines
510 + // to start their server shutdowns, and another to wait for
511 + // them to finish; we'll always block for them to start so
512 + // that when we return the caller can be confident* that the
513 + // old servers are no longer accepting new connections
514 + // (* the scheduler might still pause them right before
515 + // calling Shutdown(), but it's unlikely)
516 + var startedShutdown, finishedShutdown sync.WaitGroup
517 +
518 + // these will run in goroutines
519 + stopServer := func(server *Server) {
520 + defer finishedShutdown.Done()
521 + startedShutdown.Done()
522 +
523 if err := server.server.Shutdown(ctx); err != nil {
524 app.logger.Error("server shutdown",
525 zap.Error(err),
526 zap.Strings("addresses", server.Listen))
527 }
528 + }
529 + stopH3Server := func(server *Server) {
530 + defer finishedShutdown.Done()
531 + startedShutdown.Done()
532
516 - if server.h3server != nil {
517 - // TODO: CloseGracefully, once implemented upstream (see https://github.com/lucas-clemente/quic-go/issues/2103)
518 - if err := server.h3server.Close(); err != nil {
519 - app.logger.Error("HTTP/3 server shutdown",
520 - zap.Error(err),
521 - zap.Strings("addresses", server.Listen))
522 - }
533 + if server.h3server == nil {
534 + return
535 + }
536 + // TODO: CloseGracefully, once implemented upstream (see https://github.com/lucas-clemente/quic-go/issues/2103)
537 + if err := server.h3server.Close(); err != nil {
538 + app.logger.Error("HTTP/3 server shutdown",
539 + zap.Error(err),
540 + zap.Strings("addresses", server.Listen))
541 }
542 }
543
544 + for _, server := range app.Servers {
545 + startedShutdown.Add(2)
546 + finishedShutdown.Add(2)
547 + go stopServer(server)
548 + go stopH3Server(server)
549 + }
550 +
551 + // block until all the goroutines have been run by the scheduler;
552 + // this means that they have likely called Shutdown() by now
553 + startedShutdown.Wait()
554 +
555 + // if the process is exiting, we need to block here and wait
556 + // for the grace periods to complete, otherwise the process will
557 + // terminate before the servers are finished shutting down; but
558 + // we don't really need to wait for the grace period to finish
559 + // if the process isn't exiting (but note that frequent config
560 + // reloads with long grace periods for a sustained length of time
561 + // may deplete resources)
562 + if caddy.Exiting() {
563 + finishedShutdown.Wait()
564 + }
565 +
566 return nil
567 }
568