Repository navigation
feat(limits): stage-1 resource-limit telemetry for instance containers - #140
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
omarsy
force-pushed
the
feat/instance-limits-telemetry
branch
2 times, most recently
from
October 4, 2026 22:17
54b2431 to
01efd57
Compare
omarsy
force-pushed
the
feat/instance-limits-telemetry
branch
from
October 9, 2026 10:33
01efd57 to
0bac9b4
Compare
compose-containment Feature 4, stage 1 of the HLD's two-stage rollout.
No container is constrained yet: the measurements the stage-2 limits
need do not exist (observe is one-shot, no history, no pids collection),
so this stage IS the measurement.
- apps/utils/docker/limits.py: docker-style size parser (overflow- and
NaN-safe on the SCALED product — int(inf) once escaped the 'never
raises' contract and crash-looped boot via the Settings validator),
the computed per-node limit set read from
GREFFER_INSTANCE_MEM_LIMIT / _CPUS / _PIDS (defaults 2g / 2.0 / 512,
exported as the single source app/settings.py also binds; '' or 0
disables a dimension — for mem too, matching docker's --memory=0 and
the cpus/pids semantics), and the over-limit comparison against a
stats digest. Warnings are rate-limited per (instance, service) per
10 minutes, with an explicit membership test rather than a 0.0
sentinel (monotonic clocks start near 0; the sentinel swallowed every
first warning for ten minutes after boot) and a snapshot-before-
delete prune (a live-view iteration raced concurrent digests into
RuntimeError). pids is carried for stage 2 but deliberately not
compared — it has no observation channel, and a pretend measurement
would be worse than none.
- observe.instance_stats runs the pass per digested read, wrapped so a
telemetry bug can never fail a stats response; the payload is
unchanged (log signal, not a contract change).
- Settings gains the three knobs with the typo-must-not-crash
validators of the log-rotation precedent, sharing limits.py's
constants and its finite/non-negative acceptance rule so the two
views cannot drift (a divergent validator held -1.0/nan while the
telemetry fell back).
- tests (27): parser incl. overflow + loud compound-size refusal, env
override/off/fallback incl. MEM=0 and CPUS=nan, exceed, multi-core
CPU scale, rate limit, disabled dimensions, malformed entries, the
concurrent-prune snapshot, first-warn-after-boot, and the Settings
coercions that had zero coverage when the boot crash was found. An
autouse fixture clears/restores the limits cache and rate-limit map
(the test_observe convention) and forces greffer-logger propagation
(app/logging.py sets propagate False, blinding caplog otherwise).
Independent adversarial review (DO NOT SHIP -> folded): the three P1s
above with their reproductions, the P2s (defaults single-sourced,
Settings validator coverage, reformat churn reverted to base style so
the feature hunks are the whole diff of the two touched files).
Stage 2 (rendering mem_limit/cpus/pids_limit per service) waits for one
release of these warnings; HLD open question 'resource-limit values'.
Codex CLI review (bundled 0.162): P2 — docker's two-letter size suffixes
(kb/mb/gb) fell back to the default, silently disabling the operator's
threshold ('512mb' -> 2g, a 1GiB container warned nothing); the parser
now strips the trailing b before the suffix lookup, with regression
tests. P3 — the concurrency test leaked the 1e-6 switch interval into
the rest of the suite; restored in the finally (the prior verification
round had flagged this as a future one-liner; codex promoted it).
omarsy
force-pushed
the
feat/instance-limits-telemetry
branch
from
October 10, 2026 11:58
0bac9b4 to
80ae6f3
Compare
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.
compose-containment Feature 4, stage 1 (telemetry only) per the HLD two-stage rollout: no measurement history exists (observe is one-shot, no pids collection), so stage 1 is the measurement — this constrains nothing.
What it adds:
apps/utils/docker/limits.py— docker-style size parser; the computed per-node limit set fromGREFFER_INSTANCE_MEM_LIMIT/_CPUS/_PIDS(defaults2g/2.0/512, the calibration hypotheses;''/0disables a dimension — the semantics stage 2 will reuse); and the over-limit comparison against a stats digest. Warnings rate-limited to one per (instance, service) per 10 min — the digest is pull-driven, so every metrics poll is an observation.pidsis carried for stage 2 but deliberately not compared: no observation channel exists, and a pretend measurement would be worse than none.observe.instance_statsruns the pass per digested read, wrapped so a telemetry bug can never fail a stats response; the payload is unchanged.Tests: 16 new (parser incl. loud compound-size refusal, env override/off/fallback, exceed, multi-core CPU scale, rate limit, disabled dimensions, malformed entries); full suite 939 passed. The autouse propagation fixture exists because
app/logging.pysetspropagate: Falseand any earlier app-importing test otherwise blindscaplog.Stage 2 (rendering
mem_limit/cpus/pids_limitper service) waits for one release of these warnings — HLD open question "resource-limit values".