Skip to content

Commit 7a9e282

Browse files
committed
fix(gateway): release neutral stream circuit probes
Terminal-missing HTTP SSE closes are health-neutral, but they still need to complete any half-open routing circuit probe started before dispatch. Release the probe slot without counting success or failure so OMP retry storms do not collapse into no_eligible_backend while an interrupted stream is in flight.
1 parent 4c1873f commit 7a9e282

6 files changed

Lines changed: 166 additions & 2 deletions

File tree

lib/codex_pooler/gateway/routing/circuit_state.ex

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,45 @@ defmodule CodexPooler.Gateway.Routing.CircuitState do
233233
),
234234
do: {:error, :invalid_route_class}
235235

236+
@spec record_neutral_completion(auth(), Model.t(), PoolUpstreamAssignment.t(), String.t()) ::
237+
{:ok, :ok | RoutingCircuitState.t()} | {:error, term()}
238+
def record_neutral_completion(
239+
%{pool: %Pool{}, api_key: %APIKey{}} = auth,
240+
%Model{} = model,
241+
%PoolUpstreamAssignment{} = assignment,
242+
route_class
243+
)
244+
when is_binary(route_class) and route_class != "" do
245+
now = now()
246+
247+
Repo.transaction(fn ->
248+
case latest_for_update(auth, model, assignment, route_class) do
249+
%RoutingCircuitState{status: @half_open_status} = state ->
250+
state
251+
|> RoutingCircuitState.changeset(%{
252+
metadata: probe_metadata(state, max(probe_in_flight_count(state) - 1, 0)),
253+
updated_at: now
254+
})
255+
|> persist_or_rollback(:update)
256+
257+
%RoutingCircuitState{} = state ->
258+
state
259+
260+
nil ->
261+
:ok
262+
end
263+
end)
264+
|> unwrap_transaction()
265+
end
266+
267+
def record_neutral_completion(
268+
%{pool: %Pool{}, api_key: %APIKey{}},
269+
%Model{},
270+
%PoolUpstreamAssignment{},
271+
_route_class
272+
),
273+
do: {:error, :invalid_route_class}
274+
236275
defp begin_attempt_with_snapshot(
237276
_auth,
238277
_model,

lib/codex_pooler/gateway/routing/route_lifecycle.ex

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,28 @@ defmodule CodexPooler.Gateway.Routing.RouteLifecycle do
4949
end
5050
end
5151

52+
@spec selection_neutral_completion(map(), CodexPooler.Catalog.Model.t(), RoutingSelection.t()) ::
53+
success_result()
54+
def selection_neutral_completion(
55+
auth,
56+
model,
57+
%RoutingSelection{
58+
circuit_state: %RoutingCircuitState{}
59+
} = selection
60+
) do
61+
case CircuitState.record_neutral_completion(
62+
auth,
63+
model,
64+
selection.assignment,
65+
selection.route_class
66+
) do
67+
{:ok, _state} -> :ok
68+
{:error, reason} -> lifecycle_failure(:record_route_circuit_neutral_completion, reason)
69+
end
70+
end
71+
72+
def selection_neutral_completion(_auth, _model, %RoutingSelection{}), do: :ok
73+
5274
@spec log_optional_result(String.t(), keyword(), success_result() | failure_result()) :: :ok
5375
def log_optional_result(_operation, _metadata, :ok), do: :ok
5476
def log_optional_result(_operation, _metadata, {:ok, _value}), do: :ok

lib/codex_pooler/gateway/runtime/finalization/streaming.ex

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,9 @@ defmodule CodexPooler.Gateway.Runtime.Finalization.Streaming do
204204
"upstream_stream_error",
205205
nil,
206206
_headers,
207-
_context
207+
context
208208
),
209-
do: :ok
209+
do: RouteLifecycle.neutral_completion(context)
210210

211211
defp record_stream_failure_health(reason, code, nil, _headers, context) do
212212
record_health_failure(reason, code, context)

lib/codex_pooler/gateway/runtime/routing/route_lifecycle.ex

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ defmodule CodexPooler.Gateway.Runtime.Routing.RouteLifecycle do
3838
end
3939
end
4040

