Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

Update profiletrigger: switch from vsz to rss and add threshold for heap as well#1914

Merged
Dieterbe merged 3 commits intomasterfrom
update-profiletrigger
Oct 14, 2020
Merged

Update profiletrigger: switch from vsz to rss and add threshold for heap as well#1914
Dieterbe merged 3 commits intomasterfrom
update-profiletrigger

Conversation

@Dieterbe
Copy link
Copy Markdown
Contributor

@Dieterbe Dieterbe commented Oct 2, 2020

  • old version used to look at Memstats.Sys, which was the entire virtual
    memory space, and effectively useless. Hence also why it's been hidden
    in the dashboard since forever. Most people expect this would
    look effectively at rss, so that's what we do now. + optional
    parameter to look at (only) the heap specifically
  • change default collection interval to 10s. That's what we've been
    running in prod for years without any issues.

* old version used to look at Memstats.Sys, which was the entire virtual
  memory space, and effectively useless. Hence also why it's been hidden
  in the dashboard since forever.   Most people expect this would
  look effectively at rss, so that's what we do now. + optional
  parameter to look at (only) the heap specifically
* change default collection interval to 10s. That's what we've been
  running in prod for years without any issues.
@Dieterbe Dieterbe force-pushed the update-profiletrigger branch from 2b28318 to d4d8971 Compare October 2, 2020 14:16
Comment thread cmd/metrictank/metrictank.go Outdated
Copy link
Copy Markdown
Contributor

@robert-milan robert-milan left a comment

Choose a reason for hiding this comment

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

Just the change for clarification already noted, other than that looks fine, but I haven't reviewed the library code from profiletrigger so I am unaware of all those changes. Would you like me to take a look at those?

@Dieterbe
Copy link
Copy Markdown
Contributor Author

Would you like me to take a look at those?

nah those went through review already in that repo.

@Dieterbe Dieterbe merged commit 63bf743 into master Oct 14, 2020
@Dieterbe Dieterbe deleted the update-profiletrigger branch October 14, 2020 15:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants