From 40866e974023eaac50f77f0c913baf04dd4935c9 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 27 Mar 2024 10:39:42 +0200 Subject: [PATCH] Wait for handlers to exit when stopping gRPC server We've recently migrated our logging from the interceptors to the stats handler. The stats handler runs after an RPC has completed. gRPC by default doesn't wait for all handlers to return when stopping. This may lead to some of our log messages being not logged as the server stops before the log handler has executed after RPC completion. Other than leading to missing logs on shutdown, this also leads to flaky tests as not all log entries are guaranteed to be flushed by the time the test is accessing them after stopping the server. This commit configures gRPC servers in Gitaly and Prafect to wait until all handlers have finished before returning from Stop(). This ensures our logging handlers have also had the chance to run and that there are no goroutines running that were spawned by the gRPC server by the time Stop() returns. Changelog: fixed --- internal/gitaly/server/server.go | 1 + internal/praefect/server.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/gitaly/server/server.go b/internal/gitaly/server/server.go index fd39ec9ab1..bd373b87ab 100644 --- a/internal/gitaly/server/server.go +++ b/internal/gitaly/server/server.go @@ -187,6 +187,7 @@ func (s *GitalyServerFactory) New(external, secure bool, opts ...Option) (*grpc. grpc.KeepaliveParams(keepalive.ServerParameters{ Time: 5 * time.Minute, }), + grpc.WaitForHandlers(true), } return grpc.NewServer(serverOptions...), nil diff --git a/internal/praefect/server.go b/internal/praefect/server.go index f985791b1c..48cacfc2fa 100644 --- a/internal/praefect/server.go +++ b/internal/praefect/server.go @@ -167,6 +167,7 @@ func NewGRPCServer( grpc.KeepaliveParams(keepalive.ServerParameters{ Time: 5 * time.Minute, }), + grpc.WaitForHandlers(true), }...) // Accept backchannel connections so that we can proxy sidechannels -- GitLab