-
Notifications
You must be signed in to change notification settings - Fork 342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(cli): A: Add ability to dump pprof data to logs ... #3454
Conversation
@aaron-kasten The steps below allow you to directly get pprof files in (binary) protobuf format that can be then directly loaded in Would that approach work for your use case? if not, why not?
Other kopia pprof endpoints: |
There are disadvantages to using the publish URL and poll vs. the method proposed in this PR train:
|
@aaron-kasten I see your point. It makes sense. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3454 +/- ##
==========================================
- Coverage 75.85% 75.84% -0.02%
==========================================
Files 465 465
Lines 37166 37166
==========================================
- Hits 28193 28188 -5
- Misses 7047 7048 +1
- Partials 1926 1930 +4 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Shikhar Mall <mall.shikhar.in@gmail.com>
Co-authored-by: Shikhar Mall <mall.shikhar.in@gmail.com>
Co-authored-by: Shikhar Mall <mall.shikhar.in@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…to pprof_extensions-A
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No request for immediate changes.
@aaron-kasten There are a number of inline questions, PTAL.
Merging as is, we can come back and iterate.
_, ok := v.GetValue(KopiaDebugFlagForceGc) | ||
if ok { | ||
log(ctx).Debug("performing GC before PPROF dump ...") | ||
runtime.GC() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should KopiaDebugFlagForceGc
be a "global" flag that applies to all profiles?
or
Should runtime.GC()
be only called once even if multiple profiles specify the flag?
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to ensure that GC was run immediately before any memory profiling. This will allow users to check for memory leaks. There's a chance that other operations in the loop will allocate additional memory that I did not want to show up on the profile.
I did not want to special case which profiles allowed GC because PPROF dumps can be added by users - and there's no determining which profiles will benefit from an immediate GC. I could not conceive a scenario where only one GC would be required to produce accurate results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asking because it is not clear to me whether a GC is needed before dumping each profile, or what difference it makes. To be clear, I don't have a good answer.
When stopping and dumping the profiles, it seems that a possible sequence would be
GC
-> dump profile 1
-> GC -> dump profile 2
-> GC
-> dump profile 3
In this case profile 2 would contain some information (allocations or side effects) coming from performing dump profile 1
. Similarly, profile 3 would contain information coming from dumping profiles 1 and 2.
An alternative sequence would be.
GC
-> dump profile 1
, dump profile 2
-> dump profile 3
Not sure if it would make a difference, or if it matters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway 🤷🏼♂️
MaybeStartPofileBuffers
and related refactoring
aaron-kasten/kopia#3
This PR adds the ability to dump pprof data to logs for debugging.
This is one of 4 PRs in a PR train:
aaron-kasten/kopia:pprof-extensions-A
aaron-kasten/kopia:pprof-extensions-B
aaron-kasten/kopia:pprof-extensions-C
aaron-kasten/kopia:pprof-extensions-D
Usage
pprof dumps are configured using the
KOPIA_DEBUG_PPROF
environment variable. The variable is a list of pprof profile names (seepprof.Lookup
) separated by,
. Optional parameters can be set with '=', delimited by ':'.example:
export KOPIA_DEBUG_PPROF=cpu,heap=debug=1,mutex=debug=1:rate=1000
The above setting will produce CPU, heap and mutex profiles. The block profile will have its debug parameter set to 1 and its sample rate set to 1000
Once run, profile data will be output in the Kopia logs on termination. Profile dumps are generated as base64 output (PEM) to the log on termination.
You should consider captureing logs to a file when running the Kopia command:
Once the logs are captured, a dump can created by terminating the command:
The following signals (on Linux and macos) can be used to dump profiles: SIGTERM, SIGINT, and SIGUSR1.
Captured standard-output should look similar to:
The captured output can then be converted to a pprof binary by using
kats
. The Kopiakats
tool can be used to convert the PEM file into a binary:When successful, kats will output the file found in the capture file.
kats expects that there is a well formed PEM record in the capture file.
Once successful, the binary can be used in PPROF:
Configuration Options
TBD