storage controller: a reconcile result can clear a newer pending compute notification

Problem

There appears to be a rare race in the storage controller's handling of pending_compute_notification.

An in-flight reconciler snapshots this flag when it is spawned. Separately, a failed startup or shard-split notification can set the live flag to true. When the older reconciler completes, process_result unconditionally replaces the live flag with the value from that reconciler's result:

tenant.pending_compute_notification = result.pending_compute_notification;

If the reconciler started before the notification failed, did not send a notification itself, and finishes after the failure was recorded, its false result can overwrite the newer true value.

I found this by source inspection. I have not observed this exact interleaving in a deployed environment or reproduced it end to end. It requires several events to align, so this should be treated as a rare convergence edge case rather than a common failure mode.

Affected revision reviewed: 8f60b04da47ffefe0e52bda2440134b42874eb75.

Relevant code

The reconciler copies the pending state once, when it is spawned:

let must_notify = self.pending_compute_notification;

TenantShard::spawn_reconciler

Its result reports only whether that particular reconciler failed to notify compute:

ReconcileResult {
    // ...
    pending_compute_notification: reconciler.compute_notify_failure,
}

TenantShard::reconcile

process_result then treats that snapshot-derived value as the current value:

tenant.pending_compute_notification = result.pending_compute_notification;

Service::process_result

There are two asynchronous writers that can set the flag after the reconciler has already taken its snapshot.

Failed background notification during startup:

if let Some(shard) = locked.tenants.get_mut(&tenant_shard_id) {
    shard.pending_compute_notification = true;
}

Service::process_results

Failed child-shard notification during a split:

for failed in failed_notifications {
    if let Some(shard) = locked.tenants.get_mut(&failed) {
        shard.pending_compute_notification = true;
    }
}

Service::do_tenant_shard_split

Neither writer advances the shard sequence or otherwise marks an already-running reconciler's result as stale.

Possible interleaving

A shard split is the clearest example I found:

1. A tenant using PlacementPolicy::Attached(1) is split into child shards.
2. tenant_shard_split_commit_inmem constructs a child whose attached location is already present in both intent and observed state. Scheduling its secondary starts reconciler R with must_notify = false.
3. R sees that the attached configuration is already correct. It can perform the secondary-location work without calling compute_notify for the attachment.
4. The split attempts to send the complete child-shard map to the control plane. That call fails, so the split records child.pending_compute_notification = true for a later retry.
5. R finishes afterward. Because it did not attempt a notification, R.compute_notify_failure is false.
6. process_result applies R's result and changes the live flag from true back to false.

The child reconcilers are started in tenant_shard_split_commit_inmem, before the child notifications and failure flags are handled in do_tenant_shard_split.

For a sharded tenant, ComputeHookTenant waits until it knows all child locations before transmitting the new map. The failed flag is associated with the child whose update completes that map. The race therefore requires that particular child's reconciler to still be running when the notification fails.

There is a similar startup variant:

1. startup_reconcile queues background notifications for stably attached shards.
2. It then starts reconciliation without awaiting those notifications.
3. A shard with a correct attachment but stale or missing secondary starts a secondary-only reconcile with must_notify = false.
4. The background notification fails and sets the live pending flag.
5. The secondary-only reconcile finishes later and clears it.

See the notification/reconciliation ordering in Service::startup_reconcile.

Conditions and likelihood

The overwrite requires all of the following:

1. A startup or shard-split compute notification fails.
2. A reconciler for the same shard is already in flight and had copied pending_compute_notification = false.
3. That reconciler does not independently send a successful compute notification.
4. Its result is processed after the asynchronous failure handler sets the flag.
5. No later event sends the missing routing update through another path.

This is possible but timing-sensitive. A fast terminal response such as 423 Locked can make the notification fail while a child reconciler is still doing pageserver I/O, but the opposite completion order is also possible and is safe: if the reconcile result is processed first, the later failure leaves the flag set to true.

I am not aware of public telemetry that can establish how often this exact ordering occurs. Compute-notification failures themselves have occurred—for example, #8820 records Control plane tenant busy responses—but that does not demonstrate this race.

Possible impact

If the flag is lost after a shard split, a running compute may retain the old parent shard map after the old location has been detached or cleaned up. If it is lost during startup, a compute may retain an old pageserver for the affected shard. Requests to a pageserver that no longer hosts that shard can fail and reconnect repeatedly.

This is the same general availability symptom previously recorded for a different missed-notification path in #11291: the compute continued using an old pageserver that no longer hosted the shard. PR #11342 fixed that earlier path by notifying after refreshed observed state; it does not appear to cover a newer pending flag being overwritten by an in-flight result.

I do not see evidence here of stale data, cross-tenant access, or data corruption. The expected failure mode is read unavailability until another routing update repairs the compute configuration.

The state can self-heal after a later attachment change or storage-controller restart. However, the ordinary 20-second reconciliation loop will not repair an otherwise-clean shard after the pending flag has been cleared, because get_reconcile_needed then returns No.

Suggested fix

A reconcile result should only clear the notification obligation that the reconciler observed when it started. It should not clear a newer obligation recorded while it was running.

One option is to version the pending state:

1. Increment a per-shard notification epoch whenever asynchronous code records a new pending notification.
2. Snapshot the epoch when spawning a reconciler and include it in ReconcileResult.
3. Allow a successful result to clear the live flag only when its snapshot epoch still matches the current epoch.
4. Preserve the live flag when the epochs differ or when the result reports a new notification failure.

An equivalent requested/acknowledged generation pair would also work.

A bare OR merge is probably not sufficient:

tenant.pending_compute_notification |= result.pending_compute_notification;

The existing flag remains true while a reconciler spawned to handle it is running. After that reconciler successfully notifies compute and returns false, an OR-only merge would leave the old flag set and cause redundant retries indefinitely.

Regression test idea

The race should be reproducible with existing failpoints:

1. Let the initial compute notification for an Attached(1) tenant succeed, then make the test hook return 423.
2. Start a 1 -> 2 split and pause it at shard-split-pre-complete-pause.
3. Enable reconciler-epilogue = pause, then release the split failpoint. This allows the child notification to fail while keeping the child reconcile result in flight.
4. After the split records the failed notification, release reconciler-epilogue.
5. On the affected code, the child result should clear the pending flag. After changing the hook to 200, reconcile_all should send no replacement notification for an otherwise-clean child.
6. The fixed behavior should retain the newer obligation and send the complete child map once the hook recovers.