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

Move exec WaitGroup from Exec instance level to Gather function level. #1464

Closed
wants to merge 1 commit into from

Conversation

allen13
Copy link
Contributor

@allen13 allen13 commented Jul 7, 2016

Required for all PRs:

  • [ X] CHANGELOG.md updated
  • [ X] Sign CLA (if not already signed)
  • [ N/A] README.md updated (if adding a new plugin)

This pull-request addresses #1463: Shared WaitGroup in Exec plugin Bug

@@ -156,8 +152,6 @@ func gatherWithTimeout(
acc *accumulator,
timeout time.Duration,
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is no longer gatherWithTimeout as the timeout argument isn't used anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't mean for the last commit to end up in the pull request.

@@ -1,3 +1,7 @@
### Bugfixes

- [#1463](https://github.com/influxdata/telegraf/issues/1463): Shared WaitGroup in Exec plugin
Copy link
Contributor

Choose a reason for hiding this comment

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

this should go in the 1.0 bugfixes section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sparrc

this should go in the 1.0 bugfixes section

Why? this "bug" only happens if you run a modified version of telegraf.

@allen13 allen13 force-pushed the master branch 2 times, most recently from 3a0cc4d to d14c8a7 Compare July 11, 2016 17:35
@sparrc
Copy link
Contributor

sparrc commented Jul 14, 2016

please rebase and out "closes #1463" in your commit message, and then I'll merge

…. If Gather is run concurently the shared WaitGroup variable never finishes.
@allen13
Copy link
Contributor Author

allen13 commented Jul 14, 2016

CHANGELOG.md conflicts resolved.

@sparrc sparrc closed this in 1d9745e Jul 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants