Skip to content

feat(env): Add automatic memory limit handling #650

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dongjiang1989
Copy link
Contributor

add auto GOMEMLIMIT setting

@dongjiang1989
Copy link
Contributor Author

cc @EItanya PTAL

Copy link
Contributor

@EItanya EItanya left a comment

Choose a reason for hiding this comment

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

So I've been thinking about this PR and your previous one a little more, and I think they may be overkill tbh. I've used the following approach before which has been super simple and worked well.

         - name: GOMEMLIMIT
            valueFrom:
              resourceFieldRef:
                resource: limits.memory
                divisor: "1"
          - name: GOMAXPROCS
            valueFrom:
              resourceFieldRef:
                resource: limits.cpu
                divisor: "1"

Here it is in Istio: https://github.com/istio/istio/blob/b6df261b7c9f66c10b0d17b02f9c9b9ebb546033/manifests/charts/istio-control/istio-discovery/templates/deployment.yaml#L209-L218

@dongjiang1989
Copy link
Contributor Author

dongjiang1989 commented Jul 28, 2025

thanks @EItanya

This method is also effective. However, there is a drawback: under VPA adjustment, the Go runtime configuration cannot be dynamically adjusted. Just start it once for the first time.

@dongjiang1989
Copy link
Contributor Author

thanks @EItanya

This method is also effective. However, there is a drawback: under VPA adjustment, the Go runtime configuration cannot be dynamically adjusted. Just start it once for the first time.

@dongjiang1989 dongjiang1989 force-pushed the add-automemlimit branch 2 times, most recently from 3539316 to 8d47371 Compare August 6, 2025 09:59
Signed-off-by: dongjiang <[email protected]>
@howardjohn
Copy link

thanks @EItanya

This method is also effective. However, there is a drawback: under VPA adjustment, the Go runtime configuration cannot be dynamically adjusted. Just start it once for the first time.

I don't think thats true. a VPA change triggers a new pod, which will have a freshly computed value.

I agree with @EItanya on the approach

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.

3 participants