Skip to content
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

(#780) puppetserver: don't purge metrics.conf / Use dedicated parameter for jruby profiler #781

Merged
merged 3 commits into from
Mar 29, 2021

Conversation

bastelfreak
Copy link
Member

This introduces a new parameter to disable the profiling indepently from
the metrics. Previously this was enabled by default. However Puppet Inc.
disables it in their standard configuration and the profiler causes a
significate memory and CPU overhead so I guess we should stick with the
upstream default.

@bastelfreak
Copy link
Member Author

that's my (currently untested) approach to fix #780 . Let me know what you think / if you like two PRs for this.

@bastelfreak bastelfreak force-pushed the do-not-remove-metrics-tmu branch 2 times, most recently from 4983f58 to fe45a60 Compare March 24, 2021 08:09
@ekohl
Copy link
Member

ekohl commented Mar 24, 2021

Other than the failing unit tests, I think this looks good. I already started to change the odd Optional[Boolean] in #779 anyway.

This introduces a new parameter to disable the profiling indepently from
the metrics. Previously this was enabled by default. However Puppet Inc.
disables it in their standard configuration and the profiler causes a
significate memory and CPU overhead so I guess we should stick with the
upstream default.
This parameter only enables/disables the *http-client* metrics within
puppetserver. It's disabled by default in the Puppet Inc. config file.
@bastelfreak bastelfreak force-pushed the do-not-remove-metrics-tmu branch from 6f37175 to 44a0400 Compare March 29, 2021 08:32
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Looks good to me, let's see if the test agree.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Test failures look unrelated, merging.

@ekohl ekohl linked an issue Mar 29, 2021 that may be closed by this pull request
@ekohl ekohl merged commit e799269 into theforeman:master Mar 29, 2021
@bastelfreak bastelfreak deleted the do-not-remove-metrics-tmu branch March 29, 2021 11:16
@theforeman theforeman deleted a comment from alexwrinner Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

puppet::server_puppetserver_metrics: false causes puppetserver to fail
4 participants