Found by CodeRabbit while reviewing #1540; the code is on main, not in that PR's diff.
In Orchestrator::submit_task (src/common/hierarchical/orchestrator.cpp), the accept count is raised before the poisoned-producer branch:
550: increment_run_accepts(run->id, s.group_size());
551:
552: if (poisoned_by_failed_producer) {
...marks the slot failed and consumes it without enqueueing a dispatch...
return SubmitResult{slot};
}
A poisoned slot never reaches an endpoint, so no mark_task_accepted ever decrements for those group members. run->pending_accepts stays above zero for the rest of the run.
acceptance_ready() is submission_closed && (pending_accepts == 0 || is_terminal(phase)), so the fence is not stranded — the terminal disjunct eventually satisfies it. But it does mean the fence only opens at completion instead of at acceptance, and Worker.submit() waits on prior handles' acceptance before building the next graph. So one poisoned producer silently serialises the next submission behind the whole run finishing, which is exactly the overlap the launch-acceptance fence exists to provide.
Fix is to move the increment past the branch so it counts only actually-dispatched members:
if (scope_ref > 0) scope_->register_task(slot);
increment_run_tasks(run->id);
- increment_run_accepts(run->id, s.group_size());
if (poisoned_by_failed_producer) {
...
return SubmitResult{slot};
}
+ increment_run_accepts(run->id, s.group_size());
Worth a regression test that submits into a run with a failed producer and asserts the acceptance fence opens before the run goes terminal.
Found by CodeRabbit while reviewing #1540; the code is on
main, not in that PR's diff.In
Orchestrator::submit_task(src/common/hierarchical/orchestrator.cpp), the accept count is raised before the poisoned-producer branch:A poisoned slot never reaches an endpoint, so no
mark_task_acceptedever decrements for those group members.run->pending_acceptsstays above zero for the rest of the run.acceptance_ready()issubmission_closed && (pending_accepts == 0 || is_terminal(phase)), so the fence is not stranded — the terminal disjunct eventually satisfies it. But it does mean the fence only opens at completion instead of at acceptance, andWorker.submit()waits on prior handles' acceptance before building the next graph. So one poisoned producer silently serialises the next submission behind the whole run finishing, which is exactly the overlap the launch-acceptance fence exists to provide.Fix is to move the increment past the branch so it counts only actually-dispatched members:
if (scope_ref > 0) scope_->register_task(slot); increment_run_tasks(run->id); - increment_run_accepts(run->id, s.group_size()); if (poisoned_by_failed_producer) { ... return SubmitResult{slot}; } + increment_run_accepts(run->id, s.group_size());Worth a regression test that submits into a run with a failed producer and asserts the acceptance fence opens before the run goes terminal.