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
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,19 @@ impl CodeIndexWorktreeSchedulerV1 {
.sealed_replay_binding(generation_id)
.map_err(|error| CodeIndexProductionErrorV1::Publication(error).into())
}

/// Whether the durable publication pointer names `generation_id`. Graph heads
/// only follow the pointer, so a head on any other generation is older than it.
pub fn names_active_publication(
&self,
generation_id: &tracedecay_domain::CodeGenerationId,
) -> Result<bool, CodeIndexSchedulerErrorV1> {
Ok(self
.publication
.read_publication_pointer()
.map_err(CodeIndexProductionErrorV1::Publication)?
.is_some_and(|pointer| pointer.generation_id == generation_id.as_str()))
}
}

struct SchedulerGraphCancellation(Arc<AtomicBool>);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ pub(crate) fn retained_noop_requires_follow_up_wake(
serving_empty && !activation_deferred && consumed_external_arrival && source_is_noop
}

/// The verified head belongs to a different generation than the retained
/// manifest this pass tried to recover. Replaying that manifest publishes a
/// second graph over the generation that owns the head.
/// The verified head generation differs from the retained manifest this pass
/// tried to recover.
pub(crate) fn graph_head_belongs_to_another_generation(error: &CodeIndexSchedulerErrorV1) -> bool {
error.activation_conflict_context().is_some_and(|context| {
context.site == "code_graph.recover_verified_snapshot_from_head.generation"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1936,17 +1936,20 @@ impl CodeIndexSchedulerRegistryV1 {
let shutting_down = Arc::clone(&worker_shutting_down);
let replay_passes = Arc::clone(&worker_reconcile_in_progress);
let replay_binding = tokio::task::spawn_blocking(move || {
Self::lock_scheduler_for_graph_step(
let scheduler = Self::lock_scheduler_for_graph_step(
&replay_scheduler,
&shutting_down,
&replay_passes,
)?
.1
.code_graph_replay_binding(&generation_id)
.1;
Ok::<_, CodeIndexSchedulerErrorV1>((
scheduler.code_graph_replay_binding(&generation_id)?,
scheduler.names_active_publication(&generation_id)?,
))
})
.await;
match replay_binding {
Ok(Ok(replay_binding)) => {
Ok(Ok((replay_binding, retained_names_active_publication))) => {
match worker_graph_activation
.recover_verified_head(
&worker_project_id,
Expand Down Expand Up @@ -1983,12 +1986,24 @@ impl CodeIndexSchedulerRegistryV1 {
);
}
Ok(false) => {}
Err(error)
if graph_head_belongs_to_another_generation(&error)
&& retained_names_active_publication =>
{
Comment on lines +1989 to +1992

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Newer graph head overwritten by stale replay

If publication advances during recover_verified_head, retained_names_active_publication remains true for the old generation. Its sealed replay can then replace the newer graph head while the pointer names the newer generation.

Learn more

The worker reads the durable text publication pointer while holding the scheduler mutex, then releases it before awaiting graph-head recovery. Another publisher can commit a successor and seat its graph during that await. A generation mismatch is then interpreted using the obsolete pointer snapshot, and the retained generation reaches publish_sealed_graph, which publishes using the graph store's current head rather than requiring that this generation still owns the text pointer. The graph can therefore regress behind the active text publication.

Example: The pointer initially names generation G2 and the graph head names G1. While G2's head recovery is awaiting, another publisher advances both to G3. G2's recovery sees G3 but the captured boolean remains true; G2 publishes its graph over G3 instead of deferring to G3.

Recommended fix: Revalidate active publication ownership at the point of replay admission and fence the graph-head commit against subsequent pointer changes. A second unchecked pointer read before the asynchronous graph publish only narrows, rather than closes, this race.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

// The durable pointer names this manifest, so the head is
// older and this generation's graph publication stopped
// before seating; resume it from its sealed segments.
tracing::info!(
event = "code_index_graph_head_recovery_resumes_publication",
error = %error,
"durable publication names the retained manifest; resume \
its interrupted graph publication"
);
}
Err(error) if graph_head_belongs_to_another_generation(&error) => {
// The head is a different generation. Falling
// through into a cold replay discards the
// in-flight successor that owns it. Leave
// that generation's own publish to seat its
// graph, and do not retry this manifest.
// A newer generation owns the durable pointer. Falling
// through into a cold replay discards the in-flight successor
// that owns the head; do not retry this manifest.
prepare_graph = false;
graph_head_conflict_generation = Some(recovered_generation);
pass_waits_for_store_release = true;
Expand Down
Loading