Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions kernel/relayflowd-core/src/machine/parallel_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,32 @@ fn overlapping_agent_surfaces_are_serialized_in_authored_order() {
let mut recovering_entries = entries.clone();
recovering_entries.extend(appended_entries(&recovery_actions(&running, 12)));
let recovering = RunState::fold("run", spec.clone(), &recovering_entries).unwrap();
// The invariant is that the sibling does not pass the unfinished lane. It
// used to be spelled "every action is an ArmTimer", which held only because
// a dead leased attempt served a retry delay there was a timer to arm for.
// Recovery no longer imposes that delay (see `abandonment_actions`), so the
// lane is due at once and the pass wakes it instead of sleeping. Assert the
// invariant itself rather than the mechanism, which is strictly stronger:
// no action may start the conflicting sibling, whether or not a timer is
// involved.
let recovering_actions = next_actions(&recovering, 12);
assert!(
next_actions(&recovering, 12)
.iter()
.all(|action| matches!(action, Action::ArmTimer { .. })),
"the conflicting sibling must not pass an unfinished lane in retry backoff"
!recovering_actions.iter().any(|action| matches!(
action,
Action::Append(entry)
if entry.entry_type == EntryType::StepAttemptStarted
&& entry.step_id.as_deref() == Some("lane-a")
)),
"the conflicting sibling must not pass an unfinished lane being recovered"
);
assert!(
recovering_actions.iter().any(|action| matches!(
action,
Action::Append(entry)
if entry.entry_type == EntryType::WaitCompleted
&& entry.step_id.as_deref() == Some("lane-b")
)),
"the recovered lane must be woken before any sibling is considered"
);

entries.push(agent_success(&spec, "lane-b", "rB", 12));
Expand Down Expand Up @@ -432,3 +453,4 @@ fn failed_run_drains_open_siblings_before_terminal_entry() {
Err(StateError::EntryAfterRunCompleted { .. })
));
}

30 changes: 22 additions & 8 deletions kernel/relayflowd-core/src/machine/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use serde_json::Value;

use super::{Action, backoff_delay_ms, deterministic_ulid, idempotency_key, retry_wait_id};
use super::{Action, deterministic_ulid, retry_wait_id};
use crate::{
entry::{
Budget, CompletionReason, Disposition, EntryType, JournalEntry, Pins, SleepUntilPayload,
Expand Down Expand Up @@ -86,13 +86,27 @@ pub fn abandonment_actions(
}
);
let may_retry = runtime.semantic_executions < spec.max_iterations;
let next_attempt_at_ms = (may_retry && !manual).then(|| {
now_ms.saturating_add(backoff_delay_ms(
&spec.retry,
&idempotency_key(&state.run_id, step_id),
attempt,
) as i64)
});
// No retry delay for a dead leased attempt. This function records an
// attempt that died WITHOUT producing a result a gate could judge -- which
// is why, as the doc comment above says, it does not charge a semantic
// iteration either. Rate-limiting it is the same category error: the
// backoff curve exists to damp a step that keeps failing on its own merits,
// not one whose worker was killed.
//
// Leaving the delay in place also made recovery order a race, which is
// issue #155. The dead lane sat in `Backoff` with `wake_at_ms` a few
// milliseconds in the future; the scheduler's `due_waits` only fires once
// that passes, and `due_waits` is returned ahead of any `starts`. So
// whether the recovered lane or an idle sibling claimed the one free worker
// depended purely on when the next pass landed relative to that deadline:
//
// PROBE pass now=1788503129020
// states=[("lane-b", Backoff { wake_at_ms: 1788503129025 }),
// ("lane-a", Runnable)] due_waits=0
//
// Five milliseconds decided it, and `worker_capacity.rs:149` saw `lane-a`
// instead of the retried `lane-b` in roughly 15% of runs.
let next_attempt_at_ms = (may_retry && !manual).then_some(now_ms);
let mut actions = vec![Action::Append(JournalEntry::new(
EntryType::StepCompleted,
state.run_id.clone(),
Expand Down
Loading