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

fix(outputs/amqp): Close the last connection when writing error to avoid connection leaks #10360

Merged
merged 1 commit into from
Jan 6, 2022

Conversation

stitchcula
Copy link
Contributor

Required for all PRs:

  • Updated associated README.md. (Internal change, doesn't need to update README)
  • Wrote appropriate unit tests. (No new functionality)
  • Pull request title or commits are in conventional commit format

When writing errors, such as occasional writing timeouts, close the last connection to avoid connection leaks.

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Dec 31, 2021
@@ -260,6 +260,9 @@ func (q *AMQP) Write(metrics []telegraf.Metric) error {
return err
}
} else {
if err := q.client.Close(); err != nil {
q.Log.Errorf("Closing connection failed: %v", err)
}
q.client = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this - the GitHub diff looks weird, but this does look good to close the connection on the 2nd and additional publish attempts.

@powersj powersj added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jan 4, 2022
@sspaink sspaink merged commit 0b96d40 into influxdata:master Jan 6, 2022
powersj pushed a commit to powersj/telegraf that referenced this pull request Jan 21, 2022
reimda pushed a commit that referenced this pull request Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants