From a44993ee6956152884f951b4907dbd135c06ebfb Mon Sep 17 00:00:00 2001 From: Linas Naginionis Date: Tue, 2 Feb 2021 14:28:16 +0200 Subject: [PATCH] implemented default NoOpProfiler. --- .../commitlog/source_instrumentation.go | 44 +++-- .../bootstrapper/fs/source_instrumentation.go | 44 +++-- .../peers/source_instrumentation.go | 44 +++-- src/dbnode/storage/profiler/options.go | 22 +-- src/dbnode/storage/profiler/profiler.go | 151 ++---------------- src/dbnode/storage/profiler/profiler_test.go | 69 +------- src/dbnode/storage/profiler/types.go | 9 -- 7 files changed, 77 insertions(+), 306 deletions(-) diff --git a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_instrumentation.go b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_instrumentation.go index 8895706022..861cad5d8c 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_instrumentation.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/commitlog/source_instrumentation.go @@ -73,29 +73,23 @@ func (i *instrumentationContext) finish() { i.span.Finish() } -func (i *instrumentationContext) startCPUProfileIfEnabled(name string) { - if i.pOpts.Enabled() { - err := i.pOpts.Profiler().StartCPUProfile(name) - if err != nil { - i.log.Error("unable to start cpu profile", zap.Error(err)) - } +func (i *instrumentationContext) startCPUProfile(name string) { + err := i.pOpts.Profiler().StartCPUProfile(name) + if err != nil { + i.log.Error("unable to start cpu profile", zap.Error(err)) } } -func (i *instrumentationContext) stopCPUProfileIfEnabled() { - if i.pOpts.Enabled() { - if err := i.pOpts.Profiler().StopCPUProfile(); err != nil { - i.log.Error("unable to stop cpu profile", zap.Error(err)) - } +func (i *instrumentationContext) stopCPUProfile() { + if err := i.pOpts.Profiler().StopCPUProfile(); err != nil { + i.log.Error("unable to stop cpu profile", zap.Error(err)) } } -func (i *instrumentationContext) writeHeapProfileIfEnabled(name string) { - if i.pOpts.Enabled() { - err := i.pOpts.Profiler().WriteHeapProfile(name) - if err != nil { - i.log.Error("unable to write heap profile", zap.Error(err)) - } +func (i *instrumentationContext) writeHeapProfile(name string) { + err := i.pOpts.Profiler().WriteHeapProfile(name) + if err != nil { + i.log.Error("unable to write heap profile", zap.Error(err)) } } @@ -103,8 +97,8 @@ func (i *instrumentationContext) bootstrapSnapshotsStarted() { i.log.Info("read snapshots start") i.span.LogFields(opentracinglog.String("event", "read_snapshots_start")) i.start = i.nowFn() - i.startCPUProfileIfEnabled(snapshotsProfileName) - i.writeHeapProfileIfEnabled(snapshotsProfileName) + i.startCPUProfile(snapshotsProfileName) + i.writeHeapProfile(snapshotsProfileName) } func (i *instrumentationContext) bootstrapSnapshotsCompleted() { @@ -112,16 +106,16 @@ func (i *instrumentationContext) bootstrapSnapshotsCompleted() { i.bootstrapSnapshotsDuration.Record(duration) i.log.Info("read snapshots done", zap.Duration("took", duration)) i.span.LogFields(opentracinglog.String("event", "read_snapshots_done")) - i.stopCPUProfileIfEnabled() - i.writeHeapProfileIfEnabled(snapshotsProfileName) + i.stopCPUProfile() + i.writeHeapProfile(snapshotsProfileName) } func (i *instrumentationContext) readCommitLogStarted() { i.log.Info("read commit log start") i.span.LogFields(opentracinglog.String("event", "read_commitlog_start")) i.start = i.nowFn() - i.startCPUProfileIfEnabled(readProfileName) - i.writeHeapProfileIfEnabled(readProfileName) + i.startCPUProfile(readProfileName) + i.writeHeapProfile(readProfileName) } func (i *instrumentationContext) readCommitLogCompleted() { @@ -129,8 +123,8 @@ func (i *instrumentationContext) readCommitLogCompleted() { i.bootstrapCommitLogDuration.Record(duration) i.log.Info("read commit log done", zap.Duration("took", duration)) i.span.LogFields(opentracinglog.String("event", "read_commitlog_done")) - i.stopCPUProfileIfEnabled() - i.writeHeapProfileIfEnabled(readProfileName) + i.stopCPUProfile() + i.writeHeapProfile(readProfileName) } type instrumentation struct { diff --git a/src/dbnode/storage/bootstrap/bootstrapper/fs/source_instrumentation.go b/src/dbnode/storage/bootstrap/bootstrapper/fs/source_instrumentation.go index 7016f6a6de..e69d4406b1 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/fs/source_instrumentation.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/fs/source_instrumentation.go @@ -79,29 +79,23 @@ func (i *instrumentationContext) finish() { i.span.Finish() } -func (i *instrumentationContext) startCPUProfileIfEnabled(name string) { - if i.pOpts.Enabled() { - err := i.pOpts.Profiler().StartCPUProfile(name) - if err != nil { - i.log.Error("unable to start cpu profile", zap.Error(err)) - } +func (i *instrumentationContext) startCPUProfile(name string) { + err := i.pOpts.Profiler().StartCPUProfile(name) + if err != nil { + i.log.Error("unable to start cpu profile", zap.Error(err)) } } -func (i *instrumentationContext) stopCPUProfileIfEnabled() { - if i.pOpts.Enabled() { - if err := i.pOpts.Profiler().StopCPUProfile(); err != nil { - i.log.Error("unable to stop cpu profile", zap.Error(err)) - } +func (i *instrumentationContext) stopCPUProfile() { + if err := i.pOpts.Profiler().StopCPUProfile(); err != nil { + i.log.Error("unable to stop cpu profile", zap.Error(err)) } } -func (i *instrumentationContext) writeHeapProfileIfEnabled(name string) { - if i.pOpts.Enabled() { - err := i.pOpts.Profiler().WriteHeapProfile(name) - if err != nil { - i.log.Error("unable to write heap profile", zap.Error(err)) - } +func (i *instrumentationContext) writeHeapProfile(name string) { + err := i.pOpts.Profiler().WriteHeapProfile(name) + if err != nil { + i.log.Error("unable to write heap profile", zap.Error(err)) } } @@ -109,8 +103,8 @@ func (i *instrumentationContext) bootstrapDataStarted() { i.log.Info("bootstrapping time series data start", i.logFields...) i.span.LogFields(opentracinglog.String("event", "bootstrap_data_start")) i.start = i.nowFn() - i.startCPUProfileIfEnabled(dataProfile) - i.writeHeapProfileIfEnabled(dataProfile) + i.startCPUProfile(dataProfile) + i.writeHeapProfile(dataProfile) } func (i *instrumentationContext) bootstrapDataCompleted() { @@ -119,16 +113,16 @@ func (i *instrumentationContext) bootstrapDataCompleted() { i.log.Info("bootstrapping time series data success", append(i.logFields, zap.Duration("took", duration))...) i.span.LogFields(opentracinglog.String("event", "bootstrap_data_done")) - i.stopCPUProfileIfEnabled() - i.writeHeapProfileIfEnabled(dataProfile) + i.stopCPUProfile() + i.writeHeapProfile(dataProfile) } func (i *instrumentationContext) bootstrapIndexStarted() { i.log.Info("bootstrapping index metadata start") i.span.LogFields(opentracinglog.String("event", "bootstrap_index_start")) i.start = i.nowFn() - i.startCPUProfileIfEnabled(indexProfile) - i.writeHeapProfileIfEnabled(indexProfile) + i.startCPUProfile(indexProfile) + i.writeHeapProfile(indexProfile) } func (i *instrumentationContext) bootstrapIndexCompleted() { @@ -136,8 +130,8 @@ func (i *instrumentationContext) bootstrapIndexCompleted() { i.bootstrapIndexDuration.Record(duration) i.log.Info("bootstrapping index metadata success", zap.Duration("took", duration)) i.span.LogFields(opentracinglog.String("event", "bootstrap_index_done")) - i.stopCPUProfileIfEnabled() - i.writeHeapProfileIfEnabled(indexProfile) + i.stopCPUProfile() + i.writeHeapProfile(indexProfile) } type instrumentation struct { diff --git a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_instrumentation.go b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_instrumentation.go index 96950a038d..18f715aac1 100644 --- a/src/dbnode/storage/bootstrap/bootstrapper/peers/source_instrumentation.go +++ b/src/dbnode/storage/bootstrap/bootstrapper/peers/source_instrumentation.go @@ -71,29 +71,23 @@ func (i *instrumentationContext) finish() { i.span.Finish() } -func (i *instrumentationContext) startCPUProfileIfEnabled(name string) { - if i.pOpts.Enabled() { - err := i.pOpts.Profiler().StartCPUProfile(name) - if err != nil { - i.log.Error("unable to start cpu profile", zap.Error(err)) - } +func (i *instrumentationContext) startCPUProfile(name string) { + err := i.pOpts.Profiler().StartCPUProfile(name) + if err != nil { + i.log.Error("unable to start cpu profile", zap.Error(err)) } } -func (i *instrumentationContext) stopCPUProfileIfEnabled() { - if i.pOpts.Enabled() { - if err := i.pOpts.Profiler().StopCPUProfile(); err != nil { - i.log.Error("unable to stop cpu profile", zap.Error(err)) - } +func (i *instrumentationContext) stopCPUProfile() { + if err := i.pOpts.Profiler().StopCPUProfile(); err != nil { + i.log.Error("unable to stop cpu profile", zap.Error(err)) } } -func (i *instrumentationContext) writeHeapProfileIfEnabled(name string) { - if i.pOpts.Enabled() { - err := i.pOpts.Profiler().WriteHeapProfile(name) - if err != nil { - i.log.Error("unable to write heap profile", zap.Error(err)) - } +func (i *instrumentationContext) writeHeapProfile(name string) { + err := i.pOpts.Profiler().WriteHeapProfile(name) + if err != nil { + i.log.Error("unable to write heap profile", zap.Error(err)) } } @@ -101,8 +95,8 @@ func (i *instrumentationContext) bootstrapDataStarted() { i.log.Info("bootstrapping time series data start") i.span.LogFields(opentracinglog.String("event", "bootstrap_data_start")) i.start = i.nowFn() - i.startCPUProfileIfEnabled(dataProfile) - i.writeHeapProfileIfEnabled(dataProfile) + i.startCPUProfile(dataProfile) + i.writeHeapProfile(dataProfile) } func (i *instrumentationContext) bootstrapDataCompleted() { @@ -110,16 +104,16 @@ func (i *instrumentationContext) bootstrapDataCompleted() { i.bootstrapDataDuration.Record(duration) i.log.Info("bootstrapping time series data success", zap.Duration("took", duration)) i.span.LogFields(opentracinglog.String("event", "bootstrap_data_done")) - i.stopCPUProfileIfEnabled() - i.writeHeapProfileIfEnabled(dataProfile) + i.stopCPUProfile() + i.writeHeapProfile(dataProfile) } func (i *instrumentationContext) bootstrapIndexStarted() { i.log.Info("bootstrapping index metadata start") i.span.LogFields(opentracinglog.String("event", "bootstrap_index_start")) i.start = i.nowFn() - i.startCPUProfileIfEnabled(indexProfile) - i.writeHeapProfileIfEnabled(indexProfile) + i.startCPUProfile(indexProfile) + i.writeHeapProfile(indexProfile) } func (i *instrumentationContext) bootstrapIndexCompleted() { @@ -127,8 +121,8 @@ func (i *instrumentationContext) bootstrapIndexCompleted() { i.bootstrapIndexDuration.Record(duration) i.log.Info("bootstrapping index metadata success", zap.Duration("took", duration)) i.span.LogFields(opentracinglog.String("event", "bootstrap_index_done")) - i.stopCPUProfileIfEnabled() - i.writeHeapProfileIfEnabled(indexProfile) + i.stopCPUProfile() + i.writeHeapProfile(indexProfile) } type instrumentationReadShardsContext struct { diff --git a/src/dbnode/storage/profiler/options.go b/src/dbnode/storage/profiler/options.go index c9460f92eb..764d305505 100644 --- a/src/dbnode/storage/profiler/options.go +++ b/src/dbnode/storage/profiler/options.go @@ -20,33 +20,15 @@ package profiler -import "fmt" - type options struct { - enabled bool profiler Profiler } // NewOptions creates new profiler options. func NewOptions() Options { - return &options{} -} - -func (o *options) Validate() error { - if o.enabled && o.profiler == nil { - return fmt.Errorf("profiler is enabled but is not set") + return &options{ + profiler: NewNoOpProfiler(), } - return nil -} - -func (o *options) Enabled() bool { - return o.enabled -} - -func (o *options) SetEnabled(value bool) Options { - opts := *o - opts.enabled = value - return &opts } func (o *options) Profiler() Profiler { diff --git a/src/dbnode/storage/profiler/profiler.go b/src/dbnode/storage/profiler/profiler.go index 46adfc7538..32933a317e 100644 --- a/src/dbnode/storage/profiler/profiler.go +++ b/src/dbnode/storage/profiler/profiler.go @@ -21,156 +21,25 @@ // Package profiler contains the code used for profiling. package profiler -import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" - "runtime/pprof" - "sync/atomic" -) +// NoOpProfiler is default disabled profiler. +type NoOpProfiler struct{} -const ( - // ProfileFileExtension is the extension of profile files. - ProfileFileExtension = ".pb.gz" -) - -// FileProfiler is profiler which writes its profiles to given path directory. -type FileProfiler struct { - path string - profileNames map[string]*profileName -} - -// NewFileProfiler creates a new file provider. -func NewFileProfiler(path string) *FileProfiler { - return &FileProfiler{ - path: path, - profileNames: make(map[string]*profileName), - } +// NewNoOpProfiler creates a new no-op profiler. +func NewNoOpProfiler() *NoOpProfiler { + return &NoOpProfiler{} } // StartCPUProfile starts named cpu profile. -func (f FileProfiler) StartCPUProfile(name string) error { - profileName := f.profileName(name) - return startCPUProfile(f.path, profileName) +func (f NoOpProfiler) StartCPUProfile(name string) error { + return nil } // StopCPUProfile stops started cpu profile. -func (f FileProfiler) StopCPUProfile() error { - stopCPUProfile() +func (f NoOpProfiler) StopCPUProfile() error { return nil } // WriteHeapProfile writes named heap profile. -func (f FileProfiler) WriteHeapProfile(name string) error { - profileName := f.profileName(name) - return writeHeapProfile(f.path, profileName) -} - -func (f FileProfiler) profileName(name string) *profileName { - profileName, ok := f.profileNames[name] - if !ok { - profileName = newProfileName(name) - f.profileNames[name] = profileName - } - return profileName -} - -type profileType int - -// String returns string representation of profile type. -func (p profileType) String() string { - switch p { - case cpuProfile: - return "cpu" - case heapProfile: - return "heap" - default: - return "" - } -} - -const ( - cpuProfile profileType = iota - heapProfile -) - -type profileName struct { - name string - counts map[profileType]*int32 -} - -func newProfileName(name string) *profileName { - return &profileName{ - name: name, - counts: make(map[profileType]*int32), - } -} - -func (p *profileName) inc(pType profileType) int32 { - count, ok := p.counts[pType] - if !ok { - count = new(int32) - p.counts[pType] = count - } - return atomic.AddInt32(count, 1) -} - -func (p *profileName) withProfileType(pType profileType) string { - return p.name + "." + pType.String() -} - -func startCPUProfile(path string, profileName *profileName) error { - file, err := newProfileFile(path, profileName, cpuProfile) - if err != nil { - return err - } - - for { - if err := pprof.StartCPUProfile(file); err != nil { - // cpu profile is already started, so we stop it and start our own. - pprof.StopCPUProfile() - continue - } - return nil - } -} - -func stopCPUProfile() { - pprof.StopCPUProfile() -} - -func writeHeapProfile(path string, profileName *profileName) error { - file, err := newProfileFile(path, profileName, heapProfile) - if err != nil { - return err - } - - return pprof.WriteHeapProfile(file) -} - -// newProfileFile creates new file for writing bootstrap profile. -// path is a directory where profile files will be put. -// If path is empty string, temp directory will be used instead. -func newProfileFile(path string, profileName *profileName, pType profileType) (*os.File, error) { - if path == "" { - tmpDir, err := ioutil.TempDir("", "profile-") - if err != nil { - return nil, err - } - path = tmpDir - } - - if err := os.MkdirAll(path, os.ModePerm); err != nil { - return nil, err - } - - for { - filename := fmt.Sprintf("%s.%d%s", - profileName.withProfileType(pType), profileName.inc(pType), ProfileFileExtension) - // fails if a file with given name exists - if file, err := os.OpenFile(filepath.Clean(filepath.Join(path, filename)), os.O_CREATE|os.O_EXCL, 0600); err == nil { - return file, nil - } - } +func (f NoOpProfiler) WriteHeapProfile(name string) error { + return nil } diff --git a/src/dbnode/storage/profiler/profiler_test.go b/src/dbnode/storage/profiler/profiler_test.go index fa95080833..f34ed8692e 100644 --- a/src/dbnode/storage/profiler/profiler_test.go +++ b/src/dbnode/storage/profiler/profiler_test.go @@ -21,70 +21,17 @@ package profiler import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" "testing" - "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -var tmpDir string - -func TestMain(m *testing.M) { - if err := setup(); err != nil { - fmt.Println("setup error") - os.Exit(1) - } - code := m.Run() - if err := teardown(); err != nil { - fmt.Println("teardown error") - os.Exit(1) - } - os.Exit(code) -} - -func setup() error { - tempDir, err := ioutil.TempDir("", "bootstrapProfileTests") - if err != nil { - return err - } - tmpDir = tempDir - return nil -} - -func teardown() error { - return os.RemoveAll(tmpDir) -} - -func TestFileProfile(t *testing.T) { - tests := []struct { - name string - fileNames []string - }{ - {name: "testProfile1", fileNames: []string{"testProfile1.cpu.1.pb.gz", "testProfile1.heap.1.pb.gz"}}, - {name: "testProfile1", fileNames: []string{"testProfile1.cpu.2.pb.gz", "testProfile1.heap.2.pb.gz"}}, - {name: "testProfile2", fileNames: []string{"testProfile2.cpu.1.pb.gz", "testProfile2.heap.1.pb.gz"}}, - } - sut := NewFileProfiler(tmpDir) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := sut.StartCPUProfile(tt.name) - require.NoError(t, err) - time.Sleep(100 * time.Millisecond) - err = sut.StopCPUProfile() - require.NoError(t, err) - err = sut.WriteHeapProfile(tt.name) - require.NoError(t, err) - - for _, fileName := range tt.fileNames { - fileInfo, err := os.Stat(filepath.Join(tmpDir, fileName)) - require.NoError(t, err) - assert.Equal(t, fileName, fileInfo.Name()) - } - }) - } +func TestNoOpProfile(t *testing.T) { + sut := NewNoOpProfiler() + err := sut.StartCPUProfile("foo") + require.NoError(t, err) + err = sut.StopCPUProfile() + require.NoError(t, err) + err = sut.WriteHeapProfile("bar") + require.NoError(t, err) } diff --git a/src/dbnode/storage/profiler/types.go b/src/dbnode/storage/profiler/types.go index f11b580902..dc025847ad 100644 --- a/src/dbnode/storage/profiler/types.go +++ b/src/dbnode/storage/profiler/types.go @@ -34,15 +34,6 @@ type Profiler interface { // Options represents the profiler options. type Options interface { - // Validate validates the options. - Validate() error - - // Enabled returns if profile is enabled. - Enabled() bool - - // SetEnabled enables profiling. - SetEnabled(value bool) Options - // Profiler returns the profiler. Profiler() Profiler