Skip to content

fix(server): give background identity refreshes their own budget - #618

Merged
krisztian-gajdar merged 3 commits into
mainfrom
fix/identity-read-budget
Oct 7, 2026
Merged

krisztian-gajdar merged 3 commits into
mainfrom
fix/identity-read-budget

Conversation

@krisztian-gajdar

@krisztian-gajdar krisztian-gajdar commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What changes

  • Invariant: background identity refreshes never consume the inference rate cap. A check that does not wait, such as the admission a cluster's remote lane reports and checks before an admitted remote attempt, refreshes an SIE upstream's identity in the background through a new per-upstream identity_limiter. It is an UpstreamLimiter built from the upstream's configuration with a reduced rate cap: a tenth of rate_cap.requests_per_minute, at least one read a minute, and one read in flight. Its circuit breaker uses the upstream's breaker settings but is its own. Inference calls never draw on it.
  • A background refresh this limiter refuses is never sent. It backs off exactly like a failed read: the observation records the failure, keeps a still-valid identity until it expires, and holds off the next read for 2 seconds.
  • Identity reads that a caller waits for, at configuration load, at hot reload and before a single-node bridge, still go through the upstream's inference limiter, as before, so startup behaviour is unchanged. _identity_observation passes the limiter for its path into _refresh and _read_identity.
  • The identity limiter reports no telemetry (UpstreamLimiter(..., telemetry=False)), so it never overwrites the inference breaker gauge of the same upstream. sie.worker.upstream.refusals and sie.worker.upstream.breaker.open keep describing the inference limiter.
  • The _limits.py docstring and REMOTE_BACKENDS.md state the invariant and the budget.

Why

Background refreshes shared the per-upstream limiter with inference. A process with many hybrid models on one upstream refreshes each identity about every 20 seconds, so the refreshes alone could spend the upstream's whole rate cap and refuse inference calls.

Ceiling

With background refreshes capped at a tenth of the rate cap, a 60 rpm upstream sustains about two admitted hybrid encode/score models per process. An identity lasts 30 seconds and is refreshed after about 20, so each model needs about three of the six reads a minute. Beyond that, admissions lapse and fail closed: requests stay local. Reads that a caller waits for, including those at startup, do not use this budget.

Possible follow-up

One read of an upstream's model list could refresh every model on that upstream. On a single-node server, GET /v1/models (list_models in api/models.py) returns for each model the same revision and profiles.<name>.identity that the single-model read takes from GET /v1/models/{model}, both built by _profile_info, and SIEClient.list_models() returns those entries unchanged. A gateway's /v1/models and /v1/models/{model} both carry revision but no per-profile identity. The list read would need a larger bound than the 64 KiB metadata limit.

Tests and validation

  • test_sie_identity.py, end to end through the identity cache and a mock transport:
    • test_background_identity_refreshes_never_consume_the_inference_rate_cap: 50 background refreshes leave an inference budget that never refills whole.
    • test_identity_reads_a_check_waits_for_count_against_the_inference_rate_cap: validation reads go through while the identity limiter's slot is held, and each takes one request from the inference budget.
    • test_a_server_with_more_hybrid_models_on_one_upstream_than_identity_reads_a_minute_starts: three hybrid models on an upstream whose identity budget is one read a minute all load through the registry's routing validation.
    • test_a_background_refresh_over_the_identity_budget_is_never_sent_and_backs_off_like_a_failed_read and test_a_background_refresh_while_another_identity_read_is_in_flight_is_never_sent_and_backs_off: the transport records no request for the refused refresh or during its back-off.
    • test_inference_calls_never_spend_the_identity_budget.
  • test_remote_upstream_limits.py: test_identity_reads_have_one_limiter_per_upstream_apart_from_the_inference_limiter, test_identity_reads_have_a_tenth_of_the_rate_cap_and_at_least_one_a_minute_of_their_own (600, 60, 19 and 5 rpm), test_inference_calls_never_take_the_identity_budget_or_its_one_slot, test_identity_reads_and_inference_calls_have_separate_breakers and test_a_limiter_without_telemetry_reports_no_refusal_or_breaker_change.
  • Identity and limiter test files: 116 passed. mise run test -- -k "remote or upstream or identity or hybrid or limit" packages/sie_server/tests: 1724 passed. mise run test -- packages/sie_server/tests/config: 1266 passed and 500 skipped; its 4 failures, in test_serving_artifacts.py, come from macOS rename and permission behaviour in code this change does not touch, and pass in CI.
  • mise run lint and mise run typecheck pass, also after merging main.
  • Mutation checks: each of 18 single-point mutations made at least one of these tests fail. They covered background refreshes on the inference limiter, waited reads on the identity limiter, the two paths swapped, the read ignoring the chosen limiter either way, a fifth instead of a tenth, no floor, a floor of two, the upstream's concurrency, telemetry on, each telemetry guard, a shared registry, no restart on reinstall, and a refused read propagating or completing as a refusal.

