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

fix: Prevented memory leak generated by not correctly cleaned HTTP resources #5293

Merged
merged 4 commits into from
Dec 18, 2023

Conversation

JorTurFer
Copy link
Member

@JorTurFer JorTurFer commented Dec 15, 2023

After profiling KEDA operator looking for this memory leak, I found a weird behavior.
When I deploy the OP given SO, during the first minutes, prometheus trigger returns 404 and the memory grows, but at some point, the memory usage becomes stable and then prometheus trigger start to produce timeouts.

This impacts in 2 way, significativelly increasing the allocation of HTTP resources
image

And also increasing the size of the in use memory due to stored rootCAs of the clients (we are cloning the current cached certs but we aren't modifying the cloned item, so doesn't make sense, at least atm)

image

In order to solve these issues, this PR does:

  • enforcing the Idle Connection closing during (scaler).Close(context.Context) to close them deterministically and not delegating this action to the garbage collector. This helps to reduce the allocated resources for stablishing connections
  • returning the rootCAs item directly instead of cloning it, as we don't need to modify it (at least for the moment) so doesn't make sense creating a copy every time when it's requested. This helps to reduce in use memory for storing the copies (one per HTTP Client can generate several instances until de GC collects them if the scaler fails all the time and KEDA regenerates the scaler cache all the time). If we need to modify it in the future, we should think about how to improve this

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #5248

…onnections

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
@JorTurFer JorTurFer requested a review from a team as a code owner December 15, 2023 20:28
Copy link

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

Learn more about:

Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
@JorTurFer
Copy link
Member Author

JorTurFer commented Dec 15, 2023

/run-e2e
Update: You can check the progress here

pkg/scalers/kafka_scaler.go Outdated Show resolved Hide resolved
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
@JorTurFer JorTurFer changed the title fix: Prevented memory leak generated by not correctly cleaning http connections fix: Prevented memory leak generated by not correctly cleaned HTTP resources Dec 15, 2023
@JorTurFer
Copy link
Member Author

JorTurFer commented Dec 15, 2023

/run-e2e
Update: You can check the progress here

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM, great job!

@zroubalik zroubalik merged commit d3751e9 into kedacore:main Dec 18, 2023
19 checks passed
@JorTurFer JorTurFer deleted the fix-leak branch December 18, 2023 14:38
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
…sources (kedacore#5293)

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: anton.lysina <alysina@gmail.com>
jkyros pushed a commit to jkyros/keda that referenced this pull request Mar 27, 2024
… cleaned HTTP resources (kedacore#5293)

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
jkyros pushed a commit to jkyros/keda that referenced this pull request Mar 27, 2024
… cleaned HTTP resources (kedacore#5293)

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
jkyros pushed a commit to jkyros/keda that referenced this pull request Mar 27, 2024
… cleaned HTTP resources (kedacore#5293)

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>

This wasn't a completely clean pick, I had to adjust the swift scaler
client variable to be a pointer so it was nillable, but everything else
was okay.
openshift-merge-bot bot added a commit to openshift/kedacore-keda that referenced this pull request Mar 27, 2024
OCPBUGS-30145: fix: Prevented memory leak generated by not correctly cleaned HTTP resources (kedacore#5293)
@rameshbabu009
Copy link

"scaler": "prometheusScaler", "error": "prometheus query api returned error. status: 403 response: {"message":"Credential should be scoped to a valid region. "}"} any suggestions on this error

@JorTurFer
Copy link
Member Author

"scaler": "prometheusScaler", "error": "prometheus query api returned error. status: 403 response: {"message":"Credential should be scoped to a valid region. "}"} any suggestions on this error

This issue was closed 4 months ago and it's totally unrelated with your topic. I'd suggest opening a new issue for your case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keda-operator memory leak when prometheus scaler having errors
3 participants