From 614ade1bb61d18f0624a0e41868ab2d5f15fd8a9 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 5 Oct 2021 11:52:14 -0400 Subject: [PATCH 1/4] logmon: refactor Logging tests Mostly to use testify assertions and close open resources --- client/logmon/logging/rotator_test.go | 266 ++++++++++---------------- 1 file changed, 99 insertions(+), 167 deletions(-) diff --git a/client/logmon/logging/rotator_test.go b/client/logmon/logging/rotator_test.go index 8f06cec70..057fcd967 100644 --- a/client/logmon/logging/rotator_test.go +++ b/client/logmon/logging/rotator_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/require" ) var ( @@ -19,168 +20,141 @@ var ( func TestFileRotator_IncorrectPath(t *testing.T) { t.Parallel() - if _, err := NewFileRotator("/foo", baseFileName, 10, 10, testlog.HCLogger(t)); err == nil { - t.Fatalf("expected error") - } + + _, err := NewFileRotator("/foo", baseFileName, 10, 10, testlog.HCLogger(t)) + require.Error(t, err) + require.Contains(t, err.Error(), "no such file or directory") } func TestFileRotator_CreateNewFile(t *testing.T) { t.Parallel() - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - t.Fatalf("test setup err: %v", err) - } + + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(t, err) defer os.RemoveAll(path) - _, err = NewFileRotator(path, baseFileName, 10, 10, testlog.HCLogger(t)) - if err != nil { - t.Fatalf("test setup err: %v", err) - } + fr, err := NewFileRotator(path, baseFileName, 10, 10, testlog.HCLogger(t)) + require.NoError(t, err) + defer fr.Close() - if _, err := os.Stat(filepath.Join(path, "redis.stdout.0")); err != nil { - t.Fatalf("expected file") - } + _, err = os.Stat(filepath.Join(path, "redis.stdout.0")) + require.NoError(t, err) } func TestFileRotator_OpenLastFile(t *testing.T) { t.Parallel() - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - t.Fatalf("test setup err: %v", err) - } + + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(t, err) defer os.RemoveAll(path) fname1 := filepath.Join(path, "redis.stdout.0") fname2 := filepath.Join(path, "redis.stdout.2") - if _, err := os.Create(fname1); err != nil { - t.Fatalf("test setup failure: %v", err) - } - if _, err := os.Create(fname2); err != nil { - t.Fatalf("test setup failure: %v", err) - } + + f1, err := os.Create(fname1) + require.NoError(t, err) + f1.Close() + + f2, err := os.Create(fname2) + require.NoError(t, err) + f2.Close() fr, err := NewFileRotator(path, baseFileName, 10, 10, testlog.HCLogger(t)) - if err != nil { - t.Fatalf("test setup err: %v", err) - } + require.NoError(t, err) + defer fr.Close() - if fr.currentFile.Name() != fname2 { - t.Fatalf("expected current file: %v, got: %v", fname2, fr.currentFile.Name()) - } + require.Equal(t, fname2, fr.currentFile.Name()) } func TestFileRotator_WriteToCurrentFile(t *testing.T) { t.Parallel() - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - t.Fatalf("test setup err: %v", err) - } + + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(t, err) defer os.RemoveAll(path) fname1 := filepath.Join(path, "redis.stdout.0") - if _, err := os.Create(fname1); err != nil { - t.Fatalf("test setup failure: %v", err) - } + f1, err := os.Create(fname1) + require.NoError(t, err) + f1.Close() fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t)) - if err != nil { - t.Fatalf("test setup err: %v", err) - } + require.NoError(t, err) + defer fr.Close() fr.Write([]byte("abcde")) - var actual int64 testutil.WaitForResult(func() (bool, error) { fi, err := os.Stat(fname1) if err != nil { - return false, err + return false, fmt.Errorf("failed to stat file %v: %w", fname1, err) } - actual = fi.Size() + actual := fi.Size() if actual != 5 { - return false, nil + return false, fmt.Errorf("expected size %d but found %d", 5, actual) } return true, nil }, func(err error) { - t.Fatalf("expected size: %v, actual: %v", 5, actual) + require.NoError(t, err) }) } func TestFileRotator_RotateFiles(t *testing.T) { t.Parallel() - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - t.Fatalf("test setup err: %v", err) - } + + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(t, err) defer os.RemoveAll(path) fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t)) - if err != nil { - t.Fatalf("test setup err: %v", err) - } + require.NoError(t, err) + defer fr.Close() str := "abcdefgh" nw, err := fr.Write([]byte(str)) - if err != nil { - t.Fatalf("got error while writing: %v", err) - } + require.NoError(t, err) + require.Equal(t, len(str), nw) - if nw != len(str) { - t.Fatalf("expected %v, got %v", len(str), nw) - } - - var lastErr error testutil.WaitForResult(func() (bool, error) { fname1 := filepath.Join(path, "redis.stdout.0") fi, err := os.Stat(fname1) if err != nil { - lastErr = err - return false, nil + return false, fmt.Errorf("failed to stat file %v: %w", fname1, err) } if fi.Size() != 5 { - lastErr = fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size()) - return false, nil + return false, fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size()) } fname2 := filepath.Join(path, "redis.stdout.1") if _, err := os.Stat(fname2); err != nil { - lastErr = fmt.Errorf("expected file %v to exist", fname2) - return false, nil + return false, fmt.Errorf("expected file %v to exist", fname2) } if fi2, err := os.Stat(fname2); err == nil { if fi2.Size() != 3 { - lastErr = fmt.Errorf("expected size: %v, actual: %v", 3, fi2.Size()) - return false, nil + return false, fmt.Errorf("expected size: %v, actual: %v", 3, fi2.Size()) } } else { - lastErr = fmt.Errorf("error getting the file info: %v", err) - return false, nil + return false, fmt.Errorf("error getting the file info: %v", err) } return true, nil }, func(err error) { - t.Fatalf("%v", lastErr) + require.NoError(t, err) }) } func TestFileRotator_RotateFiles_Boundary(t *testing.T) { t.Parallel() - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - t.Fatalf("test setup err: %v", err) - } + + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(t, err) defer os.RemoveAll(path) fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t)) - if err != nil { - t.Fatalf("test setup err: %v", err) - } + require.NoError(t, err) + defer fr.Close() // We will write three times: // 1st: Write with new lines spanning two files @@ -196,156 +170,120 @@ func TestFileRotator_RotateFiles_Boundary(t *testing.T) { for _, str := range []string{"ab\ncdef\n", "1234567890", "\n"} { nw, err := fr.Write([]byte(str)) - if err != nil { - t.Fatalf("got error while writing: %v", err) - } - - if nw != len(str) { - t.Fatalf("expected %v, got %v", len(str), nw) - } + require.NoError(t, err) + require.Equal(t, len(str), nw) } - var lastErr error testutil.WaitForResult(func() (bool, error) { for i, exp := range expectations { fname := filepath.Join(path, fmt.Sprintf("redis.stdout.%d", i)) fi, err := os.Stat(fname) if err != nil { - lastErr = err - return false, nil + return false, fmt.Errorf("failed to stat file %v: %w", fname, err) } if int(fi.Size()) != len(exp) { - lastErr = fmt.Errorf("expected size: %v, actual: %v", len(exp), fi.Size()) - return false, nil + return false, fmt.Errorf("expected size: %v, actual: %v", len(exp), fi.Size()) } } return true, nil }, func(err error) { - t.Fatalf("%v", lastErr) + require.NoError(t, err) }) } func TestFileRotator_WriteRemaining(t *testing.T) { t.Parallel() - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - t.Fatalf("test setup err: %v", err) - } + + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(t, err) defer os.RemoveAll(path) fname1 := filepath.Join(path, "redis.stdout.0") - if f, err := os.Create(fname1); err == nil { - f.Write([]byte("abcd")) - } else { - t.Fatalf("test setup failure: %v", err) - } + err = ioutil.WriteFile(fname1, []byte("abcd"), 0600) + require.NoError(t, err) fr, err := NewFileRotator(path, baseFileName, 10, 5, testlog.HCLogger(t)) - if err != nil { - t.Fatalf("test setup err: %v", err) - } + require.NoError(t, err) + defer fr.Close() str := "efghijkl" nw, err := fr.Write([]byte(str)) - if err != nil { - t.Fatalf("got error while writing: %v", err) - } - if nw != len(str) { - t.Fatalf("expected %v, got %v", len(str), nw) - } - var lastErr error + require.NoError(t, err) + require.Equal(t, len(str), nw) + testutil.WaitForResult(func() (bool, error) { fi, err := os.Stat(fname1) if err != nil { - lastErr = fmt.Errorf("error getting the file info: %v", err) - return false, nil + return false, fmt.Errorf("error getting the file info: %v", err) } if fi.Size() != 5 { - lastErr = fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size()) - return false, nil + return false, fmt.Errorf("expected size: %v, actual: %v", 5, fi.Size()) } fname2 := filepath.Join(path, "redis.stdout.1") if _, err := os.Stat(fname2); err != nil { - lastErr = fmt.Errorf("expected file %v to exist", fname2) - return false, nil + return false, fmt.Errorf("expected file %v to exist", fname2) } if fi2, err := os.Stat(fname2); err == nil { if fi2.Size() != 5 { - lastErr = fmt.Errorf("expected size: %v, actual: %v", 5, fi2.Size()) - return false, nil + return false, fmt.Errorf("expected size: %v, actual: %v", 5, fi2.Size()) } } else { - lastErr = fmt.Errorf("error getting the file info: %v", err) - return false, nil + return false, fmt.Errorf("error getting the file info: %v", err) } fname3 := filepath.Join(path, "redis.stdout.2") if _, err := os.Stat(fname3); err != nil { - lastErr = fmt.Errorf("expected file %v to exist", fname3) - return false, nil + return false, fmt.Errorf("expected file %v to exist", fname3) } if fi3, err := os.Stat(fname3); err == nil { if fi3.Size() != 2 { - lastErr = fmt.Errorf("expected size: %v, actual: %v", 2, fi3.Size()) - return false, nil + return false, fmt.Errorf("expected size: %v, actual: %v", 2, fi3.Size()) } } else { - lastErr = fmt.Errorf("error getting the file info: %v", err) - return false, nil + return false, fmt.Errorf("error getting the file info: %v", err) } return true, nil }, func(err error) { - t.Fatalf("%v", lastErr) + require.NoError(t, err) }) } func TestFileRotator_PurgeOldFiles(t *testing.T) { t.Parallel() - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - t.Fatalf("test setup err: %v", err) - } + + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(t, err) defer os.RemoveAll(path) fr, err := NewFileRotator(path, baseFileName, 2, 2, testlog.HCLogger(t)) - if err != nil { - t.Fatalf("test setup err: %v", err) - } + require.NoError(t, err) + defer fr.Close() str := "abcdeghijklmn" nw, err := fr.Write([]byte(str)) - if err != nil { - t.Fatalf("got error while writing: %v", err) - } - if nw != len(str) { - t.Fatalf("expected %v, got %v", len(str), nw) - } + require.NoError(t, err) + require.Equal(t, len(str), nw) - var lastErr error testutil.WaitForResult(func() (bool, error) { f, err := ioutil.ReadDir(path) if err != nil { - lastErr = fmt.Errorf("test error: %v", err) - return false, nil + return false, fmt.Errorf("failed to read dir %v: %w", path, err) } if len(f) != 2 { - lastErr = fmt.Errorf("expected number of files: %v, got: %v", 2, len(f)) - return false, nil + return false, fmt.Errorf("expected number of files: %v, got: %v %v", 2, len(f), f) } return true, nil }, func(err error) { - t.Fatalf("%v", lastErr) + require.NoError(t, err) }) } @@ -359,17 +297,14 @@ func BenchmarkRotator(b *testing.B) { } func benchmarkRotatorWithInputSize(size int, b *testing.B) { - var path string - var err error - if path, err = ioutil.TempDir("", pathPrefix); err != nil { - b.Fatalf("test setup err: %v", err) - } + path, err := ioutil.TempDir("", pathPrefix) + require.NoError(b, err) defer os.RemoveAll(path) fr, err := NewFileRotator(path, baseFileName, 5, 1024*1024, testlog.HCLogger(b)) - if err != nil { - b.Fatalf("test setup err: %v", err) - } + require.NoError(b, err) + defer fr.Close() + b.ResetTimer() // run the Fib function b.N times @@ -377,9 +312,7 @@ func benchmarkRotatorWithInputSize(size int, b *testing.B) { // Generate some input data := make([]byte, size) _, err := rand.Read(data) - if err != nil { - b.Fatalf("Error generating date: %v", err) - } + require.NoError(b, err) // Insert random new lines for i := 0; i < 100; i++ { @@ -388,8 +321,7 @@ func benchmarkRotatorWithInputSize(size int, b *testing.B) { } // Write the data - if _, err := fr.Write(data); err != nil { - b.Fatalf("Failed to write data: %v", err) - } + _, err = fr.Write(data) + require.NoError(b, err) } } From 9668245c4c55737866007546c58c7b54349e5464 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 5 Oct 2021 12:04:09 -0400 Subject: [PATCH 2/4] logmon: add a test for leaked goroutines --- client/logmon/logging/rotator_test.go | 17 +++++++++-------- go.mod | 6 +++--- go.sum | 27 +++++++++++++++++---------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/client/logmon/logging/rotator_test.go b/client/logmon/logging/rotator_test.go index 057fcd967..da80925b8 100644 --- a/client/logmon/logging/rotator_test.go +++ b/client/logmon/logging/rotator_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" + "go.uber.org/goleak" ) var ( @@ -19,7 +20,7 @@ var ( ) func TestFileRotator_IncorrectPath(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) _, err := NewFileRotator("/foo", baseFileName, 10, 10, testlog.HCLogger(t)) require.Error(t, err) @@ -27,7 +28,7 @@ func TestFileRotator_IncorrectPath(t *testing.T) { } func TestFileRotator_CreateNewFile(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) path, err := ioutil.TempDir("", pathPrefix) require.NoError(t, err) @@ -42,7 +43,7 @@ func TestFileRotator_CreateNewFile(t *testing.T) { } func TestFileRotator_OpenLastFile(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) path, err := ioutil.TempDir("", pathPrefix) require.NoError(t, err) @@ -67,7 +68,7 @@ func TestFileRotator_OpenLastFile(t *testing.T) { } func TestFileRotator_WriteToCurrentFile(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) path, err := ioutil.TempDir("", pathPrefix) require.NoError(t, err) @@ -101,7 +102,7 @@ func TestFileRotator_WriteToCurrentFile(t *testing.T) { } func TestFileRotator_RotateFiles(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) path, err := ioutil.TempDir("", pathPrefix) require.NoError(t, err) @@ -146,7 +147,7 @@ func TestFileRotator_RotateFiles(t *testing.T) { } func TestFileRotator_RotateFiles_Boundary(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) path, err := ioutil.TempDir("", pathPrefix) require.NoError(t, err) @@ -194,7 +195,7 @@ func TestFileRotator_RotateFiles_Boundary(t *testing.T) { } func TestFileRotator_WriteRemaining(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) path, err := ioutil.TempDir("", pathPrefix) require.NoError(t, err) @@ -256,7 +257,7 @@ func TestFileRotator_WriteRemaining(t *testing.T) { } func TestFileRotator_PurgeOldFiles(t *testing.T) { - t.Parallel() + defer goleak.VerifyNone(t) path, err := ioutil.TempDir("", pathPrefix) require.NoError(t, err) diff --git a/go.mod b/go.mod index 2fa05de3e..87213501a 100644 --- a/go.mod +++ b/go.mod @@ -125,12 +125,12 @@ require ( github.com/zclconf/go-cty v1.8.0 github.com/zclconf/go-cty-yaml v1.0.2 go.opencensus.io v0.22.1-0.20190713072201-b4a14686f0a9 // indirect + go.uber.org/goleak v1.1.12 golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0 - golang.org/x/net v0.0.0-20201224014010-6772e930b67b - golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e + golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 + golang.org/x/sync v0.0.0-20210220032951-036812b2e83c golang.org/x/sys v0.0.0-20210818153620-00dd8d7831e7 golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e - golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375 // indirect google.golang.org/api v0.13.0 // indirect google.golang.org/grpc v1.29.1 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect diff --git a/go.sum b/go.sum index f9ee95cd5..d65c8d0c6 100644 --- a/go.sum +++ b/go.sum @@ -770,7 +770,7 @@ github.com/willf/bitset v1.1.11/go.mod h1:83CECat5yLh5zVOf4P1ErAgKA5UDvKtgyUABdr github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f/go.mod h1:5yf86TLmAcydyeJq5YvxkGPE2fm/u4myDekKRoLuqhs= -github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/zclconf/go-cty v1.0.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= github.com/zclconf/go-cty v1.4.0/go.mod h1:nHzOclRkoj++EU9ZjSrZvRG0BXIWt8c7loYc0qXAFGQ= @@ -784,6 +784,8 @@ go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.opencensus.io v0.22.1-0.20190713072201-b4a14686f0a9 h1:7LiVwYOeGhrZmChB6cSFzXlk3v0aRNA28kOEygIK9mw= go.opencensus.io v0.22.1-0.20190713072201-b4a14686f0a9/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= +go.uber.org/goleak v1.1.12 h1:gZAh5/EyT/HQwlpkCy6wTpqfH9H8Lz8zbm3dZh+OyzA= +go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= @@ -817,14 +819,15 @@ golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvx golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/lint v0.0.0-20190409202823-959b441ac422/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= -golang.org/x/lint v0.0.0-20190909230951-414d861bb4ac h1:8R1esu+8QioDxo4E4mX6bFztO+dMTM49DNAaWfO5OeY= golang.org/x/lint v0.0.0-20190909230951-414d861bb4ac/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE= golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028/go.mod h1:E/iHnbuqvinMTCcRqshq8CkpyQDoeVncDDYHnLhea+o= golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc= golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY= -golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ= -golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -848,13 +851,13 @@ golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20190923162816-aa69164e4478/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20191004110552-13f9640d40b9/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200602114024-627f9648deb9/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20201002202402-0a1ea396d57c/go.mod h1:iQL9McJNjoIa5mjH6nYTCTZXUN6RP+XW3eib7Ya3XcI= golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.0.0-20201224014010-6772e930b67b h1:iFwSg7t5GZmB/Q5TjiEAsdoLDrdJRC1RiF2WhuV29Qw= golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 h1:4nGaVu0QrbjT/AK2PRLuQfQuh6DJve+pELhqTdAj3x0= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0= @@ -864,8 +867,9 @@ golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20170830134202-bb24a47a89ea/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -909,6 +913,8 @@ golang.org/x/sys v0.0.0-20200916030750-2334cc1a136f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210316164454-77fc1eacc6aa/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210818153620-00dd8d7831e7 h1:/bmDWM82ZX7TawqxuI8kVjKI0TXHdSY6pHJArewwHtU= golang.org/x/sys v0.0.0-20210818153620-00dd8d7831e7/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -947,12 +953,13 @@ golang.org/x/tools v0.0.0-20190907020128-2ca718005c18/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20190911174233-4f2ddba30aff/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375 h1:SjQ2+AKWgZLc1xej6WSzL+Dfs5Uyd5xcZH1mGC411IA= -golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA= +golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M= google.golang.org/api v0.8.0/go.mod h1:o4eAsZoiT+ibD93RtjEohWalFOjRDx6CVaqeizhEnKg= From c86cff02f9ad1724f42a5bfbc36387451c306118 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 5 Oct 2021 10:51:15 -0400 Subject: [PATCH 3/4] logmon: Fix a memory leak on task restart Fix a logmon leak causing high goroutine and memory usage when a task restarts. Logmon `FileRotator` buffers the task stdout/stderr streams and periodically flushing them to log files. Logmon creates a new FileRotator for each stream for each task run. However, the `flushPeriodically` goroutine is leaked when a task restarts, holding a reference to a no-longer-needed `FileRotator` instance along with its 64kb buffer. The cause is that the code assumed `time.Ticker.Stop()` closes the ticker channel, thereby terminating the goroutine, but the documentation says otherwise: > Stop turns off a ticker. After Stop, no more ticks will be sent. Stop does not close the channel, to prevent a concurrent goroutine reading from the channel from seeing an erroneous "tick". https://pkg.go.dev/time#Ticker.Stop --- client/logmon/logging/rotator.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/client/logmon/logging/rotator.go b/client/logmon/logging/rotator.go index 75c0f8803..010e316e2 100644 --- a/client/logmon/logging/rotator.go +++ b/client/logmon/logging/rotator.go @@ -71,7 +71,7 @@ func NewFileRotator(path string, baseFile string, maxFiles int, flushTicker: time.NewTicker(bufferFlushDuration), logger: logger, purgeCh: make(chan struct{}, 1), - doneCh: make(chan struct{}, 1), + doneCh: make(chan struct{}), } if err := rotator.lastFile(); err != nil { @@ -237,8 +237,14 @@ func (f *FileRotator) createFile() error { // flushPeriodically flushes the buffered writer every 100ms to the underlying // file func (f *FileRotator) flushPeriodically() { - for range f.flushTicker.C { - f.flushBuffer() + for { + select { + case <-f.flushTicker.C: + f.flushBuffer() + case <-f.doneCh: + return + } + } } @@ -251,9 +257,9 @@ func (f *FileRotator) Close() error { f.flushTicker.Stop() f.flushBuffer() - // Stop the purge go routine + // Stop the go routines if !f.closed { - f.doneCh <- struct{}{} + close(f.doneCh) close(f.purgeCh) f.closed = true f.currentFile.Close() From f4b92c609ec716367fda438b826b336419b7d1f1 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 5 Oct 2021 13:01:19 -0400 Subject: [PATCH 4/4] add changelog --- .changelog/11261.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/11261.txt diff --git a/.changelog/11261.txt b/.changelog/11261.txt new file mode 100644 index 000000000..b06e58327 --- /dev/null +++ b/.changelog/11261.txt @@ -0,0 +1,3 @@ +```release-note:bug +client: Fixed a memory leak in log collector when tasks restart +```