SteNicholas commented on code in PR #3855:
URL: https://github.com/apache/celeborn/pull/3855#discussion_r4134408880
##########
charts/celeborn/templates/worker/_helpers.tpl:
##########
@@ -76,8 +84,76 @@ Create the name of the worker priority class to use
Create the name of the worker statefulset to use
*/}}
{{- define "celeborn.worker.statefulSet.name" -}}
+{{- if .zone -}}
+{{ include "celeborn.fullname" . }}-worker-{{ .zone.name }}
+{{- else -}}
{{ include "celeborn.fullname" . }}-worker
{{- end }}
+{{- end }}
+
+{{/*
+Number of replicas for a worker statefulset. Without zone-aware replication
this is
+`worker.replicas` as-is; with it, `worker.replicas` is the total across all
zones and each
+zone gets `ceil(replicas / zones)` unless the zone overrides it.
+*/}}
+{{- define "celeborn.worker.replicas" -}}
+{{- if .zone -}}
+{{- /* Key presence, not truthiness: an explicit 0 parks a zone without
removing it. */ -}}
+{{- if and (hasKey .zone "replicas") (not (kindIs "invalid" .zone.replicas))
-}}
+{{ .zone.replicas }}
+{{- else -}}
+{{ divf .Values.worker.replicas (len
.Values.worker.zoneAwareReplication.zones) | ceil | int }}
+{{- end }}
+{{- else -}}
+{{ .Values.worker.replicas }}
+{{- end }}
+{{- end }}
+
+{{/*
+Label selector used by the built-in autoscaling triggers. Scopes the query to
this
+statefulset's workers: by the `zone` label the worker publishes when it has
one, and by pod
+name when the zone metric label is turned off.
+*/}}
+{{- define "celeborn.worker.autoscaling.selector" -}}
+{{- $selector := printf "role=\"Worker\",namespace=\"%s\"" .Release.Namespace
-}}
Review Comment:
Scope built-in trigger queries to the target release. The selector does not
identify the Helm release when zones are disabled or when metricsLabel is
enabled (the default). I rendered releases alpha and beta in namespace shared:
their ScaledObjects target different StatefulSets but both disk and memory
queries are identical. The same happens with zones enabled when both releases
use the same zone name. Consequently, a busy worker in alpha makes beta scale
out too, and prevents beta from scaling back down independently. Please always
constrain the selector to the target StatefulSet, e.g. with the pod-name
matcher already used in the fallback branch, or publish and select a
release-specific metric label. A render test comparing two releases in the same
namespace would catch this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
