From b497e6718e60bd7695925435af09e9259b9b1270 Mon Sep 17 00:00:00 2001
From: George Papadrosou <gpapadrosou@gmail.com>
Date: Wed, 17 Apr 2019 22:02:35 +0300
Subject: [PATCH] goroutinedumper: Gzip files

Fixes #36846

Release note: None
---
 pkg/cli/zip.go                                     |  3 ++-
 pkg/server/goroutinedumper/goroutinedumper.go      | 13 +++++++++++--
 pkg/server/goroutinedumper/goroutinedumper_test.go |  4 ++--
 pkg/server/status_test.go                          |  4 ++--
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/pkg/cli/zip.go b/pkg/cli/zip.go
index be336a11d0e6..b104863c5ce7 100644
--- a/pkg/cli/zip.go
+++ b/pkg/cli/zip.go
@@ -398,7 +398,8 @@ func runDebugZip(cmd *cobra.Command, args []string) error {
 					return z.createError("/goroutines", err)
 				}
 				for _, file := range goroutinesResp.Files {
-					name := prefix + "/goroutines/" + file.Name + ".txt"
+					// NB: the files have a .txt.gz suffix already.
+					name := prefix + "/goroutines/" + file.Name
 					if err := z.createRawOrError(name, file.Contents, err); err != nil {
 						return err
 					}
diff --git a/pkg/server/goroutinedumper/goroutinedumper.go b/pkg/server/goroutinedumper/goroutinedumper.go
index 1bac27f34813..b63b8651b0b6 100644
--- a/pkg/server/goroutinedumper/goroutinedumper.go
+++ b/pkg/server/goroutinedumper/goroutinedumper.go
@@ -15,6 +15,7 @@
 package goroutinedumper
 
 import (
+	"compress/gzip"
 	"context"
 	"fmt"
 	"io/ioutil"
@@ -175,14 +176,22 @@ func gc(ctx context.Context, dir string, sizeLimit int64) {
 }
 
 func takeGoroutineDump(dir string, filename string) error {
+	filename = filename + ".txt.gz"
 	path := filepath.Join(dir, filename)
 	f, err := os.Create(path)
 	if err != nil {
 		return errors.Wrapf(err, "error creating file %s for goroutine dump", path)
 	}
 	defer f.Close()
-	if err = pprof.Lookup("goroutine").WriteTo(f, 2); err != nil {
+	w := gzip.NewWriter(f)
+	if err = pprof.Lookup("goroutine").WriteTo(w, 2); err != nil {
 		return errors.Wrapf(err, "error writing goroutine dump to %s", path)
 	}
-	return nil
+	// Flush and write the gzip header. It doesn't close the underlying writer.
+	if err := w.Close(); err != nil {
+		return errors.Wrapf(err, "error closing gzip writer for %s", path)
+	}
+	// Return f.Close() too so that we don't miss a potential error if everything
+	// else succeeded.
+	return f.Close()
 }
diff --git a/pkg/server/goroutinedumper/goroutinedumper_test.go b/pkg/server/goroutinedumper/goroutinedumper_test.go
index ba297a778878..ce7e350ebb97 100644
--- a/pkg/server/goroutinedumper/goroutinedumper_test.go
+++ b/pkg/server/goroutinedumper/goroutinedumper_test.go
@@ -316,11 +316,11 @@ func TestTakeGoroutineDump(t *testing.T) {
 	t.Run("fails because dump already exists as a directory", func(t *testing.T) {
 		tempDir, dirCleanupFn := testutils.TempDir(t)
 		defer dirCleanupFn()
-		filename := "goroutine_dump"
-		path := filepath.Join(tempDir, filename)
+		path := filepath.Join(tempDir, "goroutine_dump.txt.gz")
 		err := os.Mkdir(path, 0755)
 		assert.NoError(t, err, "failed to make dump directory %s", path)
 
+		filename := "goroutine_dump"
 		err = takeGoroutineDump(tempDir, filename)
 		assert.Error(t, err)
 		assert.Contains(
diff --git a/pkg/server/status_test.go b/pkg/server/status_test.go
index d4af282467ea..73fe282555b7 100644
--- a/pkg/server/status_test.go
+++ b/pkg/server/status_test.go
@@ -310,7 +310,7 @@ func TestStatusGetFiles(t *testing.T) {
 	t.Run("goroutines", func(t *testing.T) {
 		const testFilesNo = 3
 		for i := 0; i < testFilesNo; i++ {
-			testFile := filepath.Join(storeSpec.Path, "logs", goroutinesDir, fmt.Sprintf("goroutine_dump%d.txt", i))
+			testFile := filepath.Join(storeSpec.Path, "logs", goroutinesDir, fmt.Sprintf("goroutine_dump%d.txt.gz", i))
 			if err := ioutil.WriteFile(testFile, []byte(fmt.Sprintf("Goroutine dump %d", i)), 0644); err != nil {
 				t.Fatal(err)
 			}
@@ -328,7 +328,7 @@ func TestStatusGetFiles(t *testing.T) {
 		}
 
 		for i, file := range response.Files {
-			expectedFileName := fmt.Sprintf("goroutine_dump%d.txt", i)
+			expectedFileName := fmt.Sprintf("goroutine_dump%d.txt.gz", i)
 			if file.Name != expectedFileName {
 				t.Fatalf("expected file name %s, found %s", expectedFileName, file.Name)
 			}