fix(#1038): include parent_id in Rebuild dedup key to prevent cascade swallowing
This commit is contained in:
parent
146a6b7cd5
commit
bc7caf572f
1 changed files with 61 additions and 2 deletions
|
|
@ -223,7 +223,13 @@ impl RebuildQueue {
|
|||
///
|
||||
/// Dedup rule:
|
||||
/// - `Rebuild` / `Spawn` / `Destroy`: a `Queued` entry with the same
|
||||
/// `(kind, agent)` swallows the new request.
|
||||
/// `(kind, agent, parent_id)` swallows the new request. `parent_id`
|
||||
/// is part of the key so that a `MetaUpdate` cascade rebuild (with a
|
||||
/// specific `parent_id`) never collapses into a standalone rebuild or
|
||||
/// a cascade from a different `MetaUpdate`. Without this guard a
|
||||
/// cascade rebuild pre-enqueued before the lock bump would be swallowed
|
||||
/// by an existing `Queued` startup-sweep rebuild, causing the agent to
|
||||
/// never rebuild against the post-bump meta.
|
||||
/// - `MetaUpdate`: dedup ALSO requires the `inputs` field to match —
|
||||
/// two meta-updates with different input lists are distinct work
|
||||
/// and must queue separately, otherwise the second meta-update
|
||||
|
|
@ -287,13 +293,17 @@ impl RebuildQueue {
|
|||
// and, for MetaUpdate, the same `inputs` list (see method
|
||||
// docstring for why). Approval-driven entries also require the
|
||||
// approval_id to match so two distinct approvals for the same
|
||||
// agent never collapse into one queue slot.
|
||||
// agent never collapse into one queue slot. Rebuild (and Spawn /
|
||||
// Destroy) entries also require parent_id to match so a
|
||||
// MetaUpdate cascade rebuild is never swallowed by an unrelated
|
||||
// queued rebuild (e.g. from the startup sweep).
|
||||
for entry in &mut inner.entries {
|
||||
if entry.state == QueueState::Queued
|
||||
&& entry.kind == kind
|
||||
&& entry.agent == agent
|
||||
&& (kind != QueueKind::MetaUpdate || entry.inputs == inputs)
|
||||
&& entry.approval_id == approval_id
|
||||
&& entry.parent_id == parent_id
|
||||
{
|
||||
if !entry.reason.contains(&reason) {
|
||||
use std::fmt::Write as _;
|
||||
|
|
@ -1147,6 +1157,55 @@ mod tests {
|
|||
assert!(!q.set_step(999, "anything"));
|
||||
}
|
||||
|
||||
/// A MetaUpdate cascade Rebuild (with parent_id = Some(meta_id)) must
|
||||
/// NOT dedup into a pre-existing Queued Rebuild with a different parent_id
|
||||
/// (e.g. from a startup sweep). Without the parent_id dedup guard the
|
||||
/// cascade rebuild would be swallowed and the agent would never rebuild
|
||||
/// against the post-lock-bump meta.
|
||||
#[test]
|
||||
fn meta_update_cascade_does_not_dedup_into_startup_sweep_rebuild() {
|
||||
let q = RebuildQueue::new();
|
||||
// Startup sweep enqueues a Rebuild for alice with its own parent_id.
|
||||
let sweep = q.enqueue(
|
||||
QueueKind::StartupSweep,
|
||||
"hyperhive".to_owned(),
|
||||
QueueSource::AutoUpdate,
|
||||
"boot sweep".to_owned(),
|
||||
None,
|
||||
);
|
||||
let sweep_rebuild = q.enqueue(
|
||||
QueueKind::Rebuild,
|
||||
"alice".to_owned(),
|
||||
QueueSource::StartupSweep,
|
||||
"startup sweep".to_owned(),
|
||||
Some(sweep),
|
||||
);
|
||||
// MetaUpdate cascade pre-enqueues another Rebuild for alice.
|
||||
let meta = q.enqueue(
|
||||
QueueKind::MetaUpdate,
|
||||
"hyperhive".to_owned(),
|
||||
QueueSource::Manual,
|
||||
"bump nixpkgs".to_owned(),
|
||||
None,
|
||||
);
|
||||
let cascade_rebuild = q.enqueue(
|
||||
QueueKind::Rebuild,
|
||||
"alice".to_owned(),
|
||||
QueueSource::MetaUpdate,
|
||||
"meta-update cascade".to_owned(),
|
||||
Some(meta),
|
||||
);
|
||||
// The two Rebuilds have different parent_ids — must NOT dedup.
|
||||
assert_ne!(sweep_rebuild, cascade_rebuild,
|
||||
"cascade rebuild must be distinct from startup-sweep rebuild");
|
||||
let snap = q.snapshot();
|
||||
let rebuilds: Vec<_> = snap.iter()
|
||||
.filter(|e| e.kind == QueueKind::Rebuild && e.agent == "alice")
|
||||
.collect();
|
||||
assert_eq!(rebuilds.len(), 2,
|
||||
"both rebuilds must be present in the queue");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn finish_clears_step() {
|
||||
let q = RebuildQueue::new();
|
||||
|
|
|
|||
Loading…
Reference in a new issue