Skip to content

Commit 08d8737

Browse files
committed
fix(diagnostics): bound in-flight trim skip
1 parent ce9a977 commit 08d8737

4 files changed

Lines changed: 72 additions & 9 deletions

File tree

airlock/callbacks/oom_diagnostics.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,25 @@ def _signal_usr1(self, _signum: int, _frame: Any) -> None:
342342
def _signal_usr2(self, _signum: int, _frame: Any) -> None:
343343
self._signal_snapshot("signal_usr2", trim=True)
344344

345+
def _record_inflight_trim_skip(self, phase: str, in_flight: int) -> None:
346+
"""Record why a trim was skipped without perturbing the live workload.
347+
348+
A full signal snapshot enumerates the Python heap. That work is
349+
specifically unhelpful while requests are in flight, and can hold the
350+
signal gate long enough to make a second operator signal look stuck.
351+
Preserve the bounded audit event while deliberately omitting sampled
352+
process details.
353+
"""
354+
self._append(
355+
{
356+
"ts_monotonic_ns": time.monotonic_ns(),
357+
"phase": f"{phase}_skipped_in_flight",
358+
"sequence": None,
359+
"in_flight": in_flight,
360+
"trim": {"attempted": False, "reason": "requests_in_flight"},
361+
}
362+
)
363+
345364
def _signal_snapshot(self, phase: str, *, trim: bool) -> None:
346365
if not _enabled() or not self._signal_lock.acquire(blocking=False):
347366
return
@@ -351,7 +370,7 @@ def worker() -> None:
351370
with self._lock:
352371
in_flight = self._in_flight
353372
if trim and in_flight:
354-
self.snapshot(f"{phase}_skipped_in_flight")
373+
self._record_inflight_trim_skip(phase, in_flight)
355374
return
356375
self.snapshot(f"{phase}_before")
357376
if trim:

dev/debugging/instrumentation/oom-high-water.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,13 @@ Each record has a monotonic timestamp and a phase:
5050
- `periodic` — every `AIRLOCK_OOM_DIAGNOSTICS_EVERY` requests.
5151
- `signal_usr1_*` or `signal_usr2_*` — an operator snapshot.
5252

53-
The payload includes cgroup current/peak/high/max/event counters, process RSS,
53+
Most payloads include cgroup current/peak/high/max/event counters, process RSS,
5454
`smaps_rollup` anonymous/huge-page values, PSI, glibc `mallinfo2`, thread/FD
5555
counts, optional tracemalloc totals, aggregate GC type counts at checkpoints,
56-
and LiteLLM/httpx client-pool counts.
56+
and LiteLLM/httpx client-pool counts. `signal_usr2_skipped_in_flight` is the
57+
deliberate exception: it contains only its timestamp, phase, in-flight count,
58+
and `trim` decline reason. It does not scan the heap or sample process state
59+
while live requests make trimming unsafe.
5760

5861
Important: `in_flight` is decremented at `callback_complete`, not at
5962
`provider_response`. A growing value can therefore mean a stuck callback or

docs/operations.md

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,15 @@ AIRLOCK_OOM_DIAGNOSTICS_MAX_RECORDS=6000
221221
```
222222

223223
It writes aggregate process, allocator, file-descriptor, thread, cgroup, and
224-
memory-pressure counters. It does not write request/response bodies, headers,
225-
model names, exception text, or client metadata. The directory defaults to
226-
`/tmp/airlock-oom-diagnostics`; output files are owner-only. Secure or
227-
pre-create a configured directory with the permissions your deployment needs.
228-
Disable the recorder after the investigation. The repository's engineering OOM
229-
instrumentation runbook contains the signal and high-water procedure.
224+
memory-pressure counters. A `SIGUSR2` trim declined because requests are active
225+
is intentionally recorded as a minimal audit event instead, so it does not
226+
perturb those requests with a heap scan. It does not write request/response
227+
bodies, headers, model names, exception text, or client metadata. The directory
228+
defaults to `/tmp/airlock-oom-diagnostics`; output files are owner-only. Secure
229+
or pre-create a configured directory with the permissions your deployment
230+
needs. Disable the recorder after the investigation. The repository's
231+
engineering OOM instrumentation runbook contains the signal and high-water
232+
procedure.
230233

231234
## Startup Modes
232235

tests/test_oom_diagnostics.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,44 @@ def test_diagnostics_are_bounded_and_trim_skips_inflight(monkeypatch, tmp_path):
8383
assert any(record["phase"] == "signal_usr2_skipped_in_flight" for record in records)
8484

8585

86+
def test_inflight_trim_signal_uses_a_minimal_skip_record(monkeypatch, tmp_path):
87+
"""An in-flight trim must not walk the heap before releasing its signal lock."""
88+
monkeypatch.setenv("AIRLOCK_OOM_DIAGNOSTICS", "true")
89+
monkeypatch.setenv("AIRLOCK_OOM_DIAGNOSTICS_DIR", str(tmp_path))
90+
91+
diagnostics = OOMDiagnostics()
92+
diagnostics.pre_call({"metadata": {}})
93+
snapshot_calls: list[tuple[tuple[object, ...], dict[str, object]]] = []
94+
monkeypatch.setattr(
95+
diagnostics,
96+
"snapshot",
97+
lambda *args, **kwargs: snapshot_calls.append((args, kwargs)),
98+
)
99+
diagnostics._signal_snapshot("signal_usr2", trim=True)
100+
deadline = time.monotonic() + 2
101+
while diagnostics._signal_lock.locked() and time.monotonic() < deadline:
102+
time.sleep(0.01)
103+
104+
assert not diagnostics._signal_lock.locked()
105+
assert snapshot_calls == []
106+
records = [
107+
json.loads(line)
108+
for line in next(tmp_path.glob("*.jsonl")).read_text().splitlines()
109+
]
110+
skipped = next(
111+
record
112+
for record in records
113+
if record["phase"] == "signal_usr2_skipped_in_flight"
114+
)
115+
assert skipped == {
116+
"ts_monotonic_ns": skipped["ts_monotonic_ns"],
117+
"phase": "signal_usr2_skipped_in_flight",
118+
"sequence": None,
119+
"in_flight": 1,
120+
"trim": {"attempted": False, "reason": "requests_in_flight"},
121+
}
122+
123+
86124
def test_tracemalloc_can_be_disabled_for_a_low_perturbation_replay(
87125
monkeypatch, tmp_path
88126
):

0 commit comments

Comments
 (0)