Skip to content

Commit f6ecd97

Browse files
authored
[scheduler] Fix race condition in multi-scheduler environments. (AcademySoftwareFoundation#2191)
When multiple instances or async tasks are updating the same host, there was a race condition that would cause over booking errors instead of gracefully treating the condition that caused the race. This PR is intended to reduce the number database errors like the following: ``` WARN cue_scheduler::pipeline::dispatcher::actor: Wasn't able to dispatch all frames: FailedToUpdateResources( × (57b07e8d-6813-4fc5-8f71-132e968f4e29) Failed to update host resources ├─▶ error returned from database: unable to allocate additional memory ╰─▶ unable to allocate additional memory ) ``` ### AI Disclosure Claude Code Opus was used to brainstorm the root cause of this problem and also to review the changes before pushing the PR.
1 parent 66783c4 commit f6ecd97

6 files changed

Lines changed: 175 additions & 46 deletions

File tree

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,7 @@ CLAUDE.md
4646
docs/nav_order_index.txt
4747
rust/.cargo/config.toml
4848
sandbox/pgadmin-data/*
49+
.claude/settings.json
50+
plans/*
51+
.entire/.gitignore
52+
.entire/settings.json

rust/crates/scheduler/src/dao/host_dao.rs

Lines changed: 74 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use chrono::{DateTime, Utc};
1717
use miette::{Context, IntoDiagnostic, Result};
1818
use opencue_proto::host::ThreadMode;
1919
use sqlx::{Pool, Postgres, Transaction};
20+
use thiserror::Error;
2021
use tracing::trace;
2122
use uuid::Uuid;
2223

@@ -27,6 +28,45 @@ use crate::{
2728
pgpool::connection_pool,
2829
};
2930

31+
#[derive(Debug, Error)]
32+
pub enum HostDaoError {
33+
#[error("Host resources exhausted (likely booked by another scheduler)")]
34+
HostResourcesExhausted,
35+
36+
#[error("Resource limit exceeded: {message}")]
37+
ResourceLimitExceeded { message: String },
38+
39+
#[error("{context}: {source}")]
40+
DbFailure {
41+
context: &'static str,
42+
source: sqlx::Error,
43+
},
44+
}
45+
46+
/// Known PostgreSQL trigger messages that indicate expected resource limit enforcement.
47+
const RESOURCE_LIMIT_MESSAGES: &[&str] = &[
48+
"job has exceeded max cores",
49+
"job has exceeded max GPU units",
50+
"subscription has exceeded burst size",
51+
"unable to allocate additional core units",
52+
"unable to allocate additional memory",
53+
"unable to allocate additional GPU units",
54+
"unable to allocate additional GPU memory",
55+
];
56+
57+
fn check_resource_limit_error(err: sqlx::Error, context: &'static str) -> HostDaoError {
58+
let msg = err.to_string();
59+
match RESOURCE_LIMIT_MESSAGES.iter().find(|m| msg.contains(*m)) {
60+
Some(m) => HostDaoError::ResourceLimitExceeded {
61+
message: m.to_string(),
62+
},
63+
None => HostDaoError::DbFailure {
64+
context,
65+
source: err,
66+
},
67+
}
68+
}
69+
3070
/// Data Access Object for host operations in the job dispatch system.
3171
///
3272
/// Manages database operations related to render hosts, including:
@@ -114,16 +154,19 @@ impl From<HostModel> for Host {
114154
// - memory and core fields on table host are only updated when booking procs (update_host_resources)
115155
// - the table host_stat contains memory fields that are updated by cuebot on HostReportHandler
116156
//
117-
// In summary, use host_stat for most up to date memory stats
157+
// We use LEAST(h.int_mem_idle, hs.int_mem_free) to get the most conservative memory estimate.
158+
// h.int_mem_idle reflects bookings (decremented on dispatch), while hs.int_mem_free reflects
159+
// actual OS-reported free memory. Using the minimum avoids overestimating available memory
160+
// when another scheduler (e.g. Cuebot) has booked resources that haven't been consumed yet.
118161
static QUERY_HOST_BY_SHOW_FACILITY_AND_TAG: &str = r#"
119162
SELECT DISTINCT
120163
h.pk_host,
121164
h.str_name,
122165
hs.str_os,
123166
h.int_cores_idle,
124-
hs.int_mem_free,
167+
LEAST(h.int_mem_idle, hs.int_mem_free) as int_mem_free,
125168
h.int_gpus_idle,
126-
hs.int_gpu_mem_free,
169+
LEAST(h.int_gpu_mem_idle, hs.int_gpu_mem_free) as int_gpu_mem_free,
127170
h.int_cores,
128171
hs.int_mem_total,
129172
h.int_thread_mode,
@@ -149,6 +192,10 @@ SET int_cores_idle = int_cores_idle - $1,
149192
int_gpus_idle = int_gpus_idle - $3,
150193
int_gpu_mem_idle = int_gpu_mem_idle - $4
151194
WHERE pk_host = $5
195+
AND int_cores_idle >= $1
196+
AND int_mem_idle >= $2
197+
AND int_gpus_idle >= $3
198+
AND int_gpu_mem_idle >= $4
152199
RETURNING int_cores_idle, int_mem_idle, int_gpus_idle, int_gpu_mem_idle, NOW()
153200
"#;
154201

@@ -324,24 +371,22 @@ impl HostDao {
324371
transaction: &mut Transaction<'_, Postgres>,
325372
host_id: &Uuid,
326373
virtual_proc: &VirtualProc,
327-
dispatch_id: Uuid,
328-
) -> Result<UpdatedHostResources> {
329-
let (cores_idle, mem_idle, gpus_idle, gpu_mem_idle, last_updated): (
330-
i64,
331-
i64,
332-
i64,
333-
i64,
334-
DateTime<Utc>,
335-
) = sqlx::query_as(UPDATE_HOST_RESOURCES)
336-
.bind(virtual_proc.cores_reserved.value())
337-
.bind((virtual_proc.memory_reserved.as_u64() / KB) as i64)
338-
.bind(virtual_proc.gpus_reserved as i32)
339-
.bind(virtual_proc.gpu_memory_reserved.as_u64() as i64)
340-
.bind(host_id.to_string())
341-
.fetch_one(&mut **transaction)
342-
.await
343-
.into_diagnostic()
344-
.wrap_err(format!("({dispatch_id}) Failed to update host resources"))?;
374+
) -> Result<UpdatedHostResources, HostDaoError> {
375+
let row: Option<(i64, i64, i64, i64, DateTime<Utc>)> =
376+
sqlx::query_as(UPDATE_HOST_RESOURCES)
377+
.bind(virtual_proc.cores_reserved.value())
378+
.bind((virtual_proc.memory_reserved.as_u64() / KB) as i64)
379+
.bind(virtual_proc.gpus_reserved as i32)
380+
.bind(virtual_proc.gpu_memory_reserved.as_u64() as i64)
381+
.bind(host_id.to_string())
382+
.fetch_optional(&mut **transaction)
383+
.await
384+
.map_err(|err| {
385+
check_resource_limit_error(err, "Failed to update host resources")
386+
})?;
387+
388+
let (cores_idle, mem_idle, gpus_idle, gpu_mem_idle, last_updated) =
389+
row.ok_or(HostDaoError::HostResourcesExhausted)?;
345390

346391
if CONFIG.host_cache.update_stat_on_book {
347392
sqlx::query(UPDATE_HOST_STAT)
@@ -350,8 +395,7 @@ impl HostDao {
350395
.bind(host_id.to_string())
351396
.execute(&mut **transaction)
352397
.await
353-
.into_diagnostic()
354-
.wrap_err("Failed to update host stat")?;
398+
.map_err(|err| check_resource_limit_error(err, "Failed to update host stat"))?;
355399
}
356400

357401
sqlx::query(UPDATE_SUBSCRIPTION)
@@ -361,35 +405,33 @@ impl HostDao {
361405
.bind(virtual_proc.alloc_id.to_string())
362406
.execute(&mut **transaction)
363407
.await
364-
.into_diagnostic()
365-
.wrap_err("Failed to update subscription resources")?;
408+
.map_err(|err| {
409+
check_resource_limit_error(err, "Failed to update subscription resources")
410+
})?;
366411

367412
sqlx::query(UPDATE_LAYER_RESOURCE)
368413
.bind(virtual_proc.cores_reserved.value())
369414
.bind(virtual_proc.gpus_reserved as i32)
370415
.bind(virtual_proc.layer_id.to_string())
371416
.execute(&mut **transaction)
372417
.await
373-
.into_diagnostic()
374-
.wrap_err("Failed to update layer resources")?;
418+
.map_err(|err| check_resource_limit_error(err, "Failed to update layer resources"))?;
375419

376420
sqlx::query(UPDATE_JOB_RESOURCE)
377421
.bind(virtual_proc.cores_reserved.value())
378422
.bind(virtual_proc.gpus_reserved as i32)
379423
.bind(virtual_proc.job_id.to_string())
380424
.execute(&mut **transaction)
381425
.await
382-
.into_diagnostic()
383-
.wrap_err("Failed to update job resources")?;
426+
.map_err(|err| check_resource_limit_error(err, "Failed to update job resources"))?;
384427

385428
sqlx::query(UPDATE_FOLDER_RESOURCE)
386429
.bind(virtual_proc.cores_reserved.value())
387430
.bind(virtual_proc.gpus_reserved as i32)
388431
.bind(virtual_proc.job_id.to_string())
389432
.execute(&mut **transaction)
390433
.await
391-
.into_diagnostic()
392-
.wrap_err("Failed to update folder resources")?;
434+
.map_err(|err| check_resource_limit_error(err, "Failed to update folder resources"))?;
393435

394436
sqlx::query(UPDATE_POINT)
395437
.bind(virtual_proc.cores_reserved.value())
@@ -398,8 +440,7 @@ impl HostDao {
398440
.bind(virtual_proc.show_id.to_string())
399441
.execute(&mut **transaction)
400442
.await
401-
.into_diagnostic()
402-
.wrap_err("Failed to update point resources")?;
443+
.map_err(|err| check_resource_limit_error(err, "Failed to update point resources"))?;
403444

404445
Ok(UpdatedHostResources {
405446
cores_idle,

rust/crates/scheduler/src/dao/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,5 @@ pub use proc_dao::ProcDao;
2929

3030
pub use allocation_dao::{AllocationName, ShowId};
3131
pub use frame_dao::FrameDaoError;
32+
pub use host_dao::HostDaoError;
3233
pub use host_dao::UpdatedHostResources;

rust/crates/scheduler/src/pipeline/dispatcher/actor.rs

Lines changed: 72 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -347,16 +347,40 @@ impl RqdDispatcherService {
347347
if last_error.is_some() {
348348
break;
349349
}
350-
warn!(
351-
"({dispatch_id}) Failed to start frame {} on Db. {}",
352-
frame_str, err
353-
);
350+
match &err {
351+
DispatchError::FailedToUpdateResources(_)
352+
| DispatchError::FailedToCreateProc { .. } => {
353+
// Resource contention during DB updates is expected in
354+
// multi-scheduler environments.
355+
info!(
356+
"({dispatch_id}) Failed to start frame {} on Db. {}",
357+
frame_str, err
358+
);
359+
}
360+
_ => {
361+
warn!(
362+
"({dispatch_id}) Failed to start frame {} on Db. {}",
363+
frame_str, err
364+
);
365+
}
366+
}
354367
last_error = Some(err);
355368
// IMPORTANT: Do NOT update last_host_version here since the transaction
356369
// rolled back and we didn't actually consume any resources from the database.
357370
// The next iteration should use the same host state as before.
358371
continue;
359372
}
373+
DispatchVirtualProcError::HostResourcesExhausted => {
374+
// Host resources were booked by another scheduler (e.g. Cuebot)
375+
// between cache refresh and dispatch. Break the loop and try another host.
376+
info!(
377+
"({dispatch_id}) Host resources exhausted for frame {} (likely booked by another scheduler)",
378+
frame_str
379+
);
380+
last_error = Some(DispatchError::HostResourcesExhausted(()));
381+
382+
break;
383+
}
360384
DispatchVirtualProcError::FrameCouldNotBeUpdated => {
361385
// The entire transaction is probably compromised, stop working on this layer
362386
info!(
@@ -366,6 +390,13 @@ impl RqdDispatcherService {
366390
non_retrieable_frames.push(frame.id);
367391
break;
368392
}
393+
DispatchVirtualProcError::ResourceLimitExceeded(err) => {
394+
// Resource limit enforced by database trigger (e.g. job max cores,
395+
// subscription burst size). This is expected in normal operation.
396+
info!("({dispatch_id}) {frame_str} {err}");
397+
last_error = Some(err);
398+
break;
399+
}
369400
DispatchVirtualProcError::RqdConnectionFailed { host, error } => {
370401
// An error here means connection with this host is probably broken,
371402
// there's no reason to attempt the next frame
@@ -403,7 +434,18 @@ impl RqdDispatcherService {
403434
}
404435

405436
if let Some(error) = last_error {
406-
warn!("Wasn't able to dispatch all frames: {:?}", error)
437+
match &error {
438+
DispatchError::ResourceLimitExceeded(_)
439+
| DispatchError::AllocationOverBurst(_)
440+
| DispatchError::HostResourcesExhausted(_)
441+
| DispatchError::FailedToUpdateResources(_)
442+
| DispatchError::FailedToCreateProc { .. } => {
443+
info!("Wasn't able to dispatch all frames: {:?}", error)
444+
}
445+
_ => {
446+
warn!("Wasn't able to dispatch all frames: {:?}", error)
447+
}
448+
}
407449
}
408450
Ok((last_host_version, layer))
409451
}
@@ -452,7 +494,7 @@ impl RqdDispatcherService {
452494

453495
// Update database
454496
let updated_resources = self
455-
.update_database_for_dispatch(transaction, &virtual_proc, host.id, dispatch_id)
497+
.update_database_for_dispatch(transaction, &virtual_proc, host.id)
456498
.await?;
457499

458500
// When running on dry_run_mode, just log the outcome
@@ -501,7 +543,6 @@ impl RqdDispatcherService {
501543
/// * `transaction` - Database transaction for atomic updates
502544
/// * `virtual_proc` - The virtual proc being dispatched
503545
/// * `host_id` - ID of the host receiving the dispatch
504-
/// * `dispatch_id` - Unique identifier for this dispatch operation
505546
///
506547
/// # Returns
507548
/// On success, returns the updated host resources from the database.
@@ -511,7 +552,6 @@ impl RqdDispatcherService {
511552
transaction: &mut Transaction<'_, Postgres>,
512553
virtual_proc: &VirtualProc,
513554
host_id: Uuid,
514-
dispatch_id: Uuid,
515555
) -> Result<UpdatedHostResources, DispatchVirtualProcError> {
516556
self.frame_dao
517557
.update_frame_started(transaction, virtual_proc)
@@ -538,12 +578,31 @@ impl RqdDispatcherService {
538578

539579
let updated_resources = self
540580
.host_dao
541-
.update_resources(transaction, &host_id, virtual_proc, dispatch_id)
581+
.update_resources(transaction, &host_id, virtual_proc)
542582
.await
543-
.map_err(|err| {
544-
DispatchVirtualProcError::FailedToStartOnDb(DispatchError::FailedToUpdateResources(
545-
err,
546-
))
583+
.map_err(|err| match err {
584+
crate::dao::HostDaoError::HostResourcesExhausted => {
585+
DispatchVirtualProcError::HostResourcesExhausted
586+
}
587+
crate::dao::HostDaoError::ResourceLimitExceeded { message } => {
588+
DispatchVirtualProcError::ResourceLimitExceeded(
589+
DispatchError::ResourceLimitExceeded(message),
590+
)
591+
}
592+
crate::dao::HostDaoError::DbFailure { context, source } => {
593+
DispatchVirtualProcError::FailedToStartOnDb(
594+
DispatchError::FailedToUpdateResources(
595+
// It would be great to simply use
596+
// `miette::Report::new(source).wrap_err(context)` here, but
597+
// sqlx::Error doesn't implement Diagnostics, therefore needs to be
598+
// wrapped and unwrapped to add context.
599+
Result::<(), _>::Err(source)
600+
.into_diagnostic()
601+
.unwrap_err()
602+
.wrap_err(context),
603+
),
604+
)
605+
}
547606
})?;
548607

549608
Ok(updated_resources)

rust/crates/scheduler/src/pipeline/dispatcher/error.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ pub enum DispatchError {
5151

5252
#[error("DispatchError: Failed to execute command on GRPC interface")]
5353
GrpcFailure(tonic::Status),
54+
55+
#[error("DispatchError: Host Resources exhausted before updating the database")]
56+
HostResourcesExhausted(()),
57+
58+
#[error("DispatchError: Resource limit exceeded: {0}")]
59+
ResourceLimitExceeded(String),
5460
}
5561

5662
#[derive(Debug, Error, Diagnostic)]
@@ -61,9 +67,15 @@ pub enum DispatchVirtualProcError {
6167
#[error("Failed to start frame on database")]
6268
FailedToStartOnDb(DispatchError),
6369

70+
#[error("Host resources exhausted (likely booked by another scheduler)")]
71+
HostResourcesExhausted,
72+
6473
#[error("Failed to lock frame on database")]
6574
FrameCouldNotBeUpdated,
6675

6776
#[error("Failed to connect to RQD on host {host}")]
6877
RqdConnectionFailed { host: String, error: Error },
78+
79+
#[error("Resource limit exceeded")]
80+
ResourceLimitExceeded(DispatchError),
6981
}

rust/crates/scheduler/src/pipeline/matcher.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,18 @@ impl MatchingService {
485485
))
486486
);
487487
}
488+
DispatchError::HostResourcesExhausted(_) => {
489+
info!(
490+
"Host resources exhausted before updating database when dispatching {} on {}.",
491+
layer_display, host
492+
);
493+
}
494+
DispatchError::ResourceLimitExceeded(msg) => {
495+
info!(
496+
"Resource limit exceeded when dispatching {} on {}: {}",
497+
layer_display, host, msg
498+
);
499+
}
488500
}
489501
}
490502
}

0 commit comments

Comments
 (0)