feat(memory): remove the watermark self-recycle - #85
Merged
Conversation
The dest-connection age sweep (memory.dest_conn_max_age_seconds, #84) has held prod flat for ~19h — zero watermark events against a prior ~70-90min seppuku cadence. The leak the watermark papered over is bounded by the sweep, so the preemptive drain-and-exit goes away: the process no longer kills itself on an RSS watermark, and the only drain path left is the ordinary SIGTERM one (60s, unchanged). Removes: memory.self_recycle_* config, resolve_recycle_watermark, _should_self_recycle, _cgroup_memory_limit_gib, the viaduck_self_recycles_total counter, and the watermark test section. _read_rss_gib stays — the per-cycle viaduck_rss_bytes gauge still uses it.
README (config sample, failure-mode table, metrics table, Deployment section), runbook-offset-reset startup check, pyproject comment, and the log-consumer proposal's point-in-time references. The section now documents the dest-connection age sweep (the replacement mitigation). Duckling._maybe_recycle (the per-destination-process RSS-limit exit) is intentionally untouched: it is a different component with no pool to sweep — its pod IS the unit of recycling.
Adversarial review (QE + SWE) of the removal:
- config.load: retired memory.self_recycle_* keys now WARN-ignored
(delivery-section pattern), never refused — prod's chart still carries
them; silence would let an operator believe the deleted bound exists.
memory section is None-tolerant ('memory:' with all comments parses
to None).
- log-consumer-proposal.md: dated 2026-08-15 measurement row restored
verbatim + erratum (dated tables are immutable); same for the §reliable
design-assumption line.
- _read_rss_gib docstring now cites the gauge, not the deleted watermark.
- MemoryConfig signpost: removal record + revert pointer + the duckling
asymmetry. source-conn probe comment records its FALSIFIED verdict.
- README metrics table gains viaduck_rss_bytes + viaduck_dest_conn_sweeps_total.
- test: stale-chart warn-not-refuse coverage.
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
1 participant
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.
Why
The dest-connection age sweep (#84) has held prod flat for ~19h: zero watermark events against the prior ~70-90min seppuku cadence. The watermark existed to preempt the Leak-2 OOMKill with a controlled drain; the sweep makes the crossing never happen. The process no longer kills itself on an RSS watermark.
What goes away
memory.self_recycle_enabled/self_recycle_rss_fraction/self_recycle_rss_gib/self_recycle_min_uptime_seconds(config + validation + load wiring)resolve_recycle_watermark,_should_self_recycle,_cgroup_memory_limit_gib,_RECYCLE_DRAIN_TIMEOUT_Sviaduck_self_recycles_totalcounterWhat stays
_read_rss_giband the per-cycleviaduck_rss_bytesgauge (now the only memory-safety signal — see "deploy notes")_maybe_recycle(per-pod RSS limit) — single-connection pods have no pool to sweep; the pod is the unit there. Signposted inMemoryConfig.Deploy notes (from adversarial review)
memory.self_recycle_*keys in a deployed chart are now WARN-ignored, never refused (prod's chart still sets them; refusal would CrashLoop). A charts-repo cleanup PR should follow.viaduck_self_recycles_totalgone and no in-repo RSS panel, the leading indicator of a new leak class isviaduck_rss_bytesslope / OOMKill — confirm an infra-level alert exists before rollout.1440dc3,f34627e+ this one).Validation
1052 tests pass; ruff clean. Both adversarial reviews (lead QE + principal SWE) — convergent P1 (silent stale-key acceptance) fixed and covered by a new test; the P2/P3 docstring/history/metrics-table findings are in the follow-up commit.