Skip to content

Commit e3164a1

Browse files
myasnikovdaniilmyasnikovdaniil
committed
refactor(linstor): move the satellite sizing account out of the manifest
Same treatment multus just had, and for the same reason: a `#` comment in a template is copied verbatim into the rendered LinstorSatelliteConfiguration in every cluster and goes stale the moment a number changes. The block attached to the satellite's resources stanza was 22 lines, all of it written on this branch, and it is the one the reviewer's MINOR named alongside multus. The template now carries what a reader needs at the point of decision and nothing more: that the container is deliberately limit-free, that it is a JVM and that linstor-controller is exempted for the same reason, where the full account lives, and the measured peak. The peak line stays in the manifest deliberately -- it is one line, it is a measurement, and it is what someone changing the request needs in front of them rather than one file away. The plunger block gets the same treatment at 9 lines down to 5, since most of what it grew to say was mechanism that reads better beside the assertions. Everything moved out was already in tests/satellites_test.yaml after the previous commit, with one exception now added there: that the satellite runs the same JVM is evidenced by images/piraeus-server/Dockerfile installing default-jre-headless and starting it with `startSatellite`. That is the evidence which overturned the sizing premise under review, so it belongs somewhere a reader can find it rather than only in a commit message. Comments only. No rendered value changes, and satellites_test.yaml still fails on a re-added ceiling. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 3bde62d commit e3164a1

3 files changed

Lines changed: 15 additions & 33 deletions

File tree

‎packages/system/linstor/templates/satellites-cozy.yaml‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,9 @@ spec:
1414
containers:
1515
- name: linstor-satellite
1616
image: {{ .Values.piraeusServer.image.repository }}:{{ .Values.piraeusServer.image.tag }}
17-
# The satellite DaemonSet ran BestEffort on every storage node, which
18-
# made it a standing victim for the Talos userspace OOM handler
19-
# (v1.12+): that handler only ever kills cgroups with no memory.max, and
20-
# picks by QoS class rather than by what caused the pressure. A tenant
21-
# workload thrashing against its own limit would get the satellite
22-
# killed, taking DRBD connections down with it.
23-
#
24-
# Requests only, no limit: the same call linstor-controller makes in
25-
# values.yaml, and for the same reason, because this is the same JVM.
26-
# images/piraeus-server/Dockerfile installs default-jre-headless and
27-
# starts this container with `startSatellite`, the distribution the
28-
# controller runs with `startController`. A memory.max is what a
29-
# container-aware JVM reads to size its heap, so a limit here shrinks the
30-
# heap rather than only capping RSS. This moves the ceiling rather than
31-
# removing it: with the LimitRange the operator maintains in every system
32-
# namespace (#3494, which must merge with or before this) admission
33-
# defaults this container to --system-namespace-memory-limit, 4Gi, so the
34-
# pod does get memory.max and the heap is derived from one operator-facing
35-
# knob instead of a per-chart guess. Without that LimitRange the container
36-
# really is limit-free and the pod stays in the victim set.
37-
# Full account, and what would settle the sizing: tests/satellites_test.yaml.
17+
# Deliberately no memory limit: this is the same JVM as linstor-controller,
18+
# which values.yaml exempts for the same reason. Full account, and what
19+
# would settle the sizing: packages/system/linstor/tests/satellites_test.yaml.
3820
# 14d peak 368Mi on a 3-node dev cluster; grows with resource count.
3921
resources:
4022
requests:

‎packages/system/linstor/templates/satellites-plunger.yaml‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,10 @@ spec:
1212
containers:
1313
- name: plunger
1414
image: {{ .Values.piraeusServer.image.repository }}:{{ .Values.piraeusServer.image.tag }}
15-
# Sidecars in the satellite pod. These limits bound a runaway sidecar at
16-
# 128Mi rather than at the far looser ceiling it would otherwise take;
17-
# what completes the set for the pod cgroup is not them. The satellite
18-
# container beside them carries no chart-level limit because it is a JVM,
19-
# so the pod's memory.max arrives from the system-namespace LimitRange
20-
# default (#3494) — and without that LimitRange the pod has none whatever
21-
# these sidecars declare, and stays in the Talos userspace OOM handler's
22-
# victim set. See satellites-cozy.yaml.
15+
# Sidecars in the satellite pod. These limits bound a runaway sidecar;
16+
# they are not what puts memory.max on the pod cgroup, because the
17+
# satellite container beside them is deliberately limit-free.
18+
# See satellites-cozy.yaml and tests/satellites_test.yaml.
2319
# 14d peaks on a 3-node dev cluster: plunger 18Mi, drbd-logger 5Mi.
2420
resources:
2521
requests:

‎packages/system/linstor/tests/satellites_test.yaml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@ suite: satellite pod resource limits
88
#
99
# The satellite is the exception and this suite pins it as one: it is the same JVM
1010
# as linstor-controller, which cluster_test.yaml pins as deliberately limit-free,
11-
# so it carries a request and no limit. A memory.max is what a container-aware JVM
12-
# reads to size its heap, so a limit shrinks the heap rather than only capping RSS,
13-
# and the 368Mi peak behind the request grows with DRBD resource count and was
14-
# measured on three nodes.
11+
# so it carries a request and no limit. That it is the same JVM is not an inference
12+
# — images/piraeus-server/Dockerfile installs default-jre-headless and starts this
13+
# container with `startSatellite`, the same distribution the controller runs with
14+
# `startController` — and it matters because the sizing this replaced was chosen on
15+
# the premise that the satellite is a Go binary with a bounded working set. A
16+
# memory.max is what a container-aware JVM reads to size its heap, so a limit
17+
# shrinks the heap rather than only capping RSS, and the 368Mi peak behind the
18+
# request grows with DRBD resource count and was measured on three nodes.
1519
#
1620
# The exemption moves the ceiling, it does not remove it, and the distinction is
1721
# what makes it safe. With the LimitRange the operator maintains in every system

0 commit comments

Comments
 (0)