41+
@spec neutral_completion(DispatchContext.t()) :: success_result()
42+
def neutral_completion(%DispatchContext{} = context) do
43+
RoutingRouteLifecycle.selection_neutral_completion(
44+
context.auth,
45+
context.model,
46+
routing_selection(context)
47+
)
48+
end
49+
4150
defp merge_failure_metadata(%DispatchContext{} = context, demotion_reason) do
4251
case Accounting.merge_request_metadata(
4352
context.reserved.request,

test/codex_pooler/gateway/persistence/routing_circuit_state_test.exs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,22 @@ defmodule CodexPooler.Gateway.Persistence.RoutingCircuitStateTest do
175175
assert DateTime.compare(resumed.updated_at, state.updated_at) == :gt
176176
end
177177

178+
test "neutral completions release half-open probes without counting success or failure" do
179+
{auth, model, assignment} = routing_fixture()
180+
state = half_open_circuit!(auth, model, assignment, updated_at: now(), probe_count: 1)
181+
182+
assert {:ok, %RoutingCircuitState{} = updated} =
183+
CircuitState.record_neutral_completion(auth, model, assignment, "proxy_websocket")
184+
185+
assert updated.id == state.id
186+
assert updated.status == "half_open"
187+
assert updated.reason_code == "test_probe"
188+
assert updated.failure_count == state.failure_count
189+
assert updated.success_count == state.success_count
190+
assert updated.metadata["probe_in_flight_count"] == 0
191+
assert CircuitState.eligible?(auth, model, assignment, "proxy_websocket")
192+
end
193+
178194
defp open_circuit!(auth, model, assignment, attrs) do
179195
now = now()
180196
next_probe_at = Keyword.fetch!(attrs, :next_probe_at)

test/codex_pooler/gateway/runtime/streaming/stream_lifecycle_test.exs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,83 @@ defmodule CodexPooler.Gateway.Runtime.Streaming.StreamLifecycleTest do
300300
assert Repo.all(from(c in RoutingCircuitState)) == []
301301
end
302302

303+
test "terminal-missing upstream SSE close releases half-open route probe" do
304+
{setup, _first_upstream, _second_upstream} =
305+
stream_retry_setup(
306+
FakeUpstream.sse_stream([]),
307+
FakeUpstream.sse_stream([])
308+
)
309+
310+
{:ok, auth} = Access.authenticate_authorization_header(setup.authorization)
311+
payload = payload(setup)
312+
request_options = request_options(auth, payload, setup)
313+
314+
now = DateTime.utc_now() |> DateTime.truncate(:microsecond)
315+
316+
circuit =
317+
%RoutingCircuitState{
318+
pool_id: auth.pool.id,
319+
pool_upstream_assignment_id: setup.assignment.id,
320+
upstream_identity_id: setup.identity.id,
321+
model_identifier: setup.model.exposed_model_id,
322+
route_class: request_options.transport.route_class,
323+
status: "half_open",
324+
reason_code: "upstream_5xx",
325+
failure_count: 3,
326+
success_count: 0,
327+
opened_at: DateTime.add(now, -120, :second),
328+
half_opened_at: now,
329+
metadata: %{"probe_in_flight_count" => 1},
330+
created_at: DateTime.add(now, -120, :second),
331+
updated_at: now
332+
}
333+
|> Repo.insert!()
334+
335+
assert {:ok, reserved} =
336+
Accounting.reserve(auth, setup.model, payload, %{
337+
endpoint: @endpoint_path,
338+
transport: "http_sse",
339+
correlation_id:
340+
"upstream-stream-neutral-probe-#{System.unique_integer([:positive])}",
341+
request_metadata: %{}
342+
})
343+
344+
assert {:ok, attempt} = Accounting.create_attempt(reserved.request, setup.assignment)
345+
346+
context =
347+
retry_context(setup, auth, request_options, reserved.request,
348+
candidates: [{setup.assignment, setup.identity}],
349+
attempt: attempt,
350+
routing_circuit_state: circuit
351+
)
352+
353+
response_context = %ResponseContext{context: context, response: %Req.Response{status: 200}}
354+
355+
assert {:ok, _finalized} =
356+
Streaming.finalize_failure(
357+
"event: response.created\n\n",
358+
:upstream_stream_interrupted,
359+
response_context
360+
)
361+
362+
assert [request] = Repo.all(from(r in Request, where: r.id == ^reserved.request.id))
363+
assert request.status == "failed"
364+
assert request.last_error_code == "upstream_stream_error"
365+
366+
assert [attempt] = Repo.all(from(a in Attempt, where: a.request_id == ^request.id))
367+
assert attempt.status == "failed"
368+
assert attempt.network_error_code == "upstream_stream_error"
369+
370+
assert Repo.all(from(d in BridgeDemotion)) == []
371+
372+
assert %RoutingCircuitState{} = updated = Repo.get!(RoutingCircuitState, circuit.id)
373+
assert updated.status == "half_open"
374+
assert updated.reason_code == "upstream_5xx"
375+
assert updated.failure_count == 3
376+
assert updated.success_count == 0
377+
assert updated.metadata["probe_in_flight_count"] == 0
378+
end
379+
303380
defp retry_context(setup, auth, request_options, request, opts \\ []) do
304381
candidates =
305382
Keyword.get(opts, :candidates, [
@@ -330,6 +407,7 @@ defmodule CodexPooler.Gateway.Runtime.Streaming.StreamLifecycleTest do
330407
allow_retry?: true,
331408
routing_attempt_metadata: %{},
332409
route_class: request_options.transport.route_class,
410+
routing_circuit_state: Keyword.get(opts, :routing_circuit_state),
333411
attempt: Keyword.get(opts, :attempt),
334412
started: System.monotonic_time(:millisecond)
335413
}

0 commit comments

Comments
 (0)