From 4d7094733341728f89cd848078adb6f4d3f7e778 Mon Sep 17 00:00:00 2001 From: Keegan Carruthers-Smith Date: Thu, 30 Nov 2023 11:05:20 +0100 Subject: [PATCH] all: remove detailed debug file logging We have noticed profiles where lumberjack is responsible for 1GB of memory. This is surprising and we haven't used the log output in a long time. Rather than debugging, we are going to remove it for now and when we need detailed logs again we can add something back. Test Plan: go test --- build/builder.go | 26 +----------- cmd/zoekt-sourcegraph-indexserver/cleanup.go | 44 ++------------------ cmd/zoekt-sourcegraph-indexserver/merge.go | 14 ------- go.mod | 1 - go.sum | 2 - 5 files changed, 5 insertions(+), 82 deletions(-) diff --git a/build/builder.go b/build/builder.go index 1838b777a..de6e1508d 100644 --- a/build/builder.go +++ b/build/builder.go @@ -39,7 +39,6 @@ import ( "github.com/bmatcuk/doublestar" "github.com/grafana/regexp" "github.com/rs/xid" - "gopkg.in/natefinch/lumberjack.v2" "github.com/sourcegraph/zoekt" "github.com/sourcegraph/zoekt/ctags" @@ -250,7 +249,7 @@ type Builder struct { size int parserFactory ctags.ParserFactory - building sync.WaitGroup + building sync.WaitGroup errMu sync.Mutex buildError error @@ -259,8 +258,6 @@ type Builder struct { // them once all shards succeed to avoid Frankstein corpuses. finishedShards map[string]string - shardLogger io.WriteCloser - // indexTime is set by tests for doing reproducible builds. indexTime time.Time @@ -575,12 +572,6 @@ func NewBuilder(opts Options) (*Builder, error) { b.parserFactory = parserFactory - b.shardLogger = &lumberjack.Logger{ - Filename: filepath.Join(opts.IndexDir, "zoekt-builder-shard-log.tsv"), - MaxSize: 100, // Megabyte - MaxBackups: 5, - } - if opts.IsDelta { // Delta shards build on top of previously existing shards. // As a consequence, the shardNum for delta shards starts from @@ -747,8 +738,6 @@ func (b *Builder) Finish() error { return b.buildError } - defer b.shardLogger.Close() - // Collect a map of the old shards on disk. For each new shard we replace we // delete it from toDelete. Anything remaining in toDelete will be removed // after we have renamed everything into place. @@ -779,8 +768,6 @@ func (b *Builder) Finish() error { } delete(toDelete, final) - - b.shardLog("upsert", final, b.opts.RepositoryDescription.Name) } b.finishedShards = map[string]string{} @@ -791,13 +778,11 @@ func (b *Builder) Finish() error { if !strings.HasSuffix(p, ".zoekt") { continue } - b.shardLog("tomb", p, b.opts.RepositoryDescription.Name) err := zoekt.SetTombstone(p, b.opts.RepositoryDescription.ID) b.buildError = err continue } log.Printf("removing old shard file: %s", p) - b.shardLog("remove", p, b.opts.RepositoryDescription.Name) if err := os.Remove(p); err != nil { b.buildError = err } @@ -878,15 +863,6 @@ func (b *Builder) flush() error { return nil } -func (b *Builder) shardLog(action, shard string, repoName string) { - shard = filepath.Base(shard) - var shardSize int64 - if fi, err := os.Stat(filepath.Join(b.opts.IndexDir, shard)); err == nil { - shardSize = fi.Size() - } - _, _ = fmt.Fprintf(b.shardLogger, "%s\t%s\t%s\t%d\t%s\n", time.Now().UTC().Format(time.RFC3339), action, shard, shardSize, repoName) -} - var profileNumber int func (b *Builder) writeMemProfile(name string) { diff --git a/cmd/zoekt-sourcegraph-indexserver/cleanup.go b/cmd/zoekt-sourcegraph-indexserver/cleanup.go index c51a3f3d4..3a8a05407 100644 --- a/cmd/zoekt-sourcegraph-indexserver/cleanup.go +++ b/cmd/zoekt-sourcegraph-indexserver/cleanup.go @@ -13,7 +13,6 @@ import ( "github.com/grafana/regexp" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "gopkg.in/natefinch/lumberjack.v2" "github.com/sourcegraph/zoekt" ) @@ -97,10 +96,10 @@ func cleanup(indexDir string, repos []uint32, now time.Time, shardMerging bool) simple := shards[:0] for _, s := range shards { if shardMerging && maybeSetTombstone([]shard{s}, repo) { - shardsLog(indexDir, "tombname", []shard{s}) - } else { - simple = append(simple, s) + continue } + + simple = append(simple, s) } if len(simple) == 0 { @@ -108,7 +107,6 @@ func cleanup(indexDir string, repos []uint32, now time.Time, shardMerging bool) } removeAll(simple...) - shardsLog(indexDir, "removename", simple) } // index: Move missing repos from trash into index @@ -121,7 +119,6 @@ func cleanup(indexDir string, repos []uint32, now time.Time, shardMerging bool) if shards, ok := trash[repo]; ok { log.Printf("restoring shards from trash for %v", repo) moveAll(indexDir, shards) - shardsLog(indexDir, "restore", shards) continue } @@ -130,8 +127,6 @@ func cleanup(indexDir string, repos []uint32, now time.Time, shardMerging bool) err := zoekt.UnsetTombstone(s.Path, repo) if err != nil { log.Printf("error removing tombstone for %v: %s", repo, err) - } else { - shardsLog(indexDir, "untomb", []shard{s}) } } } @@ -145,11 +140,9 @@ func cleanup(indexDir string, repos []uint32, now time.Time, shardMerging bool) } if shardMerging && maybeSetTombstone(shards, repo) { - shardsLog(indexDir, "tomb", shards) continue } moveAll(trashDir, shards) - shardsLog(indexDir, "remove", shards) } // Remove .tmp files from crashed indexer runs-- for example, if an indexer @@ -382,24 +375,6 @@ func maybeSetTombstone(shards []shard, repoID uint32) bool { return true } -func shardsLog(indexDir, action string, shards []shard) { - shardLogger := &lumberjack.Logger{ - Filename: filepath.Join(indexDir, "zoekt-indexserver-shard-log.tsv"), - MaxSize: 100, // Megabyte - MaxBackups: 5, - } - defer shardLogger.Close() - - for _, s := range shards { - shardName := filepath.Base(s.Path) - var shardSize int64 - if fi, err := os.Stat(filepath.Join(indexDir, shardName)); err == nil { - shardSize = fi.Size() - } - _, _ = fmt.Fprintf(shardLogger, "%s\t%s\t%s\t%d\t%s\t%d\n", time.Now().UTC().Format(time.RFC3339), action, shardName, shardSize, s.RepoName, s.RepoID) - } -} - var metricVacuumRunning = promauto.NewGauge(prometheus.GaugeOpts{ Name: "index_vacuum_running", Help: "Set to 1 if indexserver's vacuum job is running.", @@ -448,28 +423,17 @@ func (s *Server) vacuum() { if err != nil { debug.Printf("failed to explode compound shard %s: %s", path, string(b)) - } else { - shardsLog(s.IndexDir, "explode", []shard{{Path: path}}) } continue } - var removed []*zoekt.Repository s.muIndexDir.Global(func() { - removed, err = removeTombstones(path) + _, err = removeTombstones(path) }) if err != nil { debug.Printf("error while removing tombstones in %s: %s", fn, err) } - for _, repo := range removed { - shardsLog(s.IndexDir, "vac", []shard{{ - RepoID: repo.ID, - RepoName: repo.Name, - Path: filepath.Join(s.IndexDir, fn), - ModTime: info.ModTime(), - }}) - } } } diff --git a/cmd/zoekt-sourcegraph-indexserver/merge.go b/cmd/zoekt-sourcegraph-indexserver/merge.go index 561580342..67a005c80 100644 --- a/cmd/zoekt-sourcegraph-indexserver/merge.go +++ b/cmd/zoekt-sourcegraph-indexserver/merge.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "log" "os" "os/exec" @@ -13,7 +12,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "go.uber.org/atomic" - "gopkg.in/natefinch/lumberjack.v2" "github.com/sourcegraph/zoekt" ) @@ -71,12 +69,6 @@ func (s *Server) merge(mergeCmd func(args ...string) *exec.Cmd) { metricShardMergingRunning.Set(1) defer metricShardMergingRunning.Set(0) - wc := &lumberjack.Logger{ - Filename: filepath.Join(s.IndexDir, "zoekt-merge-log.tsv"), - MaxSize: 100, // Megabyte - MaxBackups: 5, - } - // We keep creating compound shards until we run out of shards to merge or until // we encounter an error during merging. next := true @@ -107,12 +99,6 @@ func (s *Server) merge(mergeCmd func(args ...string) *exec.Cmd) { return } - newCompoundName := reCompound.Find(out) - now := time.Now() - for _, s := range c.shards { - _, _ = fmt.Fprintf(wc, "%s\t%s\t%s\t%s\n", now.UTC().Format(time.RFC3339), "merge", filepath.Base(s.path), string(newCompoundName)) - } - next = true }) } diff --git a/go.mod b/go.mod index fbdf0ac5f..6c7c8afeb 100644 --- a/go.mod +++ b/go.mod @@ -56,7 +56,6 @@ require ( golang.org/x/sys v0.11.0 google.golang.org/grpc v1.56.1 google.golang.org/protobuf v1.31.0 - gopkg.in/natefinch/lumberjack.v2 v2.2.1 ) require ( diff --git a/go.sum b/go.sum index e12f6305e..8aec9722c 100644 --- a/go.sum +++ b/go.sum @@ -558,8 +558,6 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc= -gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc= gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME= gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=