-
-
Notifications
You must be signed in to change notification settings - Fork 267
Adding support for WriteQueueLimitLow & WriteQueueLimitHigh #178
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
Conversation
…re added in collectd 5.4
it would be nice to check $::collectd_version if the options can be added to the config |
@mmoll Any preference in how that is done? If either setting is set, we could then also check the collectd_version. What would the failure case look like? Should we use fail or err with a log message? |
There is also an order of operations issue there. If the package isn't installed yet, $::collectd_version will return nil. We could try and use $version, but that can be set to installed in default cases. Having the setting in place doesn't break older clients. Perhaps it may just make more sense to add a comment on the class parameters? |
I don't think there was ever a formal discussion about that, so just as a reference point one place I know of having it implemented is there: https://github.com/pdxcat/puppet-module-collectd/blob/master/templates/plugin/df.conf.erb#L12 |
The issue you are pinpointing with $::collectd_version not being known at the first run is already reported in #162. We do not have an ideal solution yet for that, but your input is welcome :) |
Ping, please add the $::collectd_version check so we can merge this. Thanks |
@joshgarnett ping |
Adding support for WriteQueueLimitLow & WriteQueueLimitHigh
ok thanks for clearing that up. Thanks for the contribution. |
WriteQueueLimitLow & WriteQueueLimitHigh are very useful config options that were added in collectd 5.4. If for any reason the output plugin can't send events it'll prevent the collectd processes from slowly consuming all of the memory on the server.