Skip to content

add flag to enable and set the mutex profile fraction #1969

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

Merged
merged 2 commits into from
Jan 15, 2020

Conversation

jtlisi
Copy link
Contributor

@jtlisi jtlisi commented Jan 10, 2020

What this PR does:

This PR adds the ability to enable/set mutex profiling in the go runtime.

Which issue(s) this PR fixes:

In the past when profiling cortex, especially the ingesters, I have wanted to view a mutex profile. This PR adds the ability to do that.

Checklist

  • CHANGELOG.md updated

CHANGELOG.md Outdated
@@ -14,6 +14,7 @@
* [CHANGE] Ingesters now write only normalised tokens to the ring, although they can still read denormalised tokens used by other ingesters. `-ingester.normalise-tokens` is now deprecated, and ignored. If you want to switch back to using denormalised tokens, you need to downgrade to Cortex 0.4.0. Previous versions don't handle claiming tokens from normalised ingesters correctly. #1809
* [CHANGE] Overrides mechanism has been renamed to "runtime config", and is now separate from limits. Runtime config is simply a file that is reloaded by Cortex every couple of seconds. Limits and now also multi KV use this mechanism.<br />New arguments were introduced: `-runtime-config.file` (defaults to empty) and `-runtime-config.reload-period` (defaults to 10 seconds), which replace previously used `-limits.per-user-override-config` and `-limits.per-user-override-period` options. Old options are still used if `-runtime-config.file` is not specified. This change is also reflected in YAML configuration, where old `limits.per_tenant_override_config` and `limits.per_tenant_override_period` fields are replaced with `runtime_config.file` and `runtime_config.period` respectively. #1749
* [FEATURE] The distributor can now drop labels from samples (similar to the removal of the replica label for HA ingestion) per user via the `distributor.drop-label` flag. #1726
* [FEATURE] Added flag to enable mutex profiling
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of things, please:

  1. Add the new flag name
  2. Add the PR

)
flag.StringVar(&configFile, "config.file", "", "Configuration file to load.")
flag.IntVar(&eventSampleRate, "event.sample-rate", 0, "How often to sample observability events (0 = never).")
flag.IntVar(&ballastBytes, "mem-ballast-size-bytes", 0, "Size of memory ballast to allocate.")
flag.IntVar(&mutexProfileFraction, "mutex-profile-fraction", 0, "Fraction at which mutex profile vents will be reported, 0 to disable")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we prefix it with debug. (so the flag will be -debug.mutex-profile-fraction) to explicitly state it's a flag just used for debugging?

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my feedback 🙏

@gouthamve gouthamve merged commit e319ef5 into master Jan 15, 2020
@gouthamve gouthamve deleted the 20190109_mutex_profile_flag branch January 15, 2020 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants