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.

Sign up for GitHub

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

Jump to bottom

Fix memory leak in SQL helper when database is not available #26607

Merged
merged 4 commits into from
Jul 1, 2021
Merged

Fix memory leak in SQL helper when database is not available #26607

merged 4 commits into from
Jul 1, 2021

Conversation

jsoriano
Copy link
Member

What does this PR do?

Explicitly close SQL connections when connectivity checks fail during their creation.

Why is it important?

Avoid leaking resources when a connection is created but it cannot be used.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

@jsoriano jsoriano added module review Metricbeat Metricbeat backport-v7.13.0 Automated backport with mergify backport-v7.14.0 Automated backport with mergify backport-v7.15.0 Automated backport with mergify labels Jun 30, 2021
@jsoriano jsoriano requested a review from sayden June 30, 2021 09:43
@jsoriano jsoriano self-assigned this Jun 30, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 30, 2021
@jsoriano jsoriano added the Team:Integrations Label for the Integrations team label Jun 30, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 30, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 30, 2021 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #26607 updated

  • Start Time: 2021-07-01T08:51:29.009+0000

  • Duration: 108 min 44 sec

  • Commit: 96142bd

Test stats 🧪

Test Results
Failed 0
Passed 8904
Skipped 2588
Total 11492

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 8904
Skipped 2588
Total 11492

@jsoriano
Copy link
Member Author

/test

1 similar comment
@jsoriano
Copy link
Member Author

/test

@mergify
Copy link
Contributor

mergify bot commented Jun 30, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b sql-disconnection-leak upstream/sql-disconnection-leak
git merge upstream/master
git push upstream sql-disconnection-leak

sayden
sayden approved these changes Jul 1, 2021
View reviewed changes
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@jsoriano jsoriano merged commit e4d4f76 into elastic:master Jul 1, 2021
@jsoriano jsoriano deleted the sql-disconnection-leak branch July 1, 2021 11:06
mergify bot pushed a commit that referenced this pull request Jul 1, 2021
@jsoriano
Fix memory leak in SQL helper when database is not available (#26607)
01935a6 
Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)
@mergify mergify bot mentioned this pull request Jul 1, 2021
Merged
mergify bot pushed a commit that referenced this pull request Jul 1, 2021
@jsoriano
Fix memory leak in SQL helper when database is not available (#26607)
3edcddb 
Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)
@mergify mergify bot mentioned this pull request Jul 1, 2021
Merged
mergify bot pushed a commit that referenced this pull request Jul 1, 2021
@jsoriano
Fix memory leak in SQL helper when database is not available (#26607)
9b5ab5d 
Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)
@mergify mergify bot mentioned this pull request Jul 1, 2021
Merged
jsoriano added a commit that referenced this pull request Jul 1, 2021
@mergify @jsoriano
Fix memory leak in SQL helper when database is not available (#26607) (…
9a91ff8 
…#26654)

Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
jsoriano added a commit that referenced this pull request Jul 1, 2021
@mergify @jsoriano
Fix memory leak in SQL helper when database is not available (#26607) (…
1e881ba 
…#26656)

Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
v1v added a commit to v1v/beats that referenced this pull request Jul 5, 2021
@v1v
Merge remote-tracking branch 'upstream/master' into feature/retry-if-…
a26267c 
…stage-failed-within-same-build

* upstream/master: (36 commits)
  Revert "[CI] fight the flakiness with some retry option in the CI only for the Pull Requests (elastic#26617)" (elastic#26704)
  Packaging: linux/armv7 is not supported (elastic#26706)
  Cyberarkpas: Link to official docs on how to setup TLS (elastic#26614)
  Make network_direction, registered_domain and convert processors compatible with ES older than 7.13.0 (elastic#26676)
  Disable armv7 packaging (elastic#26679)
  [Heartbeat] use --params flag for synthetics (elastic#26674)
  Update dependent package to avoid downloading a suspicious file (elastic#26406)
  [mergify] set title and allow bp in any direction (elastic#26648)
  Fix memory leak in SQL helper when database is not available (elastic#26607)
  [CI] fight the flakiness with some retry option in the CI only for the Pull Requests (elastic#26617)
  [mergify] automate PRs that change the backport rules (elastic#26641)
  [Metricbeat] Add Airflow module in xpack (elastic#26220)
  chore: add-backport-next (elastic#26620)
  [metricbeat] Add state_job metricset (elastic#26479)
  CI: jenkins labels are less time consuming now (elastic#26613)
  [MetricBeat] [AWS] Fix aws metric tags with resourcegroupstaggingapi paginator (elastic#26385) (elastic#26443)
  Move openmetrics module to oss (elastic#26561)
  Skip flaky test TestFilestreamMetadataUpdatedOnRename (elastic#26609)
  [filebeat][fortinet] Use default add_locale for fortinet.firewall (elastic#26524)
  Enroll proxy settings (elastic#26514)
  ...
jsoriano added a commit that referenced this pull request Jul 28, 2021
@mergify @jsoriano
[7.13](backport #26607) Fix memory leak in SQL helper when database i…
9e0a764 
…s not available (#26655)

Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)

* Fix changelog

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@mergify @jsoriano
[7.13](backport elastic#26607) Fix memory leak in SQL helper when dat…
e26df63 
…abase is not available (elastic#26655)

Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit 409c244)

* Fix changelog

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sayden sayden sayden approved these changes

Assignees

@jsoriano jsoriano

Labels
backport-v7.13.0 Automated backport with mergify backport-v7.14.0 Automated backport with mergify backport-v7.15.0 Automated backport with mergify Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possible goroutine leak in metricbeat sql module when database is not available
3 participants
@jsoriano @elasticmachine @sayden