net/http/pprof: add seconds= to mutex/block profile handler

[lmb]

Using the mutex and block profiles from net/http/pprof currently requires modifying the application to call runtime.SetBlockProfileRate and runtime.SetMutexProfileFraction. Since it's not clear what the performance impact of enabling these profiles is, few people will actually do it.

I'd like to add the following parameters, to make using these two profiles easier to use:

/debug/pprof/block?seconds=1&rate=1
/debug/pprof/mutex?seconds=1&frac=1

If the seconds parameter isn't specified the endpoints will behave as they currently do. If it is specified the block profile rate or mutex profile fraction will be set (default 1) and then the handler sleeps for the specified duration. At the end of the duration the profile is written and the original profile rate / fraction is restored.

This proposal requires changing SetBlockProfileRate to return the previous rate, similar to SetMutexProfileFraction.

[rsc]

The profiles are reporting things that happened earlier in the program. The rate change should really happen once, at program startup. Otherwise if you hit the URL twice with different rates the second will report the first one's samples but scaled badly. Or do we now subtract inside the handler?

The defaults are zero, which is admittedly not very useful. Maybe we should fix that instead.

/cc @hyangah @pjweinbgo

[hyangah]

Yes, the current mutex/block profiling implementation doesn't work well with rate changes.

Inside google, we implemented custom handlers that take a 'seconds' parameter and return the delta, and they are served from separate endpoints -- the additional parameter changes the semantics of data so much that we thought they are better to be served from different endpoints.

BTW, I don't see code for scaling samples using the sampling rate and that is a bug.
(https://github.com/golang/go/blob/master/src/runtime/pprof/pprof.go#L357) Is it too late for 1.10?

[ianlancetaylor]

It's not absolutely too late, since this is a new feature, but it's awfully late.

[lmb]

I might be mistaken, but it looks like at least the mutex profile does scale samples (but ignores rate changes)?

It seems there are two options:

1. Enable block and mutex profiling by default. But what is the correct number?
2. Make current code work with rate changes and implement new (?) endpoints.

The former is a simple change but takes more work, the latter vice versa. I'd like to have mutex and block profiling work out of the box, so I'm happy to work on both.

[hyangah]

1. support for difference reporting (seconds parameter): I think this is reasonable, but first consider whether this can be implemented from the client side. I.e., capturing the profiles twice and then asking pprof to report difference (-base flag). Would it be too inconvenient?

2. support for changing the sampling rate on the fly: this is difficult. they are not meant to be changed.

3. default to non-zero values: can anyone add measurement and benchmark.

(fyi, the bug I mentioned in my previous comment is fixed now (for mutex profile) and is not relevant to this proposal.)

[rsc]

Maybe if they are off then second=XXX rate=XXX can turn them on and send a diff back?
We should probably add the diff code for Go 1.11 anyway.

/cc @hyangah.

[hyangah]

No objection. Just once the handler is hit with rate=XXX param, we may want to make it clear that the output from the default /debug/pprof/mutex endpoint is no longer interpretable.

[lmb]

I probably misunderstand how the whole profile machinery works under the hood. My initial proposal wasn't to enable differential profiling, but to make the mutex and block profile useable out of the box. My idea was to just toggle the them on / off on an as-needed basis. That was naive, for all the points you have pointed out.

As an application developer, I have no idea what the cost of enabling these profiles is. Should I sample one in thousand events? One in a hundred? The documentation isn't helping in that case. That's what I would like to solve.

Re finding a default value, I tried following @hyangah's suggestion to run a bunch of benchmarks (from https://github.com/golang/go/wiki/Benchmarks) and see how they change, but didn't get far due to noisy results. Is there a better corpus I could use?

[rsc]

@hyangah what do you think we should do here to make these profiles easy to use and helpful to developers?

[hyangah]

How about resetting the profile counters if the sampling period is changed?

Since we migrated all these profiles to the proto buf format, the output can include information like the collection time and the duration. For legacy format output, we can output the duration as a comment.

/debug/pprof/{mutex,block} endpoints keep reporting the counters since the current sampling rate is set.
The new rate=XXX param changes the sampling rate.

After talking with @pjweinbgo I became less optimistic about choosing the non-zero default values. Their impact varies a lot depending on applications. For example, benchmarks that don't involve many mutex will not be affected.

[lmb]

Sounds good to me. Would you add the seconds parameter as well?

P.S. As I said I'd like to take a stab at this, if you don't mind @hyangah ? I'd probably split this in two CLs:

• Reset profile data on rate change
• Changes to net/http/pprof to add new parameters

[gopherbot]

Change https://golang.org/cl/94335 mentions this issue: runtime: reset block, mutex profile counters after rate change

[hyangah]

There is concern about exposing the rate change functionality in production. Especially when the change can affect the server performance and when the service owner already tuned the parameters best for the service. I agree that's a dangerous feature.

@aalexand proposed an alternative that adds the 'seconds' parameter.

• If 'seconds' param is given, report diff.
• If the rate is 0 and 'seconds' param is given, set the rate for a predefined value (maybe 100 for mutex profile), report the diff after the seconds, and reset the rate back to 0.
• If the rate is 0 and 'seconds' param is not given, return zero output (the current behavior)

@lmb, does this still meet your use case?