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

New parameter to force the interval of gather for sysstat #4068

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

adrianlzt
Copy link
Contributor

Added new config parameter for sysstat plugin, sadc_interval, to force
the gather interval of the sadc command.

This is useful when we have configured jitter in the agent, because
jitter breaks the measurement of the interval used in sadc.

Example:

  • agent with interval=10, jitter=5
  • sysstat is started in t=0 and runs sadc with interval=1
  • next execution of sysstat is in t=14 (interval + some jitter)
  • sysstat plugin set the value of "s.interval" to 14s
  • sadc is run with interval=13 (s.interval - parseInterval)
  • sysstat plugin is killed because is taking longer that collection
    interval

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Added new config parameter for sysstat plugin, sadc_interval, to force
the gather interval of the sadc command.

This is useful when we have configured jitter in the agent, because
jitter breaks the measurement of the interval used in sadc.

Example:
 - agent with interval=10, jitter=5
 - sysstat is started in t=0 and runs sadc with interval=1
 - next execution of sysstat is in t=14 (interval + some jitter)
 - sysstat plugin set the value of "s.interval" to 14s
 - sadc is run with interval=13 (s.interval - parseInterval)
 - sysstat plugin is killed because is taking longer that collection
 interval
@danielnelson
Copy link
Contributor

Thanks @adrianlzt, I've been wondering if we should always require a hard timeout, and completely remove the automatic computation of the interval size. We would of course set a default timeout so it would still remain optional, but the timeout would not be dependent on the first collection. Do you think this is a good idea?

@danielnelson danielnelson added this to the 1.7.0 milestone Apr 24, 2018
@danielnelson
Copy link
Contributor

closes #3840

@adrianlzt
Copy link
Contributor Author

The computation of the interval size is a little bit hacky and I guess it will cause problems, because of the jitter or because sadc or sadf being slow.

I guess a fixed interval could be useful, but you are going to miss some data between runs.

Maybe allowing to use this plugin in two different forms could be better.
First one with a fixed interval (maybe with a default of 8s and warning if you lower the agent interval is going to fail).
And for the second one you will need to have sar running in your system and the plugin just issuing commands with sar -s HH:MM:SS -e HH:MM:SS storing the last end value.

@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Apr 26, 2018
@danielnelson danielnelson merged commit 46a8bdb into influxdata:master Apr 26, 2018
@danielnelson
Copy link
Contributor

I confess I'm not up to speed on how sysstat works. This is a step in the right direction but I will probably need more advice at some point in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants