From f7b12c1268863b49316e55cd2c7316f26bb54bb8 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:32:21 +0000 Subject: [PATCH] Apply shutdown timeout to http server to limit reload delay (#14339) (#14362) httpServer.stop may block indefinitely, potentially due to misbehaving connections. Apply shutdown timeout to http server so that in a hot reload, the old and new server overlapping time is bounded. (cherry picked from commit 24329c8b6c6b91628dc1996e691d62050944ad4b) Co-authored-by: Carson Ip --- changelogs/head.asciidoc | 1 + internal/beater/http.go | 4 ++-- internal/beater/server.go | 7 +++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/changelogs/head.asciidoc b/changelogs/head.asciidoc index d1ce40a63e..f3211c82c9 100644 --- a/changelogs/head.asciidoc +++ b/changelogs/head.asciidoc @@ -8,6 +8,7 @@ https://github.com/elastic/apm-server/compare/8.15\...main[View commits] - Track all bulk request response status codes {pull}13574[13574] - Fix a concurrent map write panic in monitoring middleware {pull}14335[14335] +- Apply shutdown timeout to http server {pull}14339[14339] [float] ==== Breaking Changes diff --git a/internal/beater/http.go b/internal/beater/http.go index 3c9993658a..33e1ffbfac 100644 --- a/internal/beater/http.go +++ b/internal/beater/http.go @@ -106,9 +106,9 @@ func (h *httpServer) start() error { return h.Serve(h.httpListener) } -func (h *httpServer) stop() { +func (h *httpServer) stop(ctx context.Context) { h.logger.Infof("Stop listening on: %s", h.Server.Addr) - if err := h.Shutdown(context.Background()); err != nil { + if err := h.Shutdown(ctx); err != nil { h.logger.Errorf("error stopping http server: %s", err.Error()) if err := h.Close(); err != nil { h.logger.Errorf("error closing http server: %s", err.Error()) diff --git a/internal/beater/server.go b/internal/beater/server.go index 397e21e506..973269cc8c 100644 --- a/internal/beater/server.go +++ b/internal/beater/server.go @@ -222,11 +222,10 @@ func (s server) run(ctx context.Context) error { }) g.Go(func() error { <-ctx.Done() - // httpServer should stop before grpcServer to avoid a panic caused by placing a new connection into - // a closed grpc connection channel during shutdown. - // See https://github.com/elastic/gmux/issues/13 - s.httpServer.stop() s.grpcServer.GracefulStop() + stopctx, cancel := context.WithTimeout(context.Background(), s.cfg.ShutdownTimeout) + defer cancel() + s.httpServer.stop(stopctx) return nil }) if err := g.Wait(); err != http.ErrServerClosed {