feat(function-autoscaler): add LLM gateway scaling - #727
Conversation
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe autoscaler now discovers LLM Gateway metrics, selects metric sources through a shared routing cache, chooses gateway targets, and calculates source-specific scaling inputs and desired instance changes. ChangesLLM Gateway autoscaling
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to This change adds LLM gateway metrics as an autoscaling fallback and preserves the existing worker and control-plane paths; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Server
participant Autoscaler as run_autoscaling_logic_p0
participant Gathering as scaling-input gathering
participant Cache as MetricRoutingCache
participant Gateway as gateway target selection
Server->>Cache: create shared routing cache
Server->>Autoscaler: pass routing cache
Autoscaler->>Gathering: gather scaling inputs
Gathering->>Cache: read or store routing decision
Gathering->>Gateway: select gateway version and target
Gateway-->>Gathering: return target and instance state
Gathering-->>Autoscaler: return inputs and selected target
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🛡️ CodeQL Analysis🚨 Found 2 issue(s) Severity Breakdown:
📋 Top Issues🔗 View full details in Security tab 🕐 Last updated: 2026-08-08 01:04:34 UTC | Commit: 772185c |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs (2)
57-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a TTL to
gateway_targets, and consider moving this cache into the scaling module.Two points:
sourcesexpires after one hour.gateway_targetshas only a capacity bound, so a version pin can live for the whole process lifetime.select_gateway_targetdoes drop a stale pin when the pinned version disappears fromnvcf_function_infoor becomes idle while another version is active. A pin can still persist for days when all versions stay idle. A TTL makes the stickiness window explicit and bounded.- The coding guidelines place policy caches and stickiness behavior in the scaling module.
gateway_targetsis a stickiness cache, andselect_gateway_targetis stickiness logic.♻️ Proposed TTL change
pub fn new_metric_routing_cache() -> MetricRoutingCache { let ttl = StdDuration::from_secs(60 * 60); MetricRoutingCache { sources: Cache::builder().time_to_live(ttl).build(), - gateway_targets: Cache::new(10_000), + gateway_targets: Cache::builder() + .max_capacity(10_000) + .time_to_live(ttl) + .build(), } }As per coding guidelines: "Keep scaling logic, policy clients and caches, thresholds, and stickiness behavior within the scaling module."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs` around lines 57 - 69, Update new_metric_routing_cache to configure gateway_targets with the same one-hour time-to-live as sources while retaining its existing capacity bound. Move MetricRoutingCache and the related select_gateway_target stickiness logic into the scaling module, preserving their current behavior and interfaces.Source: Coding guidelines
419-438: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse the request counter for activity, and guard a zero-minute lookback.
Two points:
- This query measures activity with
increase(..._duration_seconds_sum[...]). It reports idle when the gateway records a zero duration for every request in the window.llm_api_gateway_http_requests_totalis already used byllm_gateway_metrics_present, and it is the direct activity signal.recently_invokeddrives scale-to-zero, so a false negative terminates a live deployment.lookback_minutescomes fromscale_to_zero_idle_timeout.as_secs() as i64 / 60at Line 613. If the timeout is configured below 60 seconds, the value is0and the selector becomes[0m], which PromQL rejects. The error then propagates through?and fails the whole gather for the function.♻️ Proposed change
let end_time = Utc::now(); + let lookback_minutes = lookback_minutes.max(1); let query = format!( - r#"sum by(function_id) (increase(llm_api_gateway_http_request_duration_seconds_sum{{function_id="{}"}}[{}m])) > 0"#, + r#"sum by(function_id) (increase(llm_api_gateway_http_requests_total{{function_id="{}"}}[{}m])) > 0"#, function_id, lookback_minutes );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs` around lines 419 - 438, Update llm_gateway_recently_invoked to query llm_api_gateway_http_requests_total instead of the duration sum, preserving the existing activity check for scale-to-zero decisions. Normalize a zero-minute lookback to a valid positive PromQL range before constructing the query, so sub-minute idle timeouts do not produce an invalid [0m] selector or propagate a query error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs`:
- Around line 343-351: The gateway scaling flow must prevent idle versions from
retaining surplus instances and must not reduce the selected version to zero
when shared capacity is redistributed. Update gateway_target_desired_instances
and the surrounding Line 812 per-version handling so non-selected versions
receive scale-down requests, while the selected target retains at least the
count required by the shared desired-total decision; revise the test covering
the current saturating-subtraction floor accordingly.
- Around line 353-417: Scope all gateway metrics to the current environment:
update get_gateway_target in
src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs
(lines 353-417) to accept env and ignore_env and apply the appropriate matcher
to both nvcf_function_instances_current and nvcf_function_info; align the
LlmGateway numerator and denominator in the same file (lines 115-141), or
document at both sites if either metric lacks an environment label. Update
discovery in
src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs
(lines 188-199) to reuse get_timeseries_db_query’s aws_env matcher while
honoring ignore_env, and adjust the assertion at line 995 accordingly.
- Around line 115-141: Update the MetricSource::LlmGateway query in the metric
query construction to divide the in-flight request rate by both current
instances and nvcf_function_concurrency, matching the ControlPlane utilization
formula. Preserve the existing function_id grouping and zero-safe denominator
behavior so the result remains a percentage calibrated for decide_scaling.
- Around line 502-637: The TimeseriesDb helper errors are not currently recorded
in the request span. Update the existing tracing instrumentation on the shared
request execution function reached via query_range to include the returned error
field using err, preserving the existing helper behavior and span coverage.
---
Nitpick comments:
In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs`:
- Around line 57-69: Update new_metric_routing_cache to configure
gateway_targets with the same one-hour time-to-live as sources while retaining
its existing capacity bound. Move MetricRoutingCache and the related
select_gateway_target stickiness logic into the scaling module, preserving their
current behavior and interfaces.
- Around line 419-438: Update llm_gateway_recently_invoked to query
llm_api_gateway_http_requests_total instead of the duration sum, preserving the
existing activity check for scale-to-zero decisions. Normalize a zero-minute
lookback to a valid positive PromQL range before constructing the query, so
sub-minute idle timeouts do not produce an invalid [0m] selector or propagate a
query error.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 790bb3fa-c58a-4336-af88-301c8ccb43b8
📒 Files selected for processing (4)
src/control-plane-services/function-autoscaler/crates/server/src/scaling/mod.rssrc/control-plane-services/function-autoscaler/crates/server/src/server.rssrc/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rssrc/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs
| fn gateway_target_desired_instances( | ||
| desired_total: usize, | ||
| total_current: usize, | ||
| target_current: usize, | ||
| ) -> usize { | ||
| target_current | ||
| .saturating_add(desired_total) | ||
| .saturating_sub(total_current) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The gateway delta model cannot scale down idle versions, and it can zero the selected version.
gateway_target_desired_instances converts a total desired count into a per-version count with target_current + desired_total - total_current, floored at 0 by saturating_sub. Two failure modes follow, because Line 812 returns early for every version that is not the selected target:
- Non-selected versions never receive a scaling request. Their instances stay allocated indefinitely.
total_currenttherefore stays high, and the delta stays small. - When
total_current - target_current >= desired_total, the target gets 0. Example:total_current = 10split as target 4 and an idle version 6, withdesired_total = 5. The result is 0 for the target, while the idle version keeps 6. The active version drops to zero and the surplus stays.
The test at Line 1147 pins this floor behavior, so the intent is unclear. Either issue scale-down requests for non-selected versions, or clamp the target to at least the count that the shared decision requires. Do you want me to open an issue to track the multi-version scale-down path?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs`
around lines 343 - 351, The gateway scaling flow must prevent idle versions from
retaining surplus instances and must not reduce the selected version to zero
when shared capacity is redistributed. Update gateway_target_desired_instances
and the surrounding Line 812 per-version handling so non-selected versions
receive scale-down requests, while the selected target retains at least the
count required by the shared desired-total decision; revise the test covering
the current saturating-subtraction floor accordingly.
There was a problem hiding this comment.
This is intentional. We apply the function-level delta to one version at a time; the next cycle reselects the active version and converges without concurrent writes.
There was a problem hiding this comment.
@borao, the current implementation does not reselect another active version on the next cycle.
MetricRoutingCache.gateway_targets pins the selected version. select_gateway_target keeps that pinned version while it has active instances. The per-version early return then prevents all other versions from receiving a scale request.
In the total_current = 10, target = 4, and desired total = 5 case, the target becomes 0. The idle version remains at 6. Subsequent cycles do not converge because the target remains pinned until it reaches zero, and no request reduces the idle version.
The finding remains valid for the current target-selection and cache behavior.
🧠 Learnings used
Learnt from: CR
Repo: NVIDIA/nvcf PR: 0
File: src/control-plane-services/function-autoscaler/AGENTS.md:0-0
Timestamp: 2026-07-21T18:21:56.834Z
Learning: Applies to src/control-plane-services/function-autoscaler/crates/server/src/scaling/**/*.rs : Keep scaling logic, policy clients and caches, thresholds, and stickiness behavior within the scaling module.
Learnt from: CR
Repo: NVIDIA/nvcf PR: 0
File: src/control-plane-services/function-autoscaler/AGENTS.md:0-0
Timestamp: 2026-07-21T18:21:56.834Z
Learning: Applies to src/control-plane-services/function-autoscaler/**/*.rs : Implement the service as a scaling loop that queries utilization metrics, applies per-function static or custom gRPC scaling policies, and computes the desired instance count.
You are interacting with an AI system.
|
Regarding the routing-cache TTL/module-placement nit: leaving this unchanged. Pins are capacity-bounded and revalidated on every lookup; retaining idle pins provides stable version selection. Moving the code can be a separate refactor. |
1 participant
TL;DR
Add LLM API gateway metrics as an autoscaling source between worker metrics and the existing control-plane fallback.
The autoscaler now prefers worker metrics, then LLM gateway metrics, and finally control-plane metrics. The selected source is cached for one hour.
Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
nvcf_function_info.For the Reviewer
Please focus on:
work/mod.rs;work/discovery.rs;For QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
Validated with:
cargo fmt -p rs-autoscaler -- --checkcargo check -p rs-autoscaler --all-targetscargo clippy -p rs-autoscaler --all-targets -- -D warningscargo test -p rs-autoscalerIssues
NO-REF
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests