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

Adjust fluent-op request #118

Merged
merged 6 commits into from
Jan 17, 2025
Merged

Adjust fluent-op request #118

merged 6 commits into from
Jan 17, 2025

Conversation

rbapst-tamedia
Copy link
Contributor

Because of alert.
Operator uses 30MiB with max=37 on last 2 days

Description

Adapt request on what is needed

Motivation and Context

We got alert that:

[prod-discovery,production-discovery] Average RAM usage of fluent-operator is significantly higher than its request

Breaking Changes

  • None

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

Because of alert.
Operator uses 30MiB with max=37 on last 2 days
Copy link
Contributor

@swibrow swibrow left a comment

Choose a reason for hiding this comment

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

I seen its in draft but I'd recommend these changes.

files/helm/fluent-operator/common.yaml Outdated Show resolved Hide resolved
files/helm/fluent-operator/common.yaml Outdated Show resolved Hide resolved
files/helm/fluent-operator/common.yaml Outdated Show resolved Hide resolved
@swibrow
Copy link
Contributor

swibrow commented Dec 27, 2024

Sadly they set resource values:
https://github.com/fluent/fluent-operator/blob/master/charts/fluent-operator/values.yaml#L44-L50

This is normally not a good idea to add in the defaults...

@rbapst-tamedia
Copy link
Contributor Author

Sadly they set resource values: https://github.com/fluent/fluent-operator/blob/master/charts/fluent-operator/values.yaml#L44-L50

This is normally not a good idea to add in the defaults...

Yes, I just saw it :-(

Looks to work by setting limits.cpu: and limits.memory:

@rbapst-tamedia rbapst-tamedia marked this pull request as ready for review December 27, 2024 14:53
Copy link
Contributor

@swibrow swibrow left a comment

Choose a reason for hiding this comment

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

Can you set 100m 200Mi to be safe?

files/helm/fluent-operator/common.yaml Outdated Show resolved Hide resolved
@swibrow
Copy link
Contributor

swibrow commented Jan 17, 2025

Will merge since its burning credits

@swibrow swibrow merged commit 901019a into main Jan 17, 2025
24 checks passed
@swibrow swibrow deleted the remove-alert branch January 17, 2025 05:59
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.

2 participants