-
Notifications
You must be signed in to change notification settings - Fork 7
Share concurrent request limits across the fleet #975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
5cff7e2
07a09c3
389ff68
76ae48a
f783811
6eca5fd
dcb326d
272e2ea
a8d35b9
506dd41
bfe5512
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -131,6 +131,7 @@ pub fn test_config() -> ApiConfig { | |||||||||||||
| ..config::UsageReportingConfig::default() | ||||||||||||||
| }, | ||||||||||||||
| ita: config::ItaAttestationConfig::default(), | ||||||||||||||
| fleet_concurrency: config::FleetConcurrencyConfig::default(), | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -434,6 +435,36 @@ where | |||||||||||||
| (server, database) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Independent servers sharing one database, standing in for fleet replicas. | ||||||||||||||
| /// Each gets its own services, so agreement has to travel through the database. | ||||||||||||||
| pub async fn setup_test_fleet<F>( | ||||||||||||||
| instances: usize, | ||||||||||||||
| mutate: F, | ||||||||||||||
| ) -> ( | ||||||||||||||
| Vec<axum_test::TestServer>, | ||||||||||||||
| Vec<Arc<inference_providers::mock::MockProvider>>, | ||||||||||||||
| Arc<Database>, | ||||||||||||||
| ) | ||||||||||||||
| where | ||||||||||||||
| F: Fn(&mut config::ApiConfig), | ||||||||||||||
| { | ||||||||||||||
|
Comment on lines
+454
to
+456
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
Suggestion:
Suggested change
|
||||||||||||||
| let infra = setup_test_infrastructure().await; | ||||||||||||||
| let database = infra.database.clone(); | ||||||||||||||
|
|
||||||||||||||
| let mut servers = Vec::with_capacity(instances); | ||||||||||||||
| let mut mocks = Vec::with_capacity(instances); | ||||||||||||||
| for _ in 0..instances { | ||||||||||||||
| let mut config = infra.config.clone(); | ||||||||||||||
| mutate(&mut config); | ||||||||||||||
| let (server, _pool, mock, _router) = | ||||||||||||||
| build_test_server_components(database.clone(), config).await; | ||||||||||||||
| servers.push(server); | ||||||||||||||
| mocks.push(mock); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| (servers, mocks, database) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pub async fn setup_test_server_with_database() -> (axum_test::TestServer, Arc<Database>) { | ||||||||||||||
| let (server, _, _, database) = setup_test_server_with_pool().await; | ||||||||||||||
| (server, database) | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| use crate::common; | ||
| use inference_providers::mock::ResponseTemplate; | ||
| use std::time::Duration; | ||
| use uuid::Uuid; | ||
|
|
||
| const INSTANCES: usize = 4; | ||
| const LIMIT: i32 = 2; | ||
| const ATTEMPTS: usize = 8; | ||
| const HOLD: Duration = Duration::from_secs(2); | ||
| const MODEL: &str = "Qwen/Qwen3-30B-A3B-Instruct-2507"; | ||
|
|
||
| async fn set_rate_limit(database: &database::Database, organization_id: &str, limit: i32) { | ||
| let organization_id = Uuid::parse_str(organization_id).expect("organization id is a uuid"); | ||
| database | ||
| .pool() | ||
| .get() | ||
| .await | ||
| .expect("database connection") | ||
| .execute( | ||
| "UPDATE organizations SET rate_limit = $1 WHERE id = $2", | ||
| &[&limit, &organization_id], | ||
| ) | ||
| .await | ||
| .expect("rate limit update"); | ||
| } | ||
|
|
||
| async fn live_lease_count(database: &database::Database, organization_id: &str) -> i64 { | ||
| let organization_id = Uuid::parse_str(organization_id).expect("organization id is a uuid"); | ||
| database | ||
| .pool() | ||
| .get() | ||
| .await | ||
| .expect("database connection") | ||
| .query_one( | ||
| "SELECT COUNT(*) FROM concurrency_leases | ||
| WHERE organization_id = $1 AND expires_at > NOW()", | ||
| &[&organization_id], | ||
| ) | ||
| .await | ||
| .expect("lease count") | ||
| .get(0) | ||
| } | ||
|
|
||
| /// Four replicas sharing one database must not admit more than the | ||
| /// organization's limit between them. Without fleet-wide accounting each | ||
| /// replica counts only itself, so this admits INSTANCES * LIMIT. | ||
| #[tokio::test] | ||
| async fn fleet_limit_is_shared_across_replicas() { | ||
| let (servers, mocks, database) = common::setup_test_fleet(INSTANCES, |config| { | ||
| config.fleet_concurrency.mode = config::FleetConcurrencyMode::Enforce; | ||
| }) | ||
| .await; | ||
|
|
||
| for mock in &mocks { | ||
| mock.set_default_response(ResponseTemplate::new("held").with_hold(HOLD)) | ||
| .await; | ||
| } | ||
|
|
||
| let org = common::setup_org_with_credits(&servers[0], 100_000_000_000).await; | ||
| let api_key = common::get_api_key_for_org(&servers[0], org.id.clone()).await; | ||
| set_rate_limit(&database, &org.id, LIMIT).await; | ||
|
|
||
| let attempts = (0..ATTEMPTS).map(|attempt| { | ||
| let server = &servers[attempt % INSTANCES]; | ||
| let api_key = api_key.clone(); | ||
| async move { | ||
| server | ||
| .post("/v1/chat/completions") | ||
| .add_header("Authorization", format!("Bearer {api_key}")) | ||
| .json(&serde_json::json!({ | ||
| "model": MODEL, | ||
| "messages": [{ "role": "user", "content": "hello" }] | ||
| })) | ||
| .await | ||
| .status_code() | ||
| .as_u16() | ||
| } | ||
| }); | ||
|
|
||
| let statuses = futures::future::join_all(attempts).await; | ||
| let admitted = statuses.iter().filter(|status| **status == 200).count(); | ||
| let rejected = statuses.iter().filter(|status| **status == 429).count(); | ||
|
|
||
| assert_eq!( | ||
| admitted, LIMIT as usize, | ||
| "expected the fleet to admit exactly {LIMIT} of {ATTEMPTS}, got {statuses:?}" | ||
| ); | ||
| assert_eq!( | ||
| rejected, | ||
| ATTEMPTS - LIMIT as usize, | ||
| "every attempt over the limit should be rejected, got {statuses:?}" | ||
| ); | ||
| } | ||
|
|
||
| /// A finished request must give its slot back, or the organization stays | ||
| /// locked out until the leases expire. | ||
| #[tokio::test] | ||
| async fn completed_requests_release_their_lease() { | ||
| let (servers, mocks, database) = common::setup_test_fleet(1, |config| { | ||
| config.fleet_concurrency.mode = config::FleetConcurrencyMode::Enforce; | ||
| }) | ||
| .await; | ||
|
|
||
| mocks[0] | ||
| .set_default_response(ResponseTemplate::new("done")) | ||
| .await; | ||
|
|
||
| let org = common::setup_org_with_credits(&servers[0], 100_000_000_000).await; | ||
| let api_key = common::get_api_key_for_org(&servers[0], org.id.clone()).await; | ||
| set_rate_limit(&database, &org.id, LIMIT).await; | ||
|
|
||
| for _ in 0..(ATTEMPTS as i32 + LIMIT) { | ||
| let status = servers[0] | ||
| .post("/v1/chat/completions") | ||
| .add_header("Authorization", format!("Bearer {api_key}")) | ||
| .json(&serde_json::json!({ | ||
| "model": MODEL, | ||
| "messages": [{ "role": "user", "content": "hello" }] | ||
| })) | ||
| .await | ||
| .status_code(); | ||
| assert_eq!( | ||
| status, 200, | ||
| "a released slot should let the next request straight through" | ||
| ); | ||
| } | ||
|
|
||
| tokio::time::sleep(Duration::from_millis(500)).await; | ||
| assert_eq!( | ||
| live_lease_count(&database, &org.id).await, | ||
| 0, | ||
| "leases outlived the requests that took them" | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,73 @@ pub struct ApiConfig { | |||||||||||||||||||||||||||||||||||||
| pub aml: AmlConfig, | ||||||||||||||||||||||||||||||||||||||
| pub usage_reporting: UsageReportingConfig, | ||||||||||||||||||||||||||||||||||||||
| pub ita: ItaAttestationConfig, | ||||||||||||||||||||||||||||||||||||||
| pub fleet_concurrency: FleetConcurrencyConfig, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /// How concurrent-request limits are enforced across replicas. | ||||||||||||||||||||||||||||||||||||||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] | ||||||||||||||||||||||||||||||||||||||
| pub enum FleetConcurrencyMode { | ||||||||||||||||||||||||||||||||||||||
| /// Each replica counts only its own in-flight requests. | ||||||||||||||||||||||||||||||||||||||
| #[default] | ||||||||||||||||||||||||||||||||||||||
| Off, | ||||||||||||||||||||||||||||||||||||||
| /// Leases are counted fleet-wide but rejection stays per-replica. | ||||||||||||||||||||||||||||||||||||||
| Shadow, | ||||||||||||||||||||||||||||||||||||||
| /// Admission is decided by the fleet-wide lease count. | ||||||||||||||||||||||||||||||||||||||
| Enforce, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| #[derive(Debug, Clone)] | ||||||||||||||||||||||||||||||||||||||
| pub struct FleetConcurrencyConfig { | ||||||||||||||||||||||||||||||||||||||
| pub mode: FleetConcurrencyMode, | ||||||||||||||||||||||||||||||||||||||
| pub lease_ttl_seconds: u64, | ||||||||||||||||||||||||||||||||||||||
| pub instance_id: String, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| impl Default for FleetConcurrencyConfig { | ||||||||||||||||||||||||||||||||||||||
| fn default() -> Self { | ||||||||||||||||||||||||||||||||||||||
| Self { | ||||||||||||||||||||||||||||||||||||||
| mode: FleetConcurrencyMode::Off, | ||||||||||||||||||||||||||||||||||||||
| lease_ttl_seconds: 60, | ||||||||||||||||||||||||||||||||||||||
| instance_id: String::new(), | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| impl FleetConcurrencyConfig { | ||||||||||||||||||||||||||||||||||||||
| pub fn from_env() -> Self { | ||||||||||||||||||||||||||||||||||||||
| let requested = env::var("FLEET_CONCURRENCY_MODE").unwrap_or_default(); | ||||||||||||||||||||||||||||||||||||||
| let mode = match requested.to_ascii_lowercase().as_str() { | ||||||||||||||||||||||||||||||||||||||
| "enforce" => FleetConcurrencyMode::Enforce, | ||||||||||||||||||||||||||||||||||||||
| "shadow" => FleetConcurrencyMode::Shadow, | ||||||||||||||||||||||||||||||||||||||
| "" | "off" => FleetConcurrencyMode::Off, | ||||||||||||||||||||||||||||||||||||||
| _ => { | ||||||||||||||||||||||||||||||||||||||
| // eprintln for the reason given on the CHUTES_MODELS warning | ||||||||||||||||||||||||||||||||||||||
| // below. Silence here reads as enforcement being on when it is | ||||||||||||||||||||||||||||||||||||||
| // not. | ||||||||||||||||||||||||||||||||||||||
| eprintln!( | ||||||||||||||||||||||||||||||||||||||
| "WARN: unrecognised FLEET_CONCURRENCY_MODE '{requested}', \ | ||||||||||||||||||||||||||||||||||||||
| falling back to off" | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| FleetConcurrencyMode::Off | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Self { | ||||||||||||||||||||||||||||||||||||||
| mode, | ||||||||||||||||||||||||||||||||||||||
| lease_ttl_seconds: env::var("FLEET_CONCURRENCY_LEASE_TTL_SECONDS") | ||||||||||||||||||||||||||||||||||||||
| .ok() | ||||||||||||||||||||||||||||||||||||||
| .and_then(|value| value.parse::<u64>().ok()) | ||||||||||||||||||||||||||||||||||||||
| .filter(|seconds| *seconds > 0) | ||||||||||||||||||||||||||||||||||||||
| .unwrap_or(60), | ||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unlike Suggestion:
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| instance_id: env::var("FLEET_CONCURRENCY_INSTANCE_ID") | ||||||||||||||||||||||||||||||||||||||
| .ok() | ||||||||||||||||||||||||||||||||||||||
| .filter(|value| !value.is_empty()) | ||||||||||||||||||||||||||||||||||||||
| .unwrap_or_else(|| { | ||||||||||||||||||||||||||||||||||||||
| let host = env::var("HOSTNAME").unwrap_or_else(|_| "unknown".to_string()); | ||||||||||||||||||||||||||||||||||||||
| format!("{host}-{}", std::process::id()) | ||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In containerized environments (Kubernetes, Docker), Suggestion:
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| impl ApiConfig { | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -60,6 +127,7 @@ impl ApiConfig { | |||||||||||||||||||||||||||||||||||||
| infra: InfraConfig::from_env(), | ||||||||||||||||||||||||||||||||||||||
| aml: AmlConfig::from_env()?, | ||||||||||||||||||||||||||||||||||||||
| ita: ItaAttestationConfig::from_env()?, | ||||||||||||||||||||||||||||||||||||||
| fleet_concurrency: FleetConcurrencyConfig::from_env(), | ||||||||||||||||||||||||||||||||||||||
| usage_reporting: UsageReportingConfig::from_env()?, | ||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| CREATE TABLE concurrency_leases ( | ||
| id UUID PRIMARY KEY, | ||
| organization_id UUID NOT NULL REFERENCES organizations(id) ON DELETE CASCADE, | ||
| model_id UUID NOT NULL REFERENCES models(id) ON DELETE CASCADE, | ||
| instance_id TEXT NOT NULL, | ||
| acquired_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | ||
| expires_at TIMESTAMPTZ NOT NULL | ||
| ); | ||
|
|
||
| CREATE INDEX idx_concurrency_leases_org_model | ||
| ON concurrency_leases (organization_id, model_id, expires_at); | ||
|
|
||
| CREATE INDEX idx_concurrency_leases_expires_at | ||
| ON concurrency_leases (expires_at); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Medium · Do not use a single shadow replica as fleet-wide rollout evidence
Instances left in
offmode never createFleetConcurrency, so they continue using only local counters and write no leases. Consequently, the documented one-instance shadow rollout measures only that instance, and enabling enforce one instance at a time still ignores traffic on off replicas. Require all replicas to write shadow leases before relying on the metric or performing a gradual enforce rollout.