Skip to content

Commit a319bc2

Browse files
authored
more perf stuff again
1 parent b267454 commit a319bc2

4 files changed

Lines changed: 113 additions & 10 deletions

File tree

odorobo/benches/agent_status.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ fn status(vm_count: usize) -> AgentStatus {
1313
ram: ByteSize::gb(256),
1414
used_vcpus: u32::try_from(vm_count).expect("benchmark VM count fits in u32"),
1515
used_ram: ByteSize::gb(vm_count as u64),
16-
vms: (0..vm_count).map(|_| Ulid::generate()).collect(),
16+
vms: {
17+
let mut vms: Vec<_> = (0..vm_count).map(|_| Ulid::generate()).collect();
18+
vms.sort_unstable();
19+
vms
20+
},
1721
metadata: ObjectMetadata::default(),
1822
}
1923
}

odorobo/src/actors/agent_actor.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,11 @@ impl Message<GetAgentStatus> for AgentActor {
360360
hostname: self.config.get_hostname().to_owned(),
361361
vcpus: self.vcpus,
362362
ram: self.memory,
363-
vms: self.vms.keys().copied().collect(),
363+
vms: {
364+
let mut vms: Vec<_> = self.vms.keys().copied().collect();
365+
vms.sort_unstable();
366+
vms
367+
},
364368
used_vcpus,
365369
used_ram,
366370
metadata: self.metadata.clone(),
@@ -379,8 +383,8 @@ impl Message<GetAgentStatus> for AgentActor {
379383
};
380384
}
381385

382-
let mut added = Vec::new();
383-
let mut removed = Vec::new();
386+
let mut added = Vec::with_capacity(self.status_history.len());
387+
let mut removed = Vec::with_capacity(self.status_history.len());
384388
for change in self
385389
.status_history
386390
.iter()