Summary by CodeRabbit

  • New Features
    • SIE identity metadata refreshes now use a separate per-upstream budget, independent of inference requests, with one concurrent read and a rate based on the inference limit.
    • Background refreshes begin before observations expire. If refresh capacity is unavailable, admissions may lapse so requests stay local.
    • When a read is limited or fails, the existing observation remains available until expiry and another read is delayed.
  • Documentation
    • Updated the remote backend guide with identity refresh limits and how reads during configuration changes use inference limits.

Identity metadata reads for hybrid SIE admission went through the same
per-upstream limiter as inference calls. A process with many hybrid
models on one upstream refreshes each identity about every 20 seconds,
so the refreshes alone could spend the upstream's whole rate cap.

Identity reads now go through a second limiter per upstream: a tenth of
rate_cap.requests_per_minute, at least one read a minute, one read in
flight, and a circuit breaker of its own. Inference calls never draw on
it. A read it refuses is never sent and backs off like a failed read.
The identity limiter reports nothing to the worker telemetry, whose
upstream instruments keep describing inference calls.
@krisztian-gajdar
krisztian-gajdar requested a review from a team as a code owner October 7, 2026 13:53
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 3d342f24-10d4-42d3-92dd-5fa4d50a514e
📥 Commits

Reviewing files that changed from the base of the PR and between bb17e19 and 9759a72.

📒 Files selected for processing (5)
  • packages/sie_server/REMOTE_BACKENDS.md
  • packages/sie_server/src/sie_server/adapters/remote/_limits.py
  • packages/sie_server/src/sie_server/config/sie_identity.py
  • packages/sie_server/tests/adapters/test_remote_upstream_limits.py
  • packages/sie_server/tests/config/test_sie_identity.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/sie_server/REMOTE_BACKENDS.md

Included review availability: This review used your included allowance. 6 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.


📝 Walkthrough

Walkthrough

SIE identity metadata reads now use a separate per-upstream limiter with a reduced rate cap, one concurrent read, and independent breaker state. Background refreshes use this limiter; synchronous reads use the inference limiter. Documentation and tests cover the limits and refusal behavior.

Changes

SIE identity metadata limits

Layer / File(s) Summary
Build and verify the identity limiter
packages/sie_server/src/sie_server/adapters/remote/_limits.py, packages/sie_server/tests/adapters/test_remote_upstream_limits.py
Identity reads use a separate limiter with one tenth of the inference rate cap, a minimum of one request per minute, and one in-flight read. It has independent breaker state and does not report refusal or breaker telemetry. Tests cover limiter caching, limits, and telemetry behavior.
Use identity limits during refresh
packages/sie_server/src/sie_server/config/sie_identity.py, packages/sie_server/tests/config/test_sie_identity.py, packages/sie_server/REMOTE_BACKENDS.md
Background identity refreshes use the identity limiter; synchronous reads use the inference limiter. Refused reads retain the previous observation until expiry and delay the next read for the refusal age. Tests and documentation cover the separate budgets and refresh behavior.

Sequence Diagram(s)

sequenceDiagram
  participant IdentityRefresh
  participant IdentityLimiter
  participant Upstream
  IdentityRefresh->>IdentityLimiter: Request admission for background metadata read
  IdentityLimiter->>Upstream: Send admitted metadata read
Loading

Suggested reviewers: dragosboca

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 9759a

Background identity refreshes now use their own small budget, so they no longer consume inference capacity. At low configured rates, hybrid admission may lapse. In that case, requests are served locally, as the remote-backend guide documents. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.63% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 4 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: background identity refreshes now use a separate budget. It matches the pull request objectives and changeset.
Full details: Docstring Coverage

Explanation

Docstring coverage is 15.63% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 7, 2026
Background identity refreshes never consume the inference rate cap:
only they draw on the per-upstream identity budget. An identity read
that its caller waits for, at configuration load, at hot reload and
before a single-node bridge, goes through the upstream's inference
limiter again, so a server starts with more hybrid models on one
upstream than the identity budget allows reads a minute.
_identity_observation passes the limiter for its path into _refresh
and _read_identity.
@krisztian-gajdar krisztian-gajdar changed the title fix(server): give upstream identity reads their own budget fix(server): give background identity refreshes their own budget Oct 7, 2026
@krisztian-gajdar
krisztian-gajdar merged commit c998453 into main Oct 7, 2026
21 checks passed
@krisztian-gajdar
krisztian-gajdar deleted the fix/identity-read-budget branch October 7, 2026 14:43
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