Skip to content
8 changes: 8 additions & 0 deletions model_gateway/src/config/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ pub struct RouterConfig {
/// Enable the priority-aware admission scheduler. When false (default),
/// the legacy concurrency-limit middleware stays wired — zero behavior
/// change for existing deployments.
///
/// Limitation: admission capacity is a single fleet-wide scalar with no
/// model dimension, so multi-model deployments get incorrect admission
/// limits (an idle model's capacity masks a saturated model's queue).
/// SMG logs a one-time warning when the healthy fleet reports more than
/// one model id. Until per-model capacity lands, prefer
/// `--worker-capacity-override` or one gateway per model. See
/// smg-project/smg#2069.
#[serde(default)]
pub priority_scheduler_enabled: bool,
/// Max priority class applied to tenants not listed in the scheduler
Expand Down
168 changes: 165 additions & 3 deletions model_gateway/src/worker/capacity.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
//! Aggregate backend capacity tracking.
//!
//! See `.claude/priority-scheduling/01-worker-capacity-design.md` for the
//! full design rationale.
//! Capacity is a single fleet-wide scalar (summed across every healthy
//! worker, with no model dimension). That is correct only for
//! single-model fleets: with multiple models, an idle model's slots
//! inflate other models' admission budgets (smg-project/smg#2069). The
//! tracker logs a one-time warning when the healthy fleet reports more
//! than one model id.

use std::sync::{
atomic::{AtomicU16, AtomicU8, Ordering},
atomic::{AtomicBool, AtomicU16, AtomicU8, Ordering},
Arc, Weak,
};

Expand Down Expand Up @@ -113,6 +117,8 @@ pub struct WorkerCapacity {
capacity: AtomicU16,
source: AtomicU8,
watch_tx: watch::Sender<u16>,
/// One-shot latch for the multi-model warning (#2069).
warned_multi_model: AtomicBool,
}

impl WorkerCapacity {
Expand Down Expand Up @@ -141,9 +147,33 @@ impl WorkerCapacity {
capacity: AtomicU16::new(capacity),
source: AtomicU8::new(source as u8),
watch_tx: tx,
warned_multi_model: AtomicBool::new(false),
})
}

/// Log once at startup when the healthy fleet serves more than one
/// model id.
///
/// Capacity is a fleet-wide scalar, so multi-model admission limits are
/// incorrect (#2069): an idle model's slots mask a saturated model's
/// queue. Startup-only by design — the check is O(fleet size), which is
/// not acceptable in the worker-event loop.
fn warn_once_if_multi_model(&self, workers: &[Arc<dyn Worker>]) {
if self.warned_multi_model.load(Ordering::Relaxed) {
return;
}
let models = distinct_model_count(workers);
if models > 1 {
self.warned_multi_model.store(true, Ordering::Relaxed);
tracing::warn!(
models,
"priority scheduler capacity is fleet-wide: an idle model's slots \
inflate other models' admission budgets (#2069). Mitigations: \
--worker-capacity-override, or one gateway per model."
);
}
}

/// Construct a `WorkerCapacity`, compute the initial value
/// synchronously, and spawn the supervised event-loop task.
///
Expand All @@ -163,7 +193,9 @@ impl WorkerCapacity {
capacity: AtomicU16::new(initial_capacity),
source: AtomicU8::new(initial_source as u8),
watch_tx,
warned_multi_model: AtomicBool::new(false),
});
this.warn_once_if_multi_model(&workers);

// Supervised loop: any panic in the inner future restarts the task
// after a 1s backoff. Graceful exit (Ok) breaks out.
Expand Down Expand Up @@ -216,6 +248,27 @@ fn healthy_workers(registry: &WorkerRegistry) -> Vec<Arc<dyn Worker>> {
.collect()
}

