-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
write_http timeout + low speed limit #985
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adding abort on stuck connection
Adding possibility to configure curl connection abort on slow connections. Reconnect on stuck or really slow connection sounds better than been stuck.
Indentation fix
Adding LowSpeedLimit and LowLimitBytesPerSec to spec
Adding PostTimeoutSec to set max timeout on http post
Changing default post timeout to plugin's interval and fixing low speed interval to use correct method for plugin interval (plugin_get_interval())
Conflicts: src/write_http.c Also slipped in a couple of conding-style consistentcy related changes.
- rename PostTimeoutSec parameter to Timeout - change Timeout to take milliseconds rather than seconds, for the sake of consistency: collectd allow working at the sub-second scale if need be, so it's a shame to restrain this here. - do *not* set the default timeout to the value of Interval. Rationale: what matters here is that each POST request completes before the send buffer fills up again. How fast the send buffer fills up completely depends on how much data is collected. Interval is unrelated to this, and unlike read plugins, I don't think it makes a good value to use as the default timeout.
- do not enable the feature with an on/off switch. Enable it by setting a positive value to the parameter instead. Therefore drop the arbitrary default value used previously. - rename LowLimitBytesPerSec to LowSpeedLimit. - make LowSpeedLimit refer to bytes per second rather than bytes per Interval.
... to avoid portability problems.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This continues the work initiated in #752, implementing @octo's recommendations when he reviewed the patchset. The goal stays the same (ie: resetting the connection when error conditions occur), how it's exposed to the user is a bit different.
@loginator17, @optimality, I'd love to have your feedback on this !