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

feat: include memory limit in memory settings #1825

Merged
merged 3 commits into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/config/crd/bases/odigos.io_collectorsgroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ spec:
this is when go runtime will start garbage collection.
it is recommended to be set to 80% of the hard limit of the memory limiter.
type: integer
memoryLimitMiB:
description: |-
This option sets the limit on the memory usage of the collector.
since the memory limiter mechanism is heuristic, and operates on fixed intervals,
while it cannot fully prevent OOMs, it can help in reducing the chances of OOMs in edge cases.
the settings should prevent the collector from exceeding the memory request,
so one can set this to the same value as the memory request or higher to allow for some buffer for bursts.
type: integer
memoryLimiterLimitMiB:
description: |-
this parameter sets the "limit_mib" parameter in the memory limiter configuration for the collector.
Expand All @@ -86,6 +94,7 @@ spec:
type: integer
required:
- gomemlimitMiB
- memoryLimitMiB
- memoryLimiterLimitMiB
- memoryLimiterSpikeLimitMiB
- memoryRequestMiB
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions api/odigos/v1alpha1/collectorsgroup_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ type CollectorsGroupMemorySettings struct {
// it will be embedded in the as a resource request of the form "memory: <value>Mi"
MemoryRequestMiB int `json:"memoryRequestMiB"`

// This option sets the limit on the memory usage of the collector.
// since the memory limiter mechanism is heuristic, and operates on fixed intervals,
// while it cannot fully prevent OOMs, it can help in reducing the chances of OOMs in edge cases.
// the settings should prevent the collector from exceeding the memory request,
// so one can set this to the same value as the memory request or higher to allow for some buffer for bursts.
Comment on lines +43 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we write the k8s value that will map into? and state that it will impact when the collector is OOMed?

MemoryLimitMiB int `json:"memoryLimitMiB"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it is better to configure this as relative to the MemoryRequest field? (i/e with precentages or fractions relative to the request amount.)


// this parameter sets the "limit_mib" parameter in the memory limiter configuration for the collector.
// it is the hard limit after which a force garbage collection will be performed.
// this value will end up comparing against the go runtime reported heap Alloc value.
Expand Down
4 changes: 4 additions & 0 deletions autoscaler/controllers/gateway/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func getDesiredDeployment(dests *odigosv1.DestinationList, configDataHash string
gateway *odigosv1.CollectorsGroup, scheme *runtime.Scheme, imagePullSecrets []string, odigosVersion string) (*appsv1.Deployment, error) {

requestMemoryQuantity := resource.MustParse(fmt.Sprintf("%dMi", gateway.Spec.MemorySettings.MemoryRequestMiB))
limitMemoryQuantity := resource.MustParse(fmt.Sprintf("%dMi", gateway.Spec.MemorySettings.MemoryLimitMiB))

desiredDeployment := &appsv1.Deployment{
ObjectMeta: v1.ObjectMeta{
Expand Down Expand Up @@ -190,6 +191,9 @@ func getDesiredDeployment(dests *odigosv1.DestinationList, configDataHash string
Requests: corev1.ResourceList{
corev1.ResourceMemory: requestMemoryQuantity,
},
Limits: corev1.ResourceList{
corev1.ResourceMemory: limitMemoryQuantity,
},
},
},
},
Expand Down
9 changes: 9 additions & 0 deletions scheduler/controllers/clustercollectorsgroup/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ const (
// the percentage out of the memory limiter hard limit, at which go runtime will start garbage collection.
// it is used to calculate the GOMEMLIMIT environment variable value.
defaultGoMemLimitPercentage = 80.0

// the memory settings should prevent the collector from exceeding the memory request.
// however, the mechanism is heuristic and does not guarantee to prevent OOMs.
// allowing the memory limit to be slightly above the memory request can help in reducing the chances of OOMs in edge cases.
// instead of having the process killed, it can use extra memory available on the node without allocating it preemptively.
memoryLimitAboveRequestFactor = 1.25
)

// process the memory settings from odigos config and return the memory settings for the collectors group.
Expand All @@ -29,6 +35,8 @@ func getMemorySettings(odigosConfig *common.OdigosConfiguration) *odigosv1.Colle
memoryRequestMiB = odigosConfig.CollectorGateway.RequestMemoryMiB
}

memoryLimitMiB := int(float64(memoryRequestMiB) * memoryLimitAboveRequestFactor)

// the memory limiter hard limit is set as 50 MiB less than the memory request
memoryLimiterLimitMiB := memoryRequestMiB - defaultMemoryLimiterLimitDiffMib
if odigosConfig.CollectorGateway != nil && odigosConfig.CollectorGateway.MemoryLimiterLimitMiB > 0 {
Expand All @@ -47,6 +55,7 @@ func getMemorySettings(odigosConfig *common.OdigosConfiguration) *odigosv1.Colle

return &odigosv1.CollectorsGroupMemorySettings{
MemoryRequestMiB: memoryRequestMiB,
MemoryLimitMiB: memoryLimitMiB,
MemoryLimiterLimitMiB: memoryLimiterLimitMiB,
MemoryLimiterSpikeLimitMiB: memoryLimiterSpikeLimitMiB,
GomemlimitMiB: gomemlimitMiB,
Expand Down
Loading