Commit graph

5 commits

Author SHA1 Message Date
Sebastian Schkudlara
517bf992cf
fix(proxy): quarantine compression while timed-out workers run (#2292)
## Description

A request-side `asyncio.wait_for()` timeout stops waiting, but it cannot
preempt an executor thread that already started. The proxy counted those
late workers and still admitted more compression, so repeated slow calls
could consume the whole compression pool and charge every request
another full timeout.

This change tracks running post-timeout workers as timeout debt and
quarantines request-path compression while that debt is non-zero. New
attempts raise `CompressionQuarantinedError` before executor admission,
using an `asyncio.TimeoutError` subclass so Python 3.10 handlers apply
the existing compression-failure policy. Quarantine clears automatically
after all known timed-out workers genuinely exit.

Mitigates #946 and #810. It does not attempt to kill the first running
thread; Python cannot safely preempt it.

## 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)
- [x] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- Track started, finished, timed-out, and debt-recorded state under the
existing compression metrics lock.
- Reject new compression before enqueue while timed-out workers remain;
clear quarantine on the final worker exit.
- Preserve queued-timeout behavior: work cancelled before worker start
does not activate quarantine; a cancellation/start race is
conservatively tracked as running debt.
- Add `/health` and `/stats` runtime fields for quarantine state, worker
debt, activations, and skips.
- Add `headroom_compression_quarantine_total{event="activated|skipped"}`
Prometheus counters.
- Add regression, recovery, queue-race, runtime-payload, export, reset,
and Python 3.10 exception-class coverage.
- Update `CHANGELOG.md`; no dependency or lockfile changes.

## Reproduction

On base commit `718c8dc5`, I applied only the new regression test and
ran:

```bash
.venv/bin/pytest -q \
  tests/test_proxy_compression_executor.py::test_timeout_quarantines_new_work_until_timed_out_worker_finishes
```

The first worker timed out but remained blocked. The second callable
entered the executor instead of being rejected:

```text
FAILED: DID NOT RAISE TimeoutError
```

## Testing

- [x] Affected unit tests pass
- [x] Linting passes (`ruff check .`)
- [x] Changed-file type checking passes
- [x] New tests added for the fix
- [x] Manual testing performed

### Test Output

```text
Python 3.10.20

$ .venv/bin/pytest -q -m 'not slow' \
    tests/test_proxy_compression_executor.py \
    tests/test_prometheus_obs_counters.py \
    tests/test_proxy/test_compression_failure_action.py \
    tests/test_proxy/test_compression_timeout_config.py \
    tests/test_anthropic_pre_upstream_backpressure.py \
    tests/test_openai_codex_ws_lifecycle.py \
    tests/test_codex_ws_compression_scheduler.py \
    tests/test_gemini_compression_offload.py \
    tests/test_proxy_handlers_batch.py \
    tests/test_tokenizer_count_offload.py \
    tests/test_cold_start_fast_pass.py
125 passed, 1 skipped, 1 deselected, 1 warning in 11.06s

$ .venv/bin/ruff check .
All checks passed!

$ .venv/bin/ruff format --check .
1310 files already formatted

$ .venv/bin/mypy headroom/proxy/server.py headroom/proxy/prometheus_metrics.py
Success: no issues found in 2 source files

$ git diff --check
# no output
```

The warning is the existing Starlette `TestClient`/`httpx` deprecation
warning.

## Real Behavior Proof

- Environment: macOS 15.7.4 x86_64, Python 3.10.20,
`compression_max_workers=2`, direct proxy executor path, no external
provider/model.
- Exact command / steps: instantiate the proxy; run a blocking
compression callable with a 50 ms timeout; immediately attempt a second
callable and time the rejection; release the first worker; wait for debt
to reach zero; run the second callable again; export Prometheus metrics.
- Observed result: the first request timed out at 51.182 ms; the second
attempt was rejected in 0.014 ms and its callable never started; debt
was 1 while quarantined, returned to 0 after release, and compression
then resumed normally.

```json
{
  "after_release": {
    "activations_total": 1,
    "leaked_threads_total": 1,
    "quarantine_active": false,
    "skips_total": 1,
    "timed_out_workers": 0
  },
  "bypass_elapsed_ms": 0.014,
  "bypass_error": "compression quarantined: 1 timed-out worker(s) still running",
  "during_quarantine": {
    "quarantine_active": true,
    "timed_out_workers": 1
  },
  "first_timeout_elapsed_ms": 51.182,
  "prometheus": [
    "headroom_compression_quarantine_total{event=\"activated\"} 1",
    "headroom_compression_quarantine_total{event=\"skipped\"} 1"
  ],
  "resumed_result": "resumed",
  "second_callable_started_during_quarantine": false
}
```

- Not tested: a live external model/provider; forced termination of a
permanently wedged native worker; the marked slow native scheduler
benchmark. A broad non-slow run collected 9,859 selected tests but was
stopped at
`tests/test_adversarial_grid.py::TestRunGrid::test_grid_shape_and_schema`
after a macOS process sample showed the pre-existing native
`_core.abi3.so` semaphore stall (`_dispatch_semaphore_wait_slow` →
`semaphore_wait_trap`). The affected executor/handler slice above
completed cleanly.

## 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 hard-to-understand concurrency paths
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove the fix is effective
- [x] New and affected existing unit tests pass locally
- [x] I have updated the CHANGELOG.md

---------

Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-07-16 14:30:06 -07:00
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
Rod Boev
15ac650d40
fix(proxy): fail open when kompress saturation would exhaust pre-upstream budget (#1430)
## Description

Concurrent Anthropic `/v1/messages` traffic can still exhaust Headroom's
pre-upstream budget because Kompress ONNX execution waits on the request
critical path. When Kompress saturates, requests eventually fail with
`503 pre-upstream queue saturated` even though compression can safely
degrade to passthrough.

This PR makes Kompress saturation fail open on the Anthropic hot path,
so requests continue uncompressed when compression capacity is under
pressure. It keeps the executor and stage-timing evidence intact, and it
preserves blocking model-load validation so runtime pressure does not
silently skip the validation path.

Closes #1025

## 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
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- add a bounded execution-slot acquire path so Anthropic requests fail
open to passthrough when Kompress saturation would consume the
pre-upstream budget
- preserve explicit execution-timeout counters and Anthropic
passthrough/stage-timing observability instead of hiding the pressure
path
- keep `_validate_pytorch_device()` on blocking acquire semantics so
model-load validation still waits for capacity instead of failing open
- make the blocking validation acquire explicit to `mypy` without
changing runtime behavior
- extend focused regressions for pre-upstream backpressure, Kompress
saturation, execution-skip observability, and validation waiting
- align the CLI timeout help text and `ProxyConfig` comment with the
fail-open runtime behavior
- update `CHANGELOG.md` for the proxy runtime fix

## Testing

- [x] Unit tests pass (`uv run pytest
tests/test_anthropic_pre_upstream_backpressure.py
tests/test_proxy_compression_executor.py
tests/test_kompress_request_nonblocking.py`)
- [x] Linting passes (`uv run ruff check
tests/test_anthropic_pre_upstream_backpressure.py` and `uv run ruff
format tests/test_anthropic_pre_upstream_backpressure.py --check`)
- [x] Type checking passes (`uv run mypy headroom
--ignore-missing-imports`)
- [x] New tests added for new functionality when applicable
- [ ] Manual testing performed

### Test Output

```text
Focused local validation passed:
- uv run pytest tests/test_anthropic_pre_upstream_backpressure.py tests/test_proxy_compression_executor.py tests/test_kompress_request_nonblocking.py -x -v
  37 passed, 1 warning in 12.01s
- uv run ruff check headroom/proxy/handlers/anthropic.py headroom/transforms/kompress_compressor.py tests/test_anthropic_pre_upstream_backpressure.py tests/test_proxy_compression_executor.py tests/test_kompress_request_nonblocking.py
  All checks passed!
- uv run ruff format headroom/proxy/handlers/anthropic.py headroom/transforms/kompress_compressor.py tests/test_anthropic_pre_upstream_backpressure.py tests/test_proxy_compression_executor.py tests/test_kompress_request_nonblocking.py --check
  5 files already formatted
- uv run mypy headroom --ignore-missing-imports
  Success: no issues found in 398 source files

Base-branch proof on origin/main (fa05ebc849):
- test_acquire_timeout_degrades_to_passthrough fails because the handler still returns 503
- test_saturation_fail_open_does_not_hang_request fails because get_kompress_execution_stats() does not exist
- test_compression_executor_skip_signal_remains_visible passes on base too, so it stays as compatibility coverage rather than the failing-then-passing proof for this fix

Review-follow-up validation passed after aligning the timeout wording with fail-open behavior:
- uv run pytest tests/test_anthropic_pre_upstream_backpressure.py -x -v
  20 passed, 1 warning in 1.38s
- uv run ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/handlers/anthropic.py headroom/transforms/kompress_compressor.py tests/test_anthropic_pre_upstream_backpressure.py tests/test_proxy_compression_executor.py tests/test_kompress_request_nonblocking.py
  All checks passed!
```

## Real Behavior Proof

- Environment: local Anthropic pre-upstream and Kompress execution
regression harnesses covering the `/v1/messages` hot path
- Exact command / steps: run the focused pytest command above on
`origin/main` and on this branch, including the semaphore-saturation
path in `test_saturation_fail_open_does_not_hang_request` and the
validation-slot hold in `test_validation_probe_waits_for_execution_slot`
- Observed result: the reviewed head no longer returns `503` on the
pre-upstream pressure path, request-thread Kompress saturation degrades
to passthrough while incrementing execution timeout stats, and
model-load validation still waits for capacity instead of failing open
- Not tested: wrap/install fallout mentioned in the original issue

## 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
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new type-check or lint 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

- Scoped to the runtime queue-pressure fault only; the issue's
wrap/unwrap and deployment complaints stay out of this PR.
- `test_compression_executor_skip_signal_remains_visible` remains in the
suite to prove the skip signal stays visible, but it is compatibility
coverage rather than the failing-then-passing regression for the bug
fix.
- Local validation included `uv run mypy headroom
--ignore-missing-imports` after the explicit validation-acquire
narrowing was added for CI parity.
- Attribution: the issue body isolated the hot-path ONNX compression
stall and the pre-upstream saturation symptom that this PR fixes.
2026-06-30 13:41:22 -05:00
Tejas Chopra
e9cae0131b fix: expose compression latency bottlenecks
Add Codex WS unit-level timing and bounded parallel compression, clarify context-tool session savings, and avoid costly diff/log fallbacks to Kompress.
2026-05-12 13:34:08 -07:00
chopratejas
ea78cf6252 fix(proxy): cache concurrency lock, multi-worker docs, bounded compression executor
Three audit follow-ups from issue #327's deep-dive review.

C1 — CompressionCache concurrency lock
======================================

`CompressionCache` instances are shared per `session_id` and accessed from
async-dispatched threadpool workers. Pre-fix, concurrent requests for the
same session raced on `_cache`, `_stable_hashes`, `_first_seen`, and
`_total_tokens_saved` with no synchronization. Observable failures:

* Lost-update on `_total_tokens_saved` (read-modify-write).
* `RuntimeError: OrderedDict mutated during iteration` from `apply_cached`
  when a concurrent `store_compressed` evicts during the walk.
* Lost stable-hash records — next-turn compute_frozen_count reads
  inconsistent state.

May also explain part of SvenMeyer's `_cache: 0 entries / 1003 misses`
observation: the cache was being clobbered concurrently.

Added `threading.RLock` guarding all mutating methods. `RLock` (not `Lock`)
so future code can call locked methods from inside another locked method
without self-deadlock. Also locked `HeadroomProxy._compression_caches`
dict-of-caches access via a separate `_compression_caches_lock` so two
concurrent calls for the same session_id can't each create distinct
CompressionCache objects (which would split the cache state between them).
The `/stats` endpoint snapshots the cache list under the dict lock before
iterating to avoid eviction-during-iteration.

C2 — Multi-worker CCR fragmentation: documented + startup warning
=================================================================

The in-memory `InMemoryCcrStore` (Rust), `_compression_caches` (Python),
`session_tracker_store` (Python), and TOIN learner state are ALL
per-process. Multi-worker uvicorn round-robins requests across workers,
so a session whose turn-1 lands on worker A may have turn-2 land on
worker B. Worker B has zero knowledge of A's CCR markers, replay cache,
or prefix-cache state. Result: `Retrieve original: hash=X` markers stay
in-context as opaque directives, every fresh tool_result is recompressed
from scratch, and `frozen_message_count=0` causes Anthropic prefix-cache
busts on every cross-worker turn.

Added a "Multi-worker deployment — CCR fragmentation" section in
`RUST_DEV.md` documenting the failure modes, the supported configuration
(`--workers 1`), and the sticky-session workaround for horizontal scale.
The proxy emits a `WARNING`-level log line on startup if `workers > 1` is
detected, pointing at the doc section.

C3 — Bounded compression executor with cancel-aware metrics
===========================================================

`asyncio.wait_for(asyncio.to_thread(pipeline.apply), timeout=...)`
cancellation does NOT propagate into the threadpool worker that's running
Rust code. Once the worker has picked up the task,
`concurrent.futures.Future.cancel()` returns False and the thread runs to
completion. Stuck threads accumulated invisibly on asyncio's default
executor, contending with unrelated `to_thread` callers (file IO, etc.).

Replaced all 7 `asyncio.to_thread` call sites for `pipeline.apply()`
across `proxy/handlers/anthropic.py` (3) and `proxy/handlers/openai.py` (4)
with a new `HeadroomProxy._run_compression_in_executor(fn, *, timeout)`
helper that:

  1. Submits to a dedicated bounded `ThreadPoolExecutor` named
     `headroom-compress` (configurable via
     `ProxyConfig.compression_max_workers`; defaults to
     `min(32, (cpu_count or 1) * 4)`).
  2. Increments `_compression_in_flight` (gauge) when work starts and
     decrements when work completes; tracks `_compression_in_flight_max`
     as a high-water mark.
  3. Detects "leaked threads" by comparing wall-clock elapsed against the
     timeout in the worker's `finally` block. Increments
     `_compression_leaked_threads` when a worker finishes after its
     asyncio future was cancelled. Operators can see the leaked-thread
     rate climbing in `/stats runtime.compression_executor` BEFORE the
     pool fills up.

Tests
=====

* `TestCompressionCacheConcurrency` (3 tests) — many threads
  store_compressed / apply_cached / update_from_result on a single
  CompressionCache; assert no exceptions, no lost updates, no partial
  state.
* `test_get_compression_cache_returns_same_instance_under_contention` —
  32 concurrent `_get_compression_cache(same_id)` calls return the
  identical instance (would split pre-lock).
* `test_proxy_compression_executor.py` (8 tests) — pool size respects
  config, in-flight gauge tracks running compressions, high-water mark
  is monotonic, timeout propagates to awaiter, leaked-thread counter
  increments on post-deadline completion, `/stats` surfaces all three
  gauges.

Verification
============

* All 123 targeted regression tests pass.
* `make ci-precheck` clean.
* No `Co-Authored-By` trailer; conventional `fix:` prefix; no
  `--no-verify`.
2026-05-01 15:25:18 -07:00