-
Notifications
You must be signed in to change notification settings - Fork 0
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
Increase default connection timeout #152
Increase default connection timeout #152
Conversation
Signed-off-by: Andrey Butusov <[email protected]>
Signed-off-by: Andrey Butusov <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #152 +/- ##
======================================
Coverage 3.03% 3.03%
======================================
Files 18 18
Lines 987 987
======================================
Hits 30 30
Misses 955 955
Partials 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is an imbalance now between chain.dial_timeout
and metrics.interval
which can lead to resource leak. In general, dial_timeout
can't be less than the interval
since new scraping attempt shouldn't be performed before the old one has finished. Either it's handled by exported internally (skip the interval
tick if still collecting), or we shouldn't have this 60/15s relation in the config.
@532910, @smallhive?
cmd/neo-exporter/config.go
Outdated
@@ -33,7 +33,7 @@ const ( | |||
|
|||
func DefaultConfiguration(cfg *viper.Viper) { | |||
cfg.SetDefault(prefix+delimiter+cfgNeoRPCEndpoint, "") | |||
cfg.SetDefault(prefix+delimiter+cfgNeoRPCDialTimeout, 5*time.Second) | |||
cfg.SetDefault(prefix+delimiter+cfgNeoRPCDialTimeout, time.Minute) | |||
|
|||
cfg.SetDefault(cfgMetricsEndpoint, ":16512") | |||
cfg.SetDefault(cfgMetricsInterval, 15*time.Minute) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is for timeout changes, let's add one more commit and change this to 15*time.Second
.
The example YAML config contains 15s
and I doubt 15 minutes is a good interval between metric gathering for default value 😅
The collection ticks executes after interval, but only if the previous execution is done. It works because we do it inside one for. |
Signed-off-by: Andrey Butusov <[email protected]>
OK then. |
Closes #147.
As I understand it, there is only a connection to the server and the value has increased significantly. Maybe put 15 seconds?