headroom/tests/test_codex_ws_compression_scheduler.py
Rod Boev 0a3851b240
perf(proxy): cap compression workers to CPU count (#1803)
## Description

The request-path compression executor currently uses asyncio-style I/O
sizing for CPU-bound Kompress work. When `compression_max_workers` is
unset, `HeadroomProxy.__init__` resolves the pool to `min(32, cpu * 4)`,
so an eight-core host can run 32 simultaneous compression workers that
all contend for real CPU.

This changes only the automatic request-path default to one worker per
reported CPU while preserving the existing explicit override path from
`--compression-max-workers` and `HEADROOM_COMPRESSION_MAX_WORKERS`.

Closes #1635

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [x] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- Cap the automatic request-path compression executor default at `max(1,
os.cpu_count() or 1)`.
- Preserve explicit `compression_max_workers` values, including the
existing clamp to at least one worker.
- Keep CLI help, `ProxyConfig` comments, and nearby test documentation
aligned with the CPU-bound default.
- Update the focused compression executor regression so the default
contract documents CPU-bound sizing, and keep the existing Codex
compression stress guard stable when p50 rounds to zero.

## Testing

- [x] Unit tests pass (`uv run pytest
tests/test_codex_ws_compression_scheduler.py
tests/test_proxy_compression_executor.py
tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q`)
- [x] Linting passes (`uv run ruff check headroom/cli/proxy.py
headroom/proxy/models.py headroom/proxy/server.py
tests/test_cli_proxy_improvements.py
tests/test_proxy_compression_executor.py
tests/test_codex_ws_compression_scheduler.py`)
- [ ] Type checking passes (`uv run mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed

### Test Output

```text
$ uv run pytest tests/test_codex_ws_compression_scheduler.py tests/test_proxy_compression_executor.py tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q
16 passed, 1 skipped, 1 warning in 6.13s

$ uv run ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/server.py tests/test_cli_proxy_improvements.py tests/test_proxy_compression_executor.py tests/test_codex_ws_compression_scheduler.py
All checks passed!
```

## Real Behavior Proof

- Environment: Windows, Python environment from `uv sync --extra dev`,
no provider credentials needed.
- Exact command / steps: construct `HeadroomProxy` with
`compression_max_workers=None`, inspect `proxy.compression_max_workers`
and `/health` `runtime.compression_executor`.
- Observed result: the automatic request-path pool resolves to reported
CPU count, while explicit overrides still resolve to the configured
value and report `source: explicit`.
- Not tested: multi-session wall-clock benchmark under live Kompress
load.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

No `CHANGELOG.md` edit: this repo generates changelog entries from
conventional commits. This intentionally does not touch the background
compression executor surface covered by #1633.
2026-07-05 14:01:23 -07:00

321 lines
14 KiB
Python
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

"""P2 — Codex compression scheduler regression coverage.
The pre-fix code throttled all concurrent Codex WS compression units
through a process-global ``threading.BoundedSemaphore(10)`` and created
a fresh ``ThreadPoolExecutor`` per frame. Under realistic concurrent
load (≥10 sessions) the semaphore saturated, ``elapsed_ms`` was measured
INCLUDING the wait time, and frames hit the parent 30s timeout.
The fix:
* Deletes the module-global ``_CODEX_WS_UNIT_ROUTER_SEMAPHORE``.
* Deletes the per-call inner ``ThreadPoolExecutor``.
* Processes routed units serially inside the frame-level worker thread
(``self._compression_executor`` already provides frame-level parallelism
via the proxy-wide bounded executor).
* Adds a ``PERF`` log emission from ``handle_openai_responses_ws`` so
Codex traffic is no longer invisible to ``headroom perf``.
These tests verify that future contributors cannot silently re-introduce
either bottleneck.
"""
from __future__ import annotations
import concurrent.futures
import logging
import sys
import time
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import MagicMock
import pytest
REPO_ROOT = Path(__file__).resolve().parents[1]
OPENAI_HANDLER = REPO_ROOT / "headroom" / "proxy" / "handlers" / "openai.py"
# ── Source-level regression guards ──────────────────────────────────────
def test_module_global_unit_semaphore_is_removed() -> None:
"""The 10-slot global semaphore that caused 30s frame timeouts must stay gone.
Read the source file directly — imported module state is not authoritative
because Python caches bytecode independently. The regression we are
guarding against is "someone reintroduces a module-level semaphore on
the Codex WS dispatch path" — that is detectable in source.
"""
source = OPENAI_HANDLER.read_text()
assert "_CODEX_WS_UNIT_ROUTER_SEMAPHORE" not in source, (
"Module-global semaphore on Codex WS path reintroduced. The P2 fix "
"deleted it because it saturated at 10 concurrent units and caused "
"the production cascade documented in issue #327's sibling slowness "
"report. Use `self._compression_executor` (the proxy-wide bounded "
"pool) for any new concurrency needs."
)
assert "_CODEX_WS_UNIT_ROUTER_MAX_WORKERS" not in source, (
"Module-global slot count for the (deleted) Codex unit semaphore reintroduced."
)
assert "_codex_ws_unit_worker_count" not in source, (
"The per-call inner-pool worker-count helper was deleted because the "
"inner pool was deleted. Reintroducing it suggests the inner pool "
"is back too — re-read docs/superpowers/specs/P2-codex-scheduler-fix.md."
)
assert "HEADROOM_CODEX_WS_UNIT_WORKERS" not in source, (
"The HEADROOM_CODEX_WS_UNIT_WORKERS env knob was removed. It only "
"existed to tune around the semaphore bottleneck, which is gone."
)
def test_no_per_call_threadpool_inside_compress_routed_units() -> None:
"""The inner ``ThreadPoolExecutor`` created per frame must stay gone.
Pre-fix, every call to ``_compress_openai_responses_payload`` created
and tore down a ``ThreadPoolExecutor(max_workers=worker_count)`` to run
routed units, layered on top of ``self._compression_executor``. That
pool-on-pool pattern added latency variance, fought for OS threads,
and made the global semaphore the binding constraint.
The exact phrase ``concurrent.futures.ThreadPoolExecutor`` should not
appear anywhere in openai.py — the dispatch uses the proxy's shared
bounded executor instead.
"""
source = OPENAI_HANDLER.read_text()
assert "concurrent.futures.ThreadPoolExecutor" not in source, (
"Per-call ThreadPoolExecutor reintroduced in handlers/openai.py. "
"Submit work to `self._compression_executor` (instrumented and "
"lifecycle-managed) instead of creating a new pool per frame."
)
# ── PERF log emission from the Codex WS path ────────────────────────────
#
# Codex WS traffic was invisible to ``headroom perf`` pre-fix because
# ``handle_openai_responses_ws`` emitted no PERF line. This is structurally
# the same bug class as #327's "Cache write: 0" for backend-routed
# streaming — the request is processed correctly but the operator can't
# see it. The new PERF emit closes that visibility gap.
class _DirectLogCapture(logging.Handler):
"""Direct handler attached to ``headroom.proxy`` so the proxy's
propagation flip in ``_setup_file_logging`` does not strip records.
Same pattern as ``tests/test_backend_streaming_cache_metrics.py`` —
see that file for the rationale.
"""
def __init__(self) -> None:
super().__init__(level=logging.INFO)
self.records: list[logging.LogRecord] = []
def emit(self, record: logging.LogRecord) -> None:
self.records.append(record)
def _attach_proxy_log_capture() -> tuple[_DirectLogCapture, logging.Logger, int]:
handler = _DirectLogCapture()
target = logging.getLogger("headroom.proxy")
target.addHandler(handler)
prior_level = target.level
target.setLevel(logging.INFO)
return handler, target, prior_level
def _detach_proxy_log_capture(handler, target, prior_level) -> None:
target.removeHandler(handler)
target.setLevel(prior_level)
def _make_perf_log_test_handler():
"""Build a minimal handler that lets us drive the PERF emit code path
of ``handle_openai_responses_ws`` end-to-end without a real upstream.
Imported lazily so a collection-time import error in the proxy module
does not break the source-level regression guards above.
"""
from headroom.proxy.handlers.openai import OpenAIHandlerMixin
from headroom.proxy.ws_session_registry import WebSocketSessionRegistry
class _M(OpenAIHandlerMixin):
OPENAI_API_URL = "https://api.openai.com"
def __init__(self) -> None:
self.rate_limiter = None
self.metrics = SimpleNamespace(
record_request=lambda **kw: None,
record_stage_timings=lambda *a, **kw: None,
inc_active_ws_sessions=lambda: None,
dec_active_ws_sessions=lambda: None,
inc_active_relay_tasks=lambda n=1: None,
dec_active_relay_tasks=lambda n=1: None,
record_ws_session_duration=lambda *a, **kw: None,
record_codex_ws_unit=lambda **kw: None,
)
self.config = SimpleNamespace(
optimize=True,
retry_max_attempts=1,
retry_base_delay_ms=1,
retry_max_delay_ms=1,
connect_timeout_seconds=10,
log_full_messages=False,
)
self.usage_reporter = None
self.openai_provider = SimpleNamespace(
get_context_limit=lambda model: 128_000,
get_token_counter=lambda model: SimpleNamespace(
count_text=lambda text: max(1, len(text) // 4),
count_messages=lambda *a, **k: 0,
),
)
self.openai_pipeline = SimpleNamespace(apply=MagicMock(), transforms=[])
self.anthropic_backend = None
self.cost_tracker = None
self.memory_handler = None
self.ws_sessions = WebSocketSessionRegistry()
self.logger = None
self.compression_executor_calls = 0
async def _next_request_id(self) -> str:
return "req-perf-emit-test"
async def _run_compression_in_executor(self, fn, *, timeout: float):
self.compression_executor_calls += 1
return fn()
return _M()
@pytest.mark.asyncio
async def test_codex_ws_emits_perf_log_with_cache_keys() -> None:
"""``handle_openai_responses_ws`` must emit a PERF line so ``headroom
perf`` counts Codex traffic instead of reporting it as zero requests.
Asserts on the structured-PERF kv fragment used by ``headroom/perf/
analyzer.py`` (``cache_read=`` / ``cache_write=`` / ``cache_hit_pct=``)
so the analyzer parser actually picks it up.
"""
pytest.skip(
"Pending: full WS lifecycle harness for handle_openai_responses_ws "
"needs a fuller FakeWebSocket+FakeUpstream wire-up than this file "
"owns. The PERF emit is verified via Tier-3 replay + Tier-4 manual "
"smoke; the source-level guards above prevent the emit from being "
"removed silently. Re-enable when the WS lifecycle harness in "
"test_openai_codex_ws_lifecycle.py is reused as a fixture."
)
# ── Concurrency stress (Tier 2) ─────────────────────────────────────────
#
# The smoking gun: with the old code, 30 concurrent calls to
# ``_compress_openai_responses_payload`` produced p99 per-call latency of
# ~2.4s on a 12-CPU machine because of the 10-slot global semaphore. After
# the fix, units run serially within the frame-level worker, but the
# frame-level compression executor lets 30 frames run in parallel without contention.
#
# Pass criteria mirror docs/superpowers/specs/P2-codex-scheduler-fix.md
# "Success criteria":
# - p99 per-frame < 250ms (vs baseline 2433ms)
# - p99/p50 < 3× (vs baseline 24×)
# - errors == 0
@pytest.mark.slow
def test_concurrent_compression_has_no_semaphore_tail() -> None:
"""Probe the 10-slot semaphore boundary with uniform-size workload.
Design notes — addresses a CI-vs-dev hardware skew that bit the
first iteration of this test:
* **12 concurrent sessions** (> the deleted 10-slot semaphore size).
Enough to saturate the gate if it ever reappears; small enough
that a 2-vCPU CI runner doesn't drown in OS-level scheduler
noise.
* **All frames the same size (4 KB)** so size-induced compute
variance cancels out. Pre-refactor the bug produced bimodal
latency (waiters vs holders) regardless of frame size; this
test must measure THAT, not size variance.
* **5 frames per session** = 60 total. Enough samples to make
the p99 statistic meaningful. Bounded runtime even on slow CI.
* **Threshold ratio < 4×.** On uniform-size workload the only
sources of p99/p50 spread are (a) the deleted semaphore tail
(≈27×) or (b) OS-level scheduler noise (≈23×). 4× sits
comfortably between the two — catches the bug, tolerates
hardware. (First iteration tried 5× with mixed sizes, which
let size-variance push CI ratios to 7.4×.) The ratio is only
enforced once p99 clears a scheduler-noise floor — on very fast
runners p50 rounds to 0ms and the ratio becomes pure jitter.
Marked ``slow`` so a normal ``pytest`` run can skip it via
``-m 'not slow'``. CI matrix runs all marks.
"""
sys.path.insert(0, str(REPO_ROOT))
from scripts.replay_codex_ws_load import ( # noqa: E402
Frame,
Scenario,
boot_proxy,
replay_session,
warmup,
)
proxy = boot_proxy()
warmup_ms = warmup(proxy)
assert warmup_ms < 30_000, (
f"Warmup took {warmup_ms:.0f}ms — Kompress model failed to load? "
"Subsequent timing assertions are meaningless without a warm router."
)
# 12 sessions × 5 frames = 60 total. Uniform 4 KB plain-text
# payload — each frame's compute time should be identical modulo
# scheduler noise.
UNIFORM_FRAME = Frame(bytes_estimate=4096, text_shape="plain_text_like")
scenarios = [
Scenario(
request_id=f"stress-{i:02d}",
frames=[UNIFORM_FRAME] * 5,
)
for i in range(12)
]
results: list = []
started = time.perf_counter()
with concurrent.futures.ThreadPoolExecutor(max_workers=12) as pool:
futures = [pool.submit(replay_session, proxy, s, "gpt-4o-mini") for s in scenarios]
for fut in concurrent.futures.as_completed(futures):
results.extend(fut.result())
wall_s = time.perf_counter() - started
elapsed = sorted(r.elapsed_ms for r in results)
p50 = elapsed[len(elapsed) // 2]
p99 = elapsed[int(len(elapsed) * 0.99)]
errors = [r for r in results if r.error]
# Always print the distribution so CI logs show numbers for
# diagnosing failures and tracking drift across runs.
print(
f"\n[stress] frames={len(results)} wall={wall_s:.2f}s "
f"p50={p50:.0f}ms p99={p99:.0f}ms ratio={p99 / max(p50, 1):.2f}× errors={len(errors)}"
)
assert not errors, f"Got {len(errors)} errors; first: {errors[0].error}"
ratio = p99 / max(p50, 1)
assert p99 < 250.0, f"p99 is {p99:.0f}ms; expected < 250ms on uniform-size workload."
# The p99/p50 ratio only signals contention when the tail is also
# *absolutely* large. On a fast/quiet runner p50 rounds toward 0ms, so the
# ratio collapses to "p99 in ms" and a few milliseconds of ordinary
# scheduler jitter reads as a spurious multiple (e.g. p50=0ms, p99=5ms →
# ~5×) that has nothing to do with the semaphore. The deleted semaphore
# produced a tail of *tens* of milliseconds (and ~27×); a healthy run keeps
# p99 in the single-digit-ms range regardless of ratio. So only treat a high
# ratio as a regression once p50 is measurable and p99 clears a noise floor.
SEMAPHORE_TAIL_FLOOR_MS = 25.0
assert p50 < 1.0 or ratio < 4.0 or p99 < SEMAPHORE_TAIL_FLOOR_MS, (
f"p99/p50 ratio is {ratio:.1f}× (p50={p50:.0f}ms, p99={p99:.0f}ms). "
f"Expected < 4× on uniform-size workload once p50 is measurable and p99 clears "
f"the {SEMAPHORE_TAIL_FLOOR_MS:.0f}ms noise floor — a high ratio with a large "
f"absolute tail means the semaphore-induced contention tail may be back. "
f"Pre-fix baseline ratio on this same workload shape was ~27× regardless "
f"of CPU speed."
)