hive-c0re: hold the in-flight rows still between polls
Coordinator::transient_snapshot builds a fresh HashMap on every call, so each /api/state render iterates it under a different hash seed and the dashboard's transient rows reorder while an operator watches an operation run. Nothing downstream sorts them. build_port_conflicts, three functions up the same file, already sorts both its levels. This is the asymmetry, not a new convention. Sorted by (name, kind, secs) so ties cannot drift either, plus five tests on a builder that had none. The clamp one is the reason to bother: `since` in the future yields a negative duration, and cast_unsigned would render that as an age near u64::MAX rather than zero.
This commit is contained in:
parent
d09ecbdc5c
commit
cd687fdb2b
1 changed files with 89 additions and 3 deletions
|
|
@ -372,7 +372,7 @@ fn build_transient_views(
|
|||
containers: &[ContainerView],
|
||||
transient_snapshot: &std::collections::HashMap<String, Vec<crate::job_queue::RunningTransient>>,
|
||||
) -> Vec<TransientView> {
|
||||
transient_snapshot
|
||||
let mut out: Vec<TransientView> = transient_snapshot
|
||||
.iter()
|
||||
.filter(|(name, _)| !containers.iter().any(|c| &c.name == *name))
|
||||
// One row per running node, so an agent with several shows several
|
||||
|
|
@ -389,7 +389,18 @@ fn build_transient_views(
|
|||
.max(0)
|
||||
.cast_unsigned(),
|
||||
})
|
||||
.collect()
|
||||
.collect();
|
||||
// The caller rebuilds this map on every poll, so its iteration order
|
||||
// differs between renders and the rows would shuffle under an operator
|
||||
// watching an operation run. Sorted for the same reason
|
||||
// `build_port_conflicts` sorts its clusters.
|
||||
out.sort_by(|a, b| {
|
||||
a.name
|
||||
.cmp(&b.name)
|
||||
.then_with(|| a.kind.cmp(&b.kind))
|
||||
.then_with(|| a.secs.cmp(&b.secs))
|
||||
});
|
||||
out
|
||||
}
|
||||
|
||||
/// Render each pending approval into its dashboard view (short sha for
|
||||
|
|
@ -717,7 +728,82 @@ pub(super) async fn dashboard_stream(
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{ContainerView, build_port_conflicts};
|
||||
use std::collections::HashMap;
|
||||
|
||||
use chrono::{Duration, Utc};
|
||||
|
||||
use super::{ContainerView, build_port_conflicts, build_transient_views};
|
||||
use crate::job_queue::RunningTransient;
|
||||
|
||||
fn rt(agent: &str, label: &str, age_secs: i64) -> RunningTransient {
|
||||
RunningTransient {
|
||||
agent: agent.to_owned(),
|
||||
label: label.to_owned(),
|
||||
takes_container_down: false,
|
||||
since: Utc::now() - Duration::seconds(age_secs),
|
||||
}
|
||||
}
|
||||
|
||||
fn snapshot(rows: Vec<RunningTransient>) -> HashMap<String, Vec<RunningTransient>> {
|
||||
let mut m: HashMap<String, Vec<RunningTransient>> = HashMap::new();
|
||||
for r in rows {
|
||||
m.entry(r.agent.clone()).or_default().push(r);
|
||||
}
|
||||
m
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_agent_whose_container_exists_is_not_transient() {
|
||||
let snap = snapshot(vec![rt("alice", "spawn", 5)]);
|
||||
let live = [cv("alice", 8101, true)];
|
||||
assert!(build_transient_views(&live, &snap).is_empty());
|
||||
// Control: with no container by that name it *is* reported, so the
|
||||
// emptiness above is the filter and not an inert builder.
|
||||
assert_eq!(build_transient_views(&[], &snap).len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_agent_with_several_running_nodes_gets_a_row_each() {
|
||||
let snap = snapshot(vec![rt("alice", "spawn", 5), rt("alice", "update", 3)]);
|
||||
assert_eq!(build_transient_views(&[], &snap).len(), 2);
|
||||
}
|
||||
|
||||
/// The snapshot is a fresh `HashMap` per poll, so its iteration order is
|
||||
/// not stable across renders. Sorting is what keeps a row still.
|
||||
#[test]
|
||||
fn rows_are_ordered_by_name_then_kind() {
|
||||
let snap = snapshot(vec![
|
||||
rt("zoe", "spawn", 1),
|
||||
rt("adam", "update", 1),
|
||||
rt("adam", "spawn", 1),
|
||||
]);
|
||||
let got: Vec<(String, String)> = build_transient_views(&[], &snap)
|
||||
.into_iter()
|
||||
.map(|v| (v.name, v.kind))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
got,
|
||||
[
|
||||
("adam".to_owned(), "spawn".to_owned()),
|
||||
("adam".to_owned(), "update".to_owned()),
|
||||
("zoe".to_owned(), "spawn".to_owned()),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// A `since` in the future yields a negative duration, and casting that
|
||||
/// unsigned would render an age near `u64::MAX` rather than a small one.
|
||||
#[test]
|
||||
fn a_future_since_clamps_to_zero_instead_of_wrapping() {
|
||||
let snap = snapshot(vec![rt("alice", "spawn", -600)]);
|
||||
let got = build_transient_views(&[], &snap);
|
||||
assert_eq!(got[0].secs, 0, "clamped, not wrapped");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_empty_snapshot_produces_no_rows() {
|
||||
assert!(build_transient_views(&[], &snapshot(vec![])).is_empty());
|
||||
}
|
||||
|
||||
fn cv(name: &str, port: u16, running: bool) -> ContainerView {
|
||||
ContainerView {
|
||||
|
|
|
|||
Loading…
Reference in a new issue