Repository navigation
fix(observability): alert on the GC ceiling, not only on replica divergence - #3319
Merged
Merged
Conversation
…rgence
PortalReplicaWorkingSetDiverged requires max > 3x min across a namespace's
replicas. Measured 2026-09-04 on ns memex: two 28h-old replicas at 9936Mi and
9409Mi -- ratio 1.06 against a threshold of 3, with BOTH at ~80% of the GC's
12 GiB hard limit and both on the curve that produced the 09-03 outage. Replicas
converge on the same ceiling with age, so the ratio is HIGHEST during the warning
and LOWEST during the incident: the detector is loudest early and silent at the
end. Its comment argued the opposite ("a namespace whose replicas all sit at
12 GB is a sizing question, not this incident"); that premise is now falsified.
Not fixed by lowering the 3x -- a rollout legitimately runs a cold pod beside
warm ones, and the rule earns its keep on the asymmetric case (7.22x on 09-01).
The terminal case needs an absolute reading, which a ratio cannot express:
PortalHeapEnteringGcDefenceBand, per namespace, no sibling comparison, at 75% of
the container memory limit -- where .NET's default container GC heap hard limit
sits. Verified against the live Prometheus before commit: the metric exists
(9 series) and the expression discriminates, memex-cloud 0.851 vs memex 0.605.
Also records why no OOMKill ever fires, which is what makes item 4 necessary:
nothing sets DOTNET_GCHeapHardLimit[Percent] anywhere (inline env, ConfigMap,
envFrom Secrets -- audited by key name on both namespaces), so the default 75%
of a 16Gi limit gives the managed heap a 12 GiB ceiling that the runtime defends
with back-to-back blocking gen2 rather than letting the kubelet restart the pod.
Measured: 0 OOMKills across 7 replicas over 28h; GC pause share 0.66 sustained
for 2h15 on 09-03 while /alive answered Healthy.
And corrects a self-contradiction in the probe comments: the readiness comment
called a progress-aware check an open design item while the liveness comment ten
lines below asserted /alive already answers 503 above a 60% pause share. Both
were committed at once. What /alive measures is image-dependent -- core registers
only the trivial "self" check on tag `live`; a progress-aware handler comes from
the host in MeshWeaver.Plugins. The load-bearing consequence is new: readiness
and liveness share the /alive path, and readiness trips at 10s x 3 = 30s against
liveness's 15s x 6 = 90s. A progress-aware /alive therefore removes a degraded
replica from the Service three times sooner than it restarts it, on replicas that
converge on the same ceiling -- the 2026-07-21 death-spiral rebuilt out of the
item-4 fix. Item 4 needs its own path, or readiness must move off this one.
Refs #2194
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are additive and low-risk (docs/comments + an additional alert rule), with only a minor wording nit identified in a comment.
Pull request overview
This PR updates the observability story for GC-bound portal replicas by adding an absolute “GC ceiling band” alert (in addition to the existing replica-divergence alert), and documenting the underlying mechanism so operators can interpret and triage the signal correctly.
Changes:
- Added a new Prometheus alert (
PortalHeapEnteringGcDefenceBand) to detect when replicas approach the GC heap hard-limit band (using working set as a proxy). - Updated the Helm chart probe commentary to clearly state that
/alivesemantics are image-dependent and that readiness/liveness sharing/aliveconstrains “progress-aware” health check work. - Added a new architecture documentation page capturing the measured behavior and its operational implications.
File summaries
| File | Description |
|---|---|
| src/MeshWeaver.Documentation/Data/Architecture/WhyAGcBoundPodStaysInRotation.md | New architecture doc explaining why GC-bound replicas can remain Ready and how to reason about detection signals. |
| deploy/helm/templates/memex-portal/deployment.yaml | Clarifies probe-path semantics and constraints around making /alive progress-aware when readiness and liveness share the endpoint. |
| deploy/aks/scripts/values.observability.yaml | Adds an absolute working-set/limit alert alongside the existing divergence alert, with updated rationale and triage guidance. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+244
to
+245
| # Prometheus cannot see it. Until that pipeline reaches Prometheus, working set over the | ||
| # container limit is the honest best available proxy. |
…emory limit is absent cAdvisor reports the denominator as 0 when a container carries no memory limit, so the ratio becomes +Inf and PortalHeapEnteringGcDefenceBand fires. That is the correct direction and worth saying explicitly: with no container limit .NET applies no GC heap hard limit either, so the heap grows until it takes the node down. The obvious 'fix' -- and container_spec_memory_limit_bytes > 0 -- would silence the rule on exactly the misconfiguration it should shout about. Refs #2194 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…09-04 measurement The in-memory retention section reads as though convergence closes the gap. It does for the mechanism it describes -- but the spiral recurred on 09-03 with Modules:AutoRecycleOnStaleBuild live on both namespaces, and counted over one replica's entire 28h life there were ZERO stale-build offers, convergences, self-heal recompiles or bakes, while the managed heap still grew 6856 -> 10065 MB across 267 gen2 collections. Convergence had nothing to converge. Two mechanisms the section does not cover were doing it instead: no hub or grain deactivates at all, so the lifetime ALC lease an instance hub takes is never released; and replicas boot having adopted as few as 8 of 377 dynamic NodeTypes, so the rest each pay a Roslyn compile on first access and pin it -- the same accumulation, arriving as a drip and driven by bundle coverage rather than supersession. Also qualifies 'no OOMKill (the pod limit was never reached)': the GC's own hard limit is 75% of the container limit, so the runtime defends memory with a sustained GC pause share instead of letting the kubelet restart the pod. Links the new derivation page so this correction is discoverable from where the next reader actually starts. Refs #2194 #3321 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mespaces
The evidence for the new rule was a 1x2 reading. It is now the full 2x2 from one
live scrape, which is what shows the two rules are complementary rather than
redundant:
divergence (>3) headroom (>0.75) actually degraded?
memex-cloud 8.45 fires 0.851 fires yes
memex 1.06 blind 0.605 quiet not yet
Four results, not two. Both rules fire on the degraded namespace and both stay
quiet on the healthy one, so neither fires on everything. The complementarity is
in the first column: memex's ratio is 1.06 and will FALL rather than rise,
because its replicas converge -- so when they reach the defence band, divergence
still reads ~1.0 and only the new rule fires. That is the uncovered case.
Refs #2194
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Author
|
Your shard-5 red is not yours — Cured in #3324: Second sighting today (the first was run 33863349195, a different method, same cause). Once #3324 lands, refresh this PR's base — a re-run replays the same commit and will not pick the fix up. |
Contributor
Contributor
…elow-container-limit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
#2194's fix table has kept items 4 (progress-aware
/alive) and 5 (working-set divergence alert) open since 2026-08-25, and the GC spiral recurred on 09-03 with fixes 1 and 2 both live in production. The maintainer's last comment posed the discriminator: is convergence enabled-but-not-converging, or does a second cause share the curve?Measured today, read-only, against
memexandmemex-cloud. It is neither of those two — it is a third answer, and it changes what items 4 and 5 have to be.The discriminator, settled
Counted over one replica's entire 28-hour life (186 137 lines), with
info:-level lines confirmed to be shipping (2 791 of them) so a zero is a real zero and not a log-level artefact:No build was ever superseded.
Modules:AutoRecycleOnStaleBuildis armed on both namespaces and has never fired, because nothing has ever been stale. Fixes 1+2 could not have prevented the recurrence — they address a state that does not arise. Anddeactivating: reason=is 0 across five sampled replicas and 514 687 lines: not one hub or grain deactivated since boot, so no ALC lease is ever released.The compile load is a coverage failure, not a publication wave — the two oldest replicas booted having adopted 8 and 9 of 377 dynamic NodeTypes, with the other 368 due to "pay a Roslyn compile on FIRST ACCESS", lazily, spread over the pod's whole life. That is fix 3's lane (
Modules:RequirePrebuilt), not fix 1's.Why nothing ever restarts the pod
Nothing sets
DOTNET_GCHeapHardLimit[Percent]— not the inlineenv:, notmemex-portal-config, not any envFrom Secret (audited by key name on both namespaces, values never read). So .NET's container default applies: the GC's heap hard limit is 75 % of the container limit = 12 GiB atlimits.memory: 16Gi, and it covers the managed heap only.The runtime defends that ceiling with back-to-back blocking gen2 instead of letting the kubelet restart the container. Measured: 0 OOMKills across 7 replicas over 28 h (the only two restarts were
exitCode 255/Unknownat one shared timestamp — a node event), and GC pause share 0.66 sustained for 2 h 15 on 09-03 while/aliveansweredHealthy. The GC converts a memory shortage — which Kubernetes fixes in one second — into CPU starvation, about which Kubernetes has no opinion.The heap growth itself is retention, not a working set: 6 856 → 10 065 MiB in 14.9 h (~215 MB/h) across 267 gen2 collections, with the floor after each collection never returning to any earlier level. And it tracks uptime rather than load — the near-idle replica (512 log lines/30 min) holds 11 443 Mi while the busiest one (2 476 lines) holds 4 134 Mi.
Item 5 — a detector that goes blind exactly when it matters
PortalReplicaWorkingSetDivergedrequiresmax > 3 × min. Onmemexright now:Both 28 h old, both at ~80 % of the 12 GiB GC ceiling, both on the curve that produced the 09-03 outage — and the alert sits at a third of its threshold and keeps falling as they converge. Its comment argued "a namespace whose replicas all sit at 12 GB is a sizing question, not this incident"; that is the premise this measurement falsifies.
This is not fixed by lowering the 3× — a rollout legitimately runs a cold pod beside warm ones, and the rule earns its keep on the asymmetric case (it read 7.22× correctly on 09-01). The terminal case needs an absolute reading, which a ratio cannot express at all. So the divergence rule is kept as the early signal and
PortalHeapEnteringGcDefenceBandis added for the other end: per namespace, no sibling comparison, thresholded at 75 % of the container limit — the point at which working set has reached the numeric value the GC is willing to let the managed heap alone occupy.Verified against the live Prometheus before committing, because an alert that cannot fire is worse than no alert.
count(container_spec_memory_limit_bytes{container="memex-portal"})→ 9 series, and both rules on one scrape:max / min(fires > 3)working set / limit(fires > 0.75)memex-cloudPlatform stalledin the logmemexFour results, not two. Both rules fire on the degraded namespace and both stay quiet on the healthy one — so neither fires on everything, and the new rule is not a restatement of the old. The complementarity is the first column:
memex's ratio is 1.06 and will fall, not rise, because its replicas are converging. When they reach the defence band, divergence still reads ~1.0 and only the new rule fires. That is the uncovered case.🚨 Neither rule is armed until
values.observability.yamlis applied. On 09-01 the live Prometheus held zero rule groups with thelokirelease still on its 31 May revision, so the divergence alert had been merged for a week and was watching nothing.git grepis evidence of authorship and never of arming; onlyGET /api/v1/rulesnaming the alert is.Item 4 — the chart contradicted itself, and the real constraint is new
deployment.yamlcarried both of these, ten lines apart:The 09-04 status comment quoted the stale half as current. What
/alivemeasures is image-dependent and this repo cannot assert it: core registers only the trivial"self"check on taglive; a progress-aware handler comes from the host (Memex.Portal.Distributed, in MeshWeaver.Plugins).The consequence that is not recorded anywhere, and which bounds item 4:
Readiness and liveness share the
/alivepath, and readiness trips first —10s × 3 = 30sagainst liveness's15s × 6 = 90s. The moment/alivebecomes progress-aware, a GC-bound replica is pulled from the Service three times sooner than it is restarted, and its traffic lands on siblings converging on the same ceiling (ratio 1.06, above). That is the 2026-07-21 readiness death-spiral, rebuilt out of the item-4 fix.⇒ A progress-aware liveness signal needs its own path, or readiness must move off this one. Both comments now say so instead of contradicting each other.
Also recorded
AddRuntimeInstrumentation()is on, so the portal already emitsdotnet_gc_*— heap size, pause time, per-generation counts. The collector's metrics pipeline exports onlyfile/metrics, a rotating JSON file on the node; logs and traces have a second path to Loki, metrics have none. That is why the only memory signal any alert can use is a kubelet-side working-set number that cannot see the GC ceiling at all, and why the new threshold is a proxy rather than a direct read of the heap.Verification
deploy/aks/scripts/check-chart-invariants.sh→All 3 values combinations render a self-consistent deployment.deploy/aks/scripts/check-values-are-read.sh→All 114 config key(s) across 3 values file(s) are read by a templatedotnet build src/MeshWeaver.Documentation/MeshWeaver.Documentation.csproj -c Release -warnaserror→0 Warning(s) 0 Error(s)DocumentationLinkIntegrityTest→Passed! Failed: 0, Passed: 1,.trxwritten.[Deliberately broken](../ThisPageDoesNotExistXYZ)turned it red withDoc/Architecture/WhyAGcBoundPodStaysInRotation: (../ThisPageDoesNotExistXYZ) — resolves to /Doc/Architecture/ThisPageDoesNotExistXYZ, which does not existand removing it turned it green again.
portal-degraded.No cluster object was mutated. All cluster reads were
kubectl get/top/logs, the Prometheus query API, and one/aliveGET through the API-server proxy. No secret value was read, printed or stored — the GC-override audit projected Secret data to key names only.Follow-up commits on this branch
+Infand this fires. That is deliberate — with no container limit .NET applies no GC heap hard limit either. The obvious "fix" (and container_spec_memory_limit_bytes > 0) would silence the rule on exactly the misconfiguration it should shout about.NodeTypeCompilation.md's in-memory retention section read as though convergence closes the gap; it now carries the 09-04 correction and links the new page, so the correction is discoverable from where the next reader actually starts.Refs #2194 · successor RCA #3321
🤖 Generated with Claude Code