Skip to content

inputs.x509_cert: Fix timeout issue #8824

Merged
ivorybilled merged 4 commits intoinfluxdata:masterfrom
viperstars:bugfix/8809
Feb 16, 2021
Merged

inputs.x509_cert: Fix timeout issue #8824
ivorybilled merged 4 commits intoinfluxdata:masterfrom
viperstars:bugfix/8809

Conversation

@viperstars
Copy link
Copy Markdown
Contributor

@viperstars viperstars commented Feb 6, 2021 •
edited
Loading

closes #8809

If timeout is not configured, it will be set to "5ns" when adding to inputs. This cause the timeout issue #8809. Test case for reproducing is added in commit ac4cda1.

I fix the issue by setting timeout to "0s".

telegraf-tiger[bot]
telegraf-tiger bot approved these changes Feb 6, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

🤝 ✅ CLA has been signed. Thank you!

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Feb 6, 2021
srebhan
srebhan reviewed Feb 7, 2021
View reviewed changes
@srebhan srebhan self-assigned this Feb 7, 2021
@srebhan srebhan added fix pr to fix corresponding bug and removed feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Feb 7, 2021
@viperstars viperstars requested a review from srebhan February 8, 2021 02:44
srebhan
srebhan approved these changes Feb 8, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@srebhan srebhan 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 Feb 8, 2021
@sjwang90 sjwang90 changed the title Bugfix/8809 inputs.x509_cert: Fix timeout issue Feb 9, 2021
@helenosheaa
Copy link
Copy Markdown
Member

helenosheaa commented Feb 10, 2021

@viperstars looks good. Can you please add closes #8809 to your description so it will close the issue when we merge?

@viperstars viperstars requested a review from srebhan February 12, 2021 09:17
@viperstars
Copy link
Copy Markdown
Contributor Author

viperstars commented Feb 12, 2021

@viperstars looks good. Can you please add closes #8809 to your description so it will close the issue when we merge?

It is added in the new commit message now.

@srebhan
Copy link
Copy Markdown
Member

srebhan commented Feb 12, 2021

@viperstars I think github only evaluates the final commit message which is the PR description above. That's due to the fact that the changes are squashed before merge. So please simply open the PR at the github site and click edit in the top PR message. Then add the line closes #8809 and save. There is no need for a code change.

@viperstars
Copy link
Copy Markdown
Contributor Author

viperstars commented Feb 12, 2021 •
edited
Loading

@viperstars I think github only evaluates the final commit message which is the PR description above. That's due to the fact that the changes are squashed before merge. So please simply open the PR at the github site and click edit in the top PR message. Then add the line closes #8809 and save. There is no need for a code change.

It is updated now. Thanks for the tip.

srebhan
srebhan approved these changes Feb 15, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Perfect.

@ivorybilled ivorybilled merged commit fe16d56 into influxdata:master Feb 16, 2021
ssoroka pushed a commit that referenced this pull request Feb 17, 2021
@viperstars @ssoroka
inputs.x509_cert: Fix timeout issue (#8824)
341aca6 
* reproduce timeout issue #8809

* fix timeout issue #8809

* set default timeout to 5s

* closes #8809

Co-authored-by: 彭浩 <penghao@qiyi.com>
(cherry picked from commit fe16d56)
@viperstars viperstars deleted the bugfix/8809 branch February 18, 2021 05:52
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@viperstars
inputs.x509_cert: Fix timeout issue (influxdata#8824)
749a28d 
* reproduce timeout issue influxdata#8809

* fix timeout issue influxdata#8809

* set default timeout to 5s

* closes influxdata#8809

Co-authored-by: 彭浩 <penghao@qiyi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srebhan srebhan srebhan approved these changes

@telegraf-tiger telegraf-tiger[bot] telegraf-tiger[bot] approved these changes

+2 more reviewers

@helenosheaa helenosheaa helenosheaa approved these changes

@ivorybilled ivorybilled ivorybilled approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@srebhan srebhan

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

telegraf plugin x509_cert stopped working from 1.17.1 upwards with dial tcp: i/o timeout

4 participants

@viperstars @helenosheaa @srebhan @ivorybilled