Repository navigation
fix(server): give background identity refreshes their own budget - #618
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 📝 WalkthroughWalkthroughSIE 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. ChangesSIE identity metadata limits
Sequence Diagram(s)sequenceDiagram
participant IdentityRefresh
participant IdentityLimiter
participant Upstream
IdentityRefresh->>IdentityLimiter: Request admission for background metadata read
IdentityLimiter->>Upstream: Send admitted metadata read
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
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.
What changes
identity_limiter. It is anUpstreamLimiterbuilt from the upstream's configuration with a reduced rate cap: a tenth ofrate_cap.requests_per_minute, at least one read a minute, and one read in flight. Its circuit breaker uses the upstream'sbreakersettings but is its own. Inference calls never draw on it._identity_observationpasses the limiter for its path into_refreshand_read_identity.UpstreamLimiter(..., telemetry=False)), so it never overwrites the inference breaker gauge of the same upstream.sie.worker.upstream.refusalsandsie.worker.upstream.breaker.openkeep describing the inference limiter._limits.pydocstring andREMOTE_BACKENDS.mdstate 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_modelsinapi/models.py) returns for each model the samerevisionandprofiles.<name>.identitythat the single-model read takes fromGET /v1/models/{model}, both built by_profile_info, andSIEClient.list_models()returns those entries unchanged. A gateway's/v1/modelsand/v1/models/{model}both carryrevisionbut no per-profileidentity. 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_readandtest_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_breakersandtest_a_limiter_without_telemetry_reports_no_refusal_or_breaker_change.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, intest_serving_artifacts.py, come from macOS rename and permission behaviour in code this change does not touch, and pass in CI.mise run lintandmise run typecheckpass, also after mergingmain.Summary by CodeRabbit