/// Count distinct advertised model ids across the given workers.
///
/// Every model card counts, not just the primary — a worker serving
/// several models must not read as single-model. Used by the multi-model
/// warning: capacity is a fleet-wide scalar, so a fleet reporting more
/// than one model id gets incorrect per-model admission limits (#2069).
pub(super) fn distinct_model_count(workers: &[Arc<dyn Worker>]) -> usize {
let mut seen = std::collections::HashSet::new();
for w in workers {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this becomes a loop in the hotloop just for the log
this doesnt seem right
it should be O(1) for logging

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b4f5345 — the warning is now startup-only (checked once in WorkerCapacity::spawn); the worker-event loop no longer pays the O(fleet) count. Added a spawn-path latch test to cover it.

let cards = w.models();
if cards.is_empty() {
// No model cards (e.g. wildcard or label-only worker) — fall back
// to the primary id, matching WorkerRegistry::worker_model_ids().
seen.insert(w.model_id().to_string());
} else {
seen.extend(cards.into_iter().map(|card| card.id));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
seen.len()
}

async fn run_event_loop(
tracker: Weak<WorkerCapacity>,
registry: Weak<WorkerRegistry>,
Expand Down Expand Up @@ -619,4 +672,113 @@ mod tests {
let rx = tracker.watch();
assert_eq!(*rx.borrow(), 42);
}

fn worker_with_model(url: &str, model: &str) -> Arc<dyn Worker> {
Arc::new(
BasicWorkerBuilder::new(url)
.model(openai_protocol::model_card::ModelCard::new(model))
.build(),
)
}

#[test]
fn distinct_model_count_counts_unique_models() {
let workers = vec![
worker_with_model("http://w1", "llama-7b"),
worker_with_model("http://w2", "llama-7b"),
worker_with_model("http://w3", "llama-70b"),
];
assert_eq!(distinct_model_count(&workers), 2);
}

#[test]
fn distinct_model_count_single_model_fleet() {
let workers = vec![
worker_with_model("http://w1", "llama-7b"),
worker_with_model("http://w2", "llama-7b"),
];
assert_eq!(distinct_model_count(&workers), 1);
}

#[test]
fn distinct_model_count_counts_every_advertised_model_card() {
// One worker can advertise multiple model cards; counting only the
// primary id would undercount and suppress the warning.
let spec: openai_protocol::worker::WorkerSpec = serde_json::from_value(serde_json::json!({
"url": "http://w1",
"models": [{"id": "llama-7b"}, {"id": "llama-70b"}],
}))
.expect("multi-model spec");
let workers =
vec![Arc::new(BasicWorkerBuilder::from_spec(spec).build()) as Arc<dyn Worker>];
assert_eq!(distinct_model_count(&workers), 2);
}

#[test]
fn distinct_model_count_falls_back_to_primary_model_id_when_no_cards() {
// Label-only workers (no model cards) must still count — matches the
// WorkerRegistry::worker_model_ids() fallback.
let with_label = |url: &str, model: &str| {
let mut labels = HashMap::new();
labels.insert("model_id".to_string(), model.to_string());
Arc::new(BasicWorkerBuilder::new(url).labels(labels).build()) as Arc<dyn Worker>
};
let workers = vec![
with_label("http://w1", "llama-7b"),
with_label("http://w2", "llama-70b"),
];
assert_eq!(distinct_model_count(&workers), 2);
}

#[tokio::test]
async fn spawn_latches_warning_for_multi_model_registry() {
let registry = Arc::new(WorkerRegistry::new());
let id1 = registry
.register(worker_with_model("http://w1", "llama-7b"))
.expect("registered");
registry.transition_status(&id1, WorkerStatus::Ready);
let id2 = registry
.register(worker_with_model("http://w2", "llama-70b"))
.expect("registered");
registry.transition_status(&id2, WorkerStatus::Ready);

let tracker = WorkerCapacity::spawn(registry, CapacityTrackerSettings::default());
assert!(tracker.warned_multi_model.load(Ordering::Relaxed));
}

#[test]
fn warn_once_if_multi_model_sets_flag_once() {
let tracker = WorkerCapacity::for_test_with_value(64, CapacitySource::Mixed);
let workers = vec![
worker_with_model("http://w1", "llama-7b"),
worker_with_model("http://w2", "llama-70b"),
];
assert!(!tracker.warned_multi_model.load(Ordering::Relaxed));
tracker.warn_once_if_multi_model(&workers);
assert!(tracker.warned_multi_model.load(Ordering::Relaxed));
// Second call is a no-op regardless of input.
tracker.warn_once_if_multi_model(&[]);
assert!(tracker.warned_multi_model.load(Ordering::Relaxed));
}

#[tracing_test::traced_test]
#[test]
fn warn_once_if_multi_model_logs_for_multi_model_fleet() {
let tracker = WorkerCapacity::for_test_with_value(64, CapacitySource::Mixed);
let workers = vec![
worker_with_model("http://w1", "llama-7b"),
worker_with_model("http://w2", "llama-70b"),
];
tracker.warn_once_if_multi_model(&workers);
assert!(logs_contain("fleet-wide"));
}

#[tracing_test::traced_test]
#[test]
fn warn_once_if_multi_model_quiet_for_single_model() {
let tracker = WorkerCapacity::for_test_with_value(64, CapacitySource::Mixed);
let workers = vec![worker_with_model("http://w1", "llama-7b")];
tracker.warn_once_if_multi_model(&workers);
assert!(!logs_contain("fleet-wide"));
}
}
Loading