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 non-closed http responses #5755

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Fix non-closed http responses #5755

merged 1 commit into from
Jun 13, 2022

Conversation

jiuker
Copy link
Contributor

@jiuker jiuker commented Jun 9, 2022

@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?

3 similar comments
@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?

@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?

@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?

@botelastic botelastic bot added the triage label Jun 9, 2022
@botelastic botelastic bot removed the triage label Jun 9, 2022
@thbkrkr
Copy link
Contributor

thbkrkr commented Jun 9, 2022

Jenkins test this please

@thbkrkr thbkrkr changed the title handler error Refactor request body closing Jun 9, 2022
thbkrkr
thbkrkr approved these changes Jun 9, 2022
View reviewed changes
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

LGTM!

barkbay
barkbay approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/controller/association/secret.go
@@ -130,11 +130,11 @@ func (r UnmanagedAssociationConnectionInfo) Request(path string, jsonPath string
if err != nil {
return "", err
}
defer resp.Body.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch, this one may lead to a [memory|fd|connection] leak. FWIW I wrote a quick and dirty integration test that demonstrates the leak, after a few iterations we have some too many open files errors.

@barkbay barkbay merged commit 1168b13 into elastic:main Jun 13, 2022
@thbkrkr thbkrkr added the v2.4.0 label Jun 14, 2022
barkbay pushed a commit to barkbay/cloud-on-k8s that referenced this pull request Jun 14, 2022
@jiuker @barkbay
Fix non closed http responses (elastic#5755)
5773fc4 
Co-authored-by: guozhi.li <guozhi.li@daocloud.io>
@barkbay barkbay mentioned this pull request Jun 14, 2022
Merged
barkbay added a commit that referenced this pull request Jun 14, 2022
@barkbay @jiuker
Fix non closed http responses (#5755) (#5770)
a830566 
Co-authored-by: jiuker <2818723467@qq.com>
Co-authored-by: guozhi.li <guozhi.li@daocloud.io>
@jiuker jiuker mentioned this pull request Jun 16, 2022
Closed
@barkbay barkbay added v2.3.0 >bug Something isn't working and removed v2.4.0 >refactoring labels Jun 17, 2022
@barkbay barkbay changed the title Refactor request body closing Fix non closed http responses Jun 17, 2022
@barkbay barkbay changed the title Fix non closed http responses Fix non-closed http responses Jun 17, 2022
fantapsody pushed a commit to fantapsody/cloud-on-k8s that referenced this pull request Feb 7, 2023
@jiuker @fantapsody
Fix non closed http responses (elastic#5755)
f7bf28c 
Co-authored-by: guozhi.li <guozhi.li@daocloud.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thbkrkr thbkrkr thbkrkr approved these changes

@barkbay barkbay barkbay approved these changes

Assignees
No one assigned
Labels
>bug Something isn't working v2.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jiuker @elasticmachine @thbkrkr @barkbay