Skip to content

fix(proxy): sweep idle bridge sessions without request traffic - #1747

Merged
Soju06 merged 3 commits into
mainfrom
fix/prune-idle-bridge-sessions-without-traffic
Aug 14, 2026
Merged

fix(proxy): sweep idle bridge sessions without request traffic#1747
Soju06 merged 3 commits into
mainfrom
fix/prune-idle-bridge-sessions-without-traffic

Conversation

@Soju06

@Soju06 Soju06 commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Replaces #1716, which I am closing. Same issue (#1354), a much smaller and safer answer — the reasoning for the change of approach is below because it corrects a premise I had wrong.

The problem

_prune_http_bridge_sessions_locked is reached from exactly one place, _get_or_create_http_bridge_session. The idle sweep is therefore request-driven: a replica that stops receiving bridge requests never evicts its idle sessions and holds their upstream WebSockets, registry entries, and durable claims until the process restarts. In a multi-replica deployment, traffic simply moving off a replica pins its warm sessions indefinitely.

The change

  • prune_idle_http_bridge_sessions() — take the bridge lock, run the same _prune_http_bridge_sessions_locked selection the request path uses, schedule the closes through the existing bounded close scheduler.
  • Driven from the per-replica ring heartbeat, next to the durable-ownership reconcile that already runs there, so it fires regardless of traffic, leadership, or durable-row cleanup.

No new selection logic and no new timing. Eligibility, the idle TTL that protects a session freshly handed to a request, and the close path are all unchanged — which is the entire point.

Why not #1716's approach

#1716 built a bespoke reconcile driven by the leader's abandoned-row purge, on the premise that orphaned in-memory sessions hold account stream leases and exhaust the per-account cap. That premise was wrong, and the maintainer caught it:

  • Releasing a lease is a counter decrement; it cannot disturb a request already sent upstream.
  • More importantly, fix (a) (fix(proxy): release idle bridge sessions' account stream leases #1476) already made leases turn-scoped. _maybe_release_idle_http_bridge_session_lease fires on the turn-completion paths and releases the lease when queued_request_count == 0 and admission_waiter_count == 0 and not pending_requests — which is exactly the set of sessions a reconcile can target. Sessions still holding a lease have pending work and are skipped. So that reconcile freed approximately zero cap slots.

With the capacity rationale gone, what remained was resource hygiene on a quiet replica — and the existing idle sweep already does that correctly, including the idle-TTL guard against the pre-submit handoff race that #1716 spent eleven review rounds failing to solve with heuristics. The right fix was to run the proven sweep off the request path, not to build a second one.

Diff: +127 lines, no deletions, versus #1716's new module, invalidation namespace, cross-replica signalling, and shutdown-drain machinery.

Tests

  • Idle session evicted with no request traffic; freshly-used session spared.
  • Session with pending work spared even past its idle TTL.
  • Empty registry is a no-op that schedules no cleanup task.

OpenSpec

openspec/changes/sweep-idle-bridge-sessions-off-request-path/ — ADDED requirement on sticky-session-operations. Validation passes.

Gates green locally: ruff, ruff format, check_proxy_architecture.py (mixin.py lands at exactly 2436/2436), scoped ty, 544 unit tests.

Two follow-ups found while investigating, both out of scope here:

  1. If the turn-scoped idle release fails, the local lease reference is already cleared, so nothing can retry it and the load balancer counts that slot until its TTL — closer to HTTP-bridge stream lease held for whole session lifetime exhausts per-account stream cap (header-less stalls) #1354's original symptom than anything either PR addresses.
  2. CacheInvalidationPoller._flush_pending_bumps clears each pending marker before awaiting its write, so a cancelled write — which stop() causes by design — silently drops that namespace, for every namespace. PR to follow.

Refs #1354

🤖 Generated with Claude Code

@Soju06

Soju06 commented Aug 14, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3198e470d3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/unit/test_proxy_http_bridge.py
@Soju06

Soju06 commented Aug 14, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7bb57ca8ee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/unit/test_proxy_http_bridge.py
Soju06 and others added 3 commits August 14, 2026 10:57
_prune_http_bridge_sessions_locked is reached from exactly one place —
_get_or_create_http_bridge_session — so the idle sweep is request-driven. A
replica that stops receiving bridge requests never evicts its idle sessions
and holds their upstream WebSockets until the process restarts.

Expose the existing sweep as prune_idle_http_bridge_sessions() and drive it
from the per-replica ring heartbeat, beside the durable-ownership reconcile
that already runs there. No new selection logic and no new timing: the same
eligibility, the same idle TTL that protects a freshly-handed session, and
the same bounded close path.

This is the residual leg of #1354; the cap-exhaustion symptom was addressed
by turn-scoped leases in #1476, which already release an idle session's slot
when its last turn detaches.

Refs #1354

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sweep tests called prune_idle_http_bridge_sessions() directly, so they
would still pass with the heartbeat wiring removed — leaving the quiet-replica
leak the change exists to fix.

Extract the heartbeat's bridge upkeep into run_http_bridge_heartbeat_maintenance()
and test it: both passes run, a failing reconcile does not skip the sweep (nor
stop the heartbeat), and a missing service or pass is tolerated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main moved mixin.py to its 2436-line architecture ratchet, and the sweep is
registry maintenance, so it belongs next to the other registry operations
rather than pushing mixin.py over the limit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant