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

[bitnami/kafka] Update default value of heapOpts to fit Kafka pod RAM limit while utilize its increase (#29670) #29782

Merged
merged 4 commits into from
Oct 7, 2024

Conversation

plblueraven
Copy link
Contributor

Description of the change

OK, here we go:

  1. You are using bitnami/minideb as base image. Unfortunately, I couldn't find any minimum requirements for this image in the README.md, therefore I assume minimal requirements of Debian itself which is 256 MB.
  2. Estimating the minimum RAM for Kafka for a development environment isn't an easy task. I found a lot of opinion based data. The only ones I consider valuable are: a) Kafka server startup script with 1G is set as Xmx; b) competitive, unofficial, helm chart where we have cp-kafka.heapOptions with Xmx set to 512M.
  3. You are using Java 17, on which (in the case of Linux distributions) -XX:+UseContainerSupport is enabled by default. In this case we could benefit from setting -XX:InitialRAMPercentage and -XX:MaxRAMPercentage

As in commit I suggest removing -Xmx1024m -Xms1024m and replacing it with -XX:InitialRAMPercentage=75 -XX:MaxRAMPercentage=75. This will ensure 256 MB for the OS (768 MB*(1-0.75)=256 MB) and sort of solve the problem of scaling RAM available for Kafka with increasing Containers.kafka.Limits.memory value.

Benefits

  • Bothering Kubernetes' OOM Killer is much less likely.
  • RAM avaialble for Kafka will automatically scale with Kafka pod RAM limit

Possible drawbacks

Kafka could be inefficient at default 768 MB*0.75=512 MB RAM.

Applicable issues

Additional information

This needs testing, which I cannot do with a satisfactory variety of traffic.

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

Fit `heapOpts` to Kafka pod memory limit while utilize its increase

Signed-off-by: Blue Raven <[email protected]>
@github-actions github-actions bot added kafka triage Triage is needed labels Oct 5, 2024
@javsalgar javsalgar added verify Execute verification workflow for these changes in-progress labels Oct 7, 2024
@github-actions github-actions bot removed the triage Triage is needed label Oct 7, 2024
@github-actions github-actions bot removed the request for review from javsalgar October 7, 2024 07:54
Signed-off-by: Bitnami Containers <[email protected]>
Copy link
Member

@dgomezleon dgomezleon 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 for contributing!

@dgomezleon dgomezleon merged commit 51a574e into bitnami:main Oct 7, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kafka solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bitnami/kafka] Wrong default value of Xmx in context of default value of Containers.kafka.Limits.memory
4 participants