Skip to content

Commit

Permalink
profiler: suppress erorrs if the profiler is stopped
Browse files Browse the repository at this point in the history
The fix in #2865 was intended to suppress spurious metrics profile
errors when the profiler is stopped. It did so by relaxing the
one-second duration constraint of the metrics profiler. However,
the Windows system timer resolution is about 15 milliseconds (see
https://learn.microsoft.com/en-us/windows-hardware/drivers/kernel/high-resolution-timers#controlling-timer-accuracy)
This caused the metrics profile tests from #2865 to fail because the
metrics profiler will likely be stopped in less than 15 milliseconds,
meaning we'll see 0 duration between profile collection and log an
error.

This commit actually suppresses the error by checking whether the
profiler was stopped (meaning interruptibleSleep was interrupted). If
so, and if the metrics profiler returned an error, we instead return a
sentinel error indicating that profiling was stopped. If we see that
error, we just drop the profile and don't log an error. We won't upload
the profile anyway. This way, we should only report an error from the
metrics profiler if there is _actually_ a problem with the timer.
  • Loading branch information
nsrip-dd committed Sep 24, 2024
1 parent eef52d3 commit e398c9c
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
8 changes: 6 additions & 2 deletions profiler/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ var profileTypes = map[ProfileType]profileType{
Filename: "metrics.json",
Collect: func(p *profiler) ([]byte, error) {
var buf bytes.Buffer
p.interruptibleSleep(p.cfg.period)
if p.interruptibleSleep(p.cfg.period) {
return nil, errProfilerStopped
}
err := p.met.report(now(), &buf)
return buf.Bytes(), err
},
Expand Down Expand Up @@ -343,7 +345,9 @@ func (p *profiler) runProfile(pt ProfileType) ([]*profile, error) {
start := now()
t := pt.lookup()
data, err := t.Collect(p)
if err != nil {
if err == errProfilerStopped {
return nil, nil
} else if err != nil {
return nil, err
}
end := now()
Expand Down
9 changes: 8 additions & 1 deletion profiler/profiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ var (
activeProfiler *profiler
containerID = internal.ContainerID() // replaced in tests
entityID = internal.EntityID() // replaced in tests

// errProfilerStopped is a sentinel for suppressng errors if we are
// about to stop the profiler
errProfilerStopped = errors.New("profiler stopped")
)

// Start starts the profiler. If the profiler is already running, it will be
Expand Down Expand Up @@ -480,10 +484,13 @@ func (p *profiler) outputDir(bat batch) error {

// interruptibleSleep sleeps for the given duration or until interrupted by the
// p.exit channel being closed.
func (p *profiler) interruptibleSleep(d time.Duration) {
// Returns whether the sleep was interrupted
func (p *profiler) interruptibleSleep(d time.Duration) bool {
select {
case <-p.exit:
return true
case <-time.After(d):
return false
}
}

Expand Down

0 comments on commit e398c9c

Please sign in to comment.