From c9e7fbb94dc18ac6f9a914635365d41ae55d3a86 Mon Sep 17 00:00:00 2001 From: Stan Hu <stanhu@gmail.com> Date: Sat, 15 Jun 2024 00:03:05 -0700 Subject: [PATCH] Ensure Workhorse log writers are closed to avoid Goroutine leaks We've seen in production the number of Goroutines in Workhorse exceed 4000. The pprof output suggests that many of these came from the logrus Goroutines lingering. When Workhorse forks `gitlab-zip-cat` and `gitlab-zip-metadata` it attempts to log all stderr via a logrus Writer. logrus launches a Goroutine to listen from a pipe. We should ensure this Writer is closed so that the Goroutines shut down as soon as possible. Changelog: fixed --- workhorse/internal/artifacts/entry.go | 9 ++++++++- workhorse/internal/upload/artifacts_uploader.go | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/workhorse/internal/artifacts/entry.go b/workhorse/internal/artifacts/entry.go index 09d9c5635b748..11b61f909409b 100644 --- a/workhorse/internal/artifacts/entry.go +++ b/workhorse/internal/artifacts/entry.go @@ -71,12 +71,19 @@ func unpackFileFromZip(ctx context.Context, archivePath, encodedFilename string, return err } + logWriter := log.ContextLogger(ctx).Writer() + defer func() { + if closeErr := logWriter.Close(); closeErr != nil { + log.ContextLogger(ctx).WithError(closeErr).Error("failed to close gitlab-zip-cat log writer") + } + }() + catFile := exec.Command("gitlab-zip-cat") catFile.Env = append(os.Environ(), "ARCHIVE_PATH="+archivePath, "ENCODED_FILE_NAME="+encodedFilename, ) - catFile.Stderr = log.ContextLogger(ctx).Writer() + catFile.Stderr = logWriter catFile.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} stdout, err := catFile.StdoutPipe() if err != nil { diff --git a/workhorse/internal/upload/artifacts_uploader.go b/workhorse/internal/upload/artifacts_uploader.go index 1e9219613c8ce..a4ecaa3b7bb64 100644 --- a/workhorse/internal/upload/artifacts_uploader.go +++ b/workhorse/internal/upload/artifacts_uploader.go @@ -72,8 +72,15 @@ func (a *artifactsUploadProcessor) generateMetadataFromZip(ctx context.Context, fileName = file.RemoteURL } + logWriter := log.ContextLogger(ctx).Writer() + defer func() { + if closeErr := logWriter.Close(); closeErr != nil { + log.ContextLogger(ctx).WithError(closeErr).Error("failed to close gitlab-zip-metadata log writer") + } + }() + zipMd := exec.CommandContext(ctx, "gitlab-zip-metadata", "-zip-reader-limit", strconv.FormatInt(readerLimit, 10), fileName) - zipMd.Stderr = log.ContextLogger(ctx).Writer() + zipMd.Stderr = logWriter zipMd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} zipMdOut, err := zipMd.StdoutPipe() -- GitLab