odorobo/src/actors/scheduler_actor.rs

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ pub struct SchedulerActor {
121121
/// this is a vec because a vmid/ulid can be scheduled on multiple boxes simultaneously during migration
122122
pub vm_data_cache: AHashMap<Ulid, Vec<CachedVMActor>>,
123123
pub vm_keepalive_tasks: AHashMap<ActorId, JoinHandle<()>>,
124+
pending_resources_cache: Option<AHashMap<ActorId, (u32, u64)>>,
125+
agent_vm_index: AHashMap<ActorId, AHashSet<Ulid>>,
124126
actor_kinds: AHashMap<ActorId, CachedActorKind>,
125127

126128
pub cache_actor_finder: Option<JoinHandle<()>>,
@@ -188,11 +190,20 @@ impl SchedulerActor {
188190
/// avoiding a temporary hash table on every affinity evaluation.
189191
fn placement_vm_ids(
190192
placements: &AHashMap<Ulid, Vec<VmPlacement>>,
193+
indexed: Option<&AHashSet<Ulid>>,
191194
agent_id: ActorId,
192195
observed: &[Ulid],
193196
) -> Vec<Ulid> {
194-
let mut vmids = Vec::with_capacity(observed.len());
197+
let indexed_len = indexed.map_or(0, |index| index.len());
198+
let mut vmids = Vec::with_capacity(observed.len().max(indexed_len));
195199
vmids.extend_from_slice(observed);
200+
if let Some(indexed) = indexed {
201+
for vmid in indexed {
202+
if !vmids.contains(vmid) {
203+
vmids.push(*vmid);
204+
}
205+
}
206+
}
196207
for vmid in placements.iter().filter_map(|(vmid, entries)| {
197208
entries
198209
.iter()
@@ -281,6 +292,8 @@ impl SchedulerActor {
281292
keepalive_task.abort();
282293
}
283294
self.agent_data_cache.remove(&actor_id);
295+
self.agent_vm_index.remove(&actor_id);
296+
self.invalidate_pending_resources();
284297
Self::remove_agent_placements(
285298
actor_id,
286299
&mut self.vm_manifests,
@@ -295,6 +308,7 @@ impl SchedulerActor {
295308
keepalive_task.abort();
296309
}
297310
let vmid = self.vm_actorid_ulid_map.remove(&actor_id);
311+
self.invalidate_pending_resources();
298312
Self::remove_vm_actor(actor_id, &mut self.vm_data_cache);
299313
if let Some(vmid) = vmid
300314
&& self
@@ -598,13 +612,33 @@ impl SchedulerActor {
598612
/// - the cache likely needs to be updated automatically when a new vm is scheduled for info like used resources, because otherwise we have to deal with latency on that data we are using
599613
/// and then if someone tries to schedule lets say 10 VMs in a batch, we could end up scheduling them all to the same agent because the metadata hasn't updated.
600614
/// - there are a few solutions for this but they all kinda suck, mostly due to also making sure we deal with latency properly. I am ignoring the issue for now.
601-
fn schedule_agent(&self, msg: &CreateVM) -> Result<RemoteActorRef<AgentActor>, Report> {
602-
let pending_resources = pending_resources_by_agent(&self.vm_manifests, &self.vm_placements);
615+
fn pending_resources(&mut self) -> &AHashMap<ActorId, (u32, u64)> {
616+
if self.pending_resources_cache.is_none() {
617+
self.pending_resources_cache = Some(pending_resources_by_agent(
618+
&self.vm_manifests,
619+
&self.vm_placements,
620+
));
621+
}
622+
self.pending_resources_cache
623+
.as_ref()
624+
.expect("pending resources cache was just initialized")
625+
}
626+
627+
fn invalidate_pending_resources(&mut self) {
628+
self.pending_resources_cache = None;
629+
}
630+
631+
fn schedule_agent(&mut self, msg: &CreateVM) -> Result<RemoteActorRef<AgentActor>, Report> {
632+
self.pending_resources();
633+
let pending_resources = self
634+
.pending_resources_cache
635+
.as_ref()
636+
.expect("pending resources cache was just initialized");
603637
let mut best_agent = None;
604638
let mut best_score = AgentScore::REJECTED;
605639

606640
for agent in self.agent_data_cache.values() {
607-
let score = self.score_agent(msg, agent, &pending_resources);
641+
let score = self.score_agent(msg, agent, pending_resources);
608642

609643
if score > best_score {
610644
best_agent = Some(agent.actor_ref.clone());
@@ -617,6 +651,32 @@ impl SchedulerActor {
617651
best_agent.ok_or_eyre("No valid agents found.")
618652
}
619653

654+
#[expect(dead_code, reason = "reserved for a future batch create message")]
655+
fn schedule_agents(
656+
&mut self,
657+
msgs: &[CreateVM],
658+
) -> Vec<Result<RemoteActorRef<AgentActor>, Report>> {
659+
self.pending_resources();
660+
let pending_resources = self
661+
.pending_resources_cache
662+
.as_ref()
663+
.expect("pending resources cache was just initialized");
664+
msgs.iter()
665+
.map(|msg| {
666+
let mut best_agent = None;
667+
let mut best_score = AgentScore::REJECTED;
668+
for agent in self.agent_data_cache.values() {
669+
let score = self.score_agent(msg, agent, pending_resources);
670+
if score > best_score {
671+
best_agent = Some(agent.actor_ref.clone());
672+
best_score = score;
673+
}
674+
}
675+
best_agent.ok_or_eyre("No valid agents found.")
676+
})
677+
.collect()
678+
}
679+
620680
// this function intentionally only checks against the cache. this has some positives and negatives:
621681
// positive: it will never trigger any network requests so its very fast, and having to do network requests for scoring whenever we want to schedule a vm is likely a bad idea
622682
// negative: it technically has a delayed view of the cluster, meaning that some things that happened in the future, may not exist yet. so we need to be careful about how this is done so affinity rules are not accidentally broken. mostly this means, if we do anything that could affect the outcome of an affinity rule (ex: network request to an agent), we need to update the cache, before we do the action.
@@ -691,6 +751,7 @@ impl SchedulerActor {
691751
metadata_tables.extend(
692752
Self::placement_vm_ids(
693753
&self.vm_placements,
754+
self.agent_vm_index.get(&agent.actor_ref.id()),
694755
agent.actor_ref.id(),
695756
&agent.data.vms,
696757
)
@@ -1081,6 +1142,8 @@ mod tests {
10811142
)]),
10821143
vm_data_cache: AHashMap::from([(vmid, vec![CachedVMActor { actor_ref: None }])]),
10831144
vm_keepalive_tasks: AHashMap::new(),
1145+
pending_resources_cache: None,
1146+
agent_vm_index: AHashMap::new(),
10841147
actor_kinds: AHashMap::from([(agent_id, CachedActorKind::Agent)]),
10851148
cache_actor_finder: None,
10861149
};
@@ -1106,6 +1169,8 @@ mod tests {
11061169
vm_placements: AHashMap::new(),
11071170
vm_data_cache: AHashMap::from([(vmid, vec![CachedVMActor { actor_ref: None }])]),
11081171
vm_keepalive_tasks: AHashMap::new(),
1172+
pending_resources_cache: None,
1173+
agent_vm_index: AHashMap::new(),
11091174
actor_kinds: AHashMap::from([(agent_id, CachedActorKind::Agent)]),
11101175
cache_actor_finder: None,
11111176
};
@@ -1260,6 +1325,8 @@ impl Actor for SchedulerActor {
12601325
vm_placements: AHashMap::new(),
12611326
vm_data_cache: AHashMap::new(),
12621327
vm_keepalive_tasks: AHashMap::new(),
1328+
pending_resources_cache: None,
1329+
agent_vm_index: AHashMap::new(),
12631330
actor_kinds: AHashMap::new(),
12641331
cache_actor_finder: None,
12651332
};
@@ -1397,12 +1464,16 @@ impl Message<AgentUpdated> for SchedulerActor {
13971464
async fn handle(&mut self, msg: AgentUpdated, _ctx: &mut Context<Self, Self::Reply>) {
13981465
let Some(cached) = self.agent_data_cache.get_mut(&msg.actor_id) else {
13991466
if let AgentStatusUpdate::Full { revision, status } = msg.update {
1467+
self.agent_vm_index
1468+
.insert(msg.actor_id, status.vms.iter().copied().collect());
1469+
self.invalidate_pending_resources();
14001470
Self::reconcile_agent_placements(
14011471
msg.actor_id,
14021472
&status,
14031473
&self.vm_manifests,
14041474
&mut self.vm_placements,
14051475
);
1476+
self.invalidate_pending_resources();
14061477
self.agent_data_cache.insert(
14071478
msg.actor_id,
14081479
CachedAgentActor {
@@ -1427,6 +1498,16 @@ impl Message<AgentUpdated> for SchedulerActor {
14271498
let removed = removed.clone();
14281499
cached.status_revision = apply_status_update(&mut cached.data, msg.update);
14291500
cached.actor_ref = msg.actor_ref;
1501+
self.agent_vm_index
1502+
.entry(msg.actor_id)
1503+
.or_default()
1504+
.extend(added.iter().copied());
1505+
if let Some(index) = self.agent_vm_index.get_mut(&msg.actor_id) {
1506+
for vmid in &removed {
1507+
index.remove(vmid);
1508+
}
1509+
}
1510+
self.invalidate_pending_resources();
14301511
Self::reconcile_agent_delta(
14311512
msg.actor_id,
14321513
&added,
@@ -1437,12 +1518,15 @@ impl Message<AgentUpdated> for SchedulerActor {
14371518
} else {
14381519
cached.status_revision = apply_status_update(&mut cached.data, msg.update);
14391520
cached.actor_ref = msg.actor_ref;
1521+
self.agent_vm_index
1522+
.insert(msg.actor_id, cached.data.vms.iter().copied().collect());
14401523
Self::reconcile_agent_placements(
14411524
msg.actor_id,
14421525
&cached.data,
14431526
&self.vm_manifests,
14441527
&mut self.vm_placements,
14451528
);
1529+
self.invalidate_pending_resources();
14461530
}
14471531
}
14481532
}
@@ -1472,6 +1556,7 @@ impl Message<ReconcileVmPlacements> for SchedulerActor {
14721556
&mut self.vm_placements,
14731557
&mut self.vm_data_cache,
14741558
);
1559+
self.invalidate_pending_resources();
14751560
}
14761561
}
14771562

@@ -1486,6 +1571,7 @@ impl Message<CreateVM> for SchedulerActor {
14861571
let target_agent = self.schedule_agent(&msg)?;
14871572

14881573
self.vm_manifests.insert(msg.vmid, msg.config.clone());
1574+
self.invalidate_pending_resources();
14891575
self.vm_placements
14901576
.entry(msg.vmid)
14911577
.or_default()

odorobo/src/messages/agent.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,17 @@ pub fn apply_status_update(status: &mut AgentStatus, update: AgentStatusUpdate)
6969
used_vcpus,
7070
used_ram,
7171
} => {
72-
status.vms.retain(|vmid| !removed.contains(vmid));
73-
status.vms.extend(added);
72+
for vmid in removed {
73+
if let Ok(index) = status.vms.binary_search(&vmid) {
74+
status.vms.remove(index);
75+
}
76+
}
77+
for vmid in added {
78+
match status.vms.binary_search(&vmid) {
79+
Ok(_) => {}
80+
Err(index) => status.vms.insert(index, vmid),
81+
}
82+
}
7483
status.used_vcpus = used_vcpus;
7584
status.used_ram = used_ram;
7685
revision

0 commit comments

Comments
 (0)