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

Prevent connection error overflow on windows #11406

Closed
wants to merge 2 commits into from
Closed

Prevent connection error overflow on windows #11406

wants to merge 2 commits into from

Conversation

leopucci
Copy link
Contributor

@leopucci leopucci commented Mar 22, 2019

This small modification assure that Metricbeat closes the connections in case of errors. As reported by #11393.
The thing is that on windows you have to install openssl to enable postgresql secure connections, the default is to use insecure connection. While pq library uses by default secure connections.
So the library is throwing an error: pq: SSL is not enabled on the server and not closing connections resulting on an overflow of connections if you set the period below 5 seconds.

This small modification assure that Metricbeat closes the connections in case of errors. As reported by #11393. 
The thing is that on windows you have to install openssl to enable postgresql secure connections, the default is to use insecure connection. While pq library uses by default security connections. 
So the library is throwing an error: pq: SSL is not enabled on the server and not closing connections resulting on an overflow of connections if you set the period below 5 seconds.
@leopucci leopucci requested a review from a team as a code owner March 22, 2019 21:38
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@adriansr adriansr added bug in progress Pull request is currently in progress. Metricbeat Metricbeat labels Mar 23, 2019
Copy link
Contributor

@adriansr adriansr left a comment

Choose a reason for hiding this comment

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

Please update all the metricsets under postgresql module.

@@ -57,12 +57,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
// of an error set the Error field of mb.Event or simply call report.Error().
func (m *MetricSet) Fetch(reporter mb.ReporterV2) {
db, err := sql.Open("postgres", m.HostData().URI)
defer db.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when db is nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a condition to check for it

@leopucci
Copy link
Contributor Author

Library related problem, closing pull request for now.

@leopucci leopucci closed this Mar 24, 2019
@leopucci leopucci deleted the patch-1 branch March 24, 2019 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug in progress Pull request is currently in progress. Metricbeat Metricbeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants