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.

Sign up for GitHub

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

Jump to bottom

Handle per-plugin flush jitter, fixes #6569 #6603

Merged
merged 1 commit into from Nov 13, 2019
Merged

Handle per-plugin flush jitter, fixes #6569 #6603

merged 1 commit into from Nov 13, 2019

Conversation

dbutler-starry
Copy link
Contributor

@dbutler-starry dbutler-starry commented Oct 31, 2019 •
edited by danielnelson

This supports setting unique flush intervals per output plugin.

closes #6569

Required for all PRs:

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

@dbutler-starry
Copy link
Contributor Author

I had some internal debate about the data type of FlushJitter in the OutputConfig.

The Agent relies on default values for other output configs (e.g. FlushInterval) to indicate that the user has not specified an override value, but the default value for FlushJitter is actually a valid value (i.e. the user could specify a jitter of 0).

For this reason I decided to implement FlushInterval as an interface to imitate "optional" behaviour. I'm open to other options/opinions about different ways to do this.

@dbutler-starry dbutler-starry marked this pull request as ready for review October 31, 2019 18:25
@danielnelson
Copy link
Contributor

Would it work/does it make sense for the value to be a pointer? *internal.Duration

@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Nov 8, 2019
@danielnelson danielnelson added this to the 1.13.0 milestone Nov 8, 2019
@dbutler-starry
Copy link
Contributor Author

It could be done with a pointer, do you like the look of this better?

@danielnelson
Copy link
Contributor

Yeah, looks good, can you clear up the conflict and then I can merge.

@dbutler-starry
Copy link
Contributor Author

👍 Fixed

@danielnelson danielnelson merged commit 2156a62 into influxdata:master Nov 13, 2019
@adisharoon adisharoon mentioned this pull request Nov 21, 2019
Closed
@dbutler-starry dbutler-starry deleted the dbutler/per-plugin-flush-jitter branch March 9, 2020 20:09
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
@dbutler-starry
Add support for per output flush jitter (influxdata#6603)
b1f0c57
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
@dbutler-starry
Add support for per output flush jitter (influxdata#6603)
b171815
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@dbutler-starry @danielnelson
Add support for per output flush jitter (influxdata#6603)
362c688
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.

Allow unique flush_jitter values for different output plugins
2 participants
@dbutler-starry @danielnelson