Fix possible race when stopping sled evacuating instance during reincarnation

In #11504 (comment) we identified the possibility of a race when a user attempts to stop an instance during reincarnation due to a sled evacuation. The result is the instance being indefinitely stuck in Rebooting state until the instance is manually stopped.

Details

From comment:

So I looked deeper into this (with ai help) and it appears to be that there is a small window where this could happen.

Let's say the VMM stopper task runs, and stops a VMM. Then, as Propolis shuts down, sled agent reports when the state changes. Once it reports the VMM is Destroyed, an instance-update saga is executed due to Destroyed being a terminal state

omicron/nexus/src/app/sagas/instance_update/mod.rs

Lines 423 to 430 in 08b9e31

    
           // Currently, an instance-update saga is required if (and only if): 
        
           // 
        
           // - The instance's active VMM has transitioned to `Destroyed` or `Failed`. 
        
           //   We don't actually know whether the VMM whose state was updated here was the 
        
           //   active VMM or not, so we will always attempt to run an instance-update 
        
           //   saga if the VMM was `Destroyed` (or `Failed`). 
        
           let vmm_needs_update = 
        
               result.vmm_updated && state.vmm_state.state.is_terminal();

During the start-instance-update saga, there is a moment where it takes a snapshot of the instance, VMM and migration info

omicron/nexus/src/app/sagas/instance_update/start.rs

Lines 329 to 332 in 40b22e5

    
           let state = datastore 
        
               .instance_fetch_all(&opctx, &authz_instance) 
        
               .await 
        
               .map_err(saga_action_failed)?;

The saga then passes that data to UpdatesRequired::for_instance. There it sees the VMM is marked and the intended state is Running so it sets the instance state as SledEvacuating.

omicron/nexus/src/app/sagas/instance_update/mod.rs

Lines 754 to 766 in 08b9e31

    
                       } else if let Some(g) = active_vmm_update_disposition_gen 
        
                           && snapshot.instance.intended_state 
        
                               == InstanceIntendedState::Running 
        
                       { 
        
                           info!( 
        
                               log, 
        
                               "instance update: VMM was stopped due to its sled being \ 
        
                                evacuated for an update. Setting new runtime instance \ 
        
                                state to SledEvacuating."; 
        
                               "instance_id" => %instance_id, 
        
                               "update_disposition_generation" => ?g 
        
                           ); 
        
                           InstanceState::SledEvacuating

After that the instance_update saga does several things:

Inherits the lock from the start saga
updates the network config
unassigns the oximeter producer
And finally, it commits the new state (SledEvacuating).

omicron/nexus/src/app/sagas/instance_update/mod.rs

Lines 941 to 974 in 08b9e31

    
           // Generate a new ID and attempt to inherit the lock from the start saga. 
        
           builder.append(Node::action( 
        
               INSTANCE_LOCK_ID, 
        
               "GenerateInstanceLockId", 
        
               ACTION_GENERATE_ID.as_ref(), 
        
           )); 
        
           builder.append(become_updater_action()); 
        
           let UpdatesRequired { 
        
               new_runtime: _, 
        
               new_intent: _, 
        
               destroy_active_vmm, 
        
               update_sled_reservations_for_migration_success, 
        
               destroy_target_vmm, 
        
               deprovision, 
        
               network_config, 
        
           } = &params.update; 
        
           // If a network config update is required, do that. 
        
           if let Some(update) = network_config { 
        
               builder.append(const_node(NETWORK_CONFIG_UPDATE, update)?); 
        
               builder.append(update_network_config_action()); 
        
           } 
        
           // If the instance now has no active VMM, release its virtual 
        
           // provisioning resources and unassign its Oximeter producer. 
        
           if deprovision.is_some() { 
        
               builder.append(release_virtual_provisioning_action()); 
        
               builder.append(unassign_oximeter_producer_action()); 
        
           } 
        
           // Once we've finished mutating everything owned by the instance, we can 
        
           // write back the updated state and release the instance lock. 
        
           builder.append(commit_instance_updates_action());

If the instance were to be stopped in the window between when the snapshot is taken and the information committed, the following would happen:

instance_stop sets the intended state to Stopped, but it doesn't check the lock or change the state generation

omicron/nexus/src/app/instance.rs

Lines 1153 to 1164 in 08b9e31

    
           let mut state = self 
        
               .db_datastore 
        
               .instance_fetch_with_vmm(opctx, &authz_instance) 
        
               .await?; 
        
           state.instance = self 
        
               .db_datastore 
        
               .instance_set_intended_state( 
        
                   opctx, 
        
                   &authz_instance, 
        
                   IntendedState::Stopped, 
        
               ) 
        
               .await?;

At this point the instance_update saga has not committed the new information, so the instance record still points to the destroyed VMM. This means we get a NO_SUCH_INSTANCE error with status code 410. It appears that this error is bubbled up to the user.

omicron/sled-agent/src/sled_agent.rs

Lines 279 to 282 in 08b9e31

    
           InstanceError::Terminating => { 
        
               HttpError::for_client_error( 
        
                   Some(NO_SUCH_INSTANCE.to_string()), 
        
                   ClientErrorStatusCode::GONE,

Now, instance_stop marks the VMM as failed, and the row is soft deleted.

Then, the previous in-flight saga commits the information snapshot it took before the user requested to stop the instance, it does not contain a new intended state, so it keeps Stopped.

The instance would be stuck in a Rebooting state, unless it is stopped again.

In short:

jgallagher left a comment:

UpdatesRequired::for_instance() decides the new nexus_state should be SledEvacuating based on the snapshot's intended state of Running

a user request arrives that changes the intended state to Stopped

siu_commit_instance_updates() commits the SledEvacuating state chosen in step 1, without realizing the intended state has changed in the meantime (and that we would've written NoVmm here instead if we had)

If that's correct, it seems like it's the last bullet that's misbehaving, right? Somehow that commit needs to either fail (because it's trying to write a state based on a stale input) or make the SledEvacuating vs NoVmm decision there based on the current intended state, rather than using the value chosen from the earlier snapshot?

In addition, there is be a similar potential race in sis_move_to_starting #11504 (comment)

Is this a blocker for 24 / graceful shutdown during update?

@davepacheco

The window is very very small, but the result is that the instance would get stuck in Rebooting state (visible to the user) indefinitely. It can get unstuck if the instance is stopped again, but it's a pretty bad experience.

Due to the window being so small, I'm not sure if we would consider this a blocker or not. What do you think @davepacheco?

I'd probably say not if push comes to shove, but I'm happy to keep tracking it on the board with the goal of fixing it. Thanks for tagging the issue already!

Navigation Menu

Repository navigation

Fix possible race when stopping sled evacuating instance during reincarnation #11706

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// Currently, an instance-update saga is required if (and only if):
	//
	// - The instance's active VMM has transitioned to `Destroyed` or `Failed`.
	// We don't actually know whether the VMM whose state was updated here was the
	// active VMM or not, so we will always attempt to run an instance-update
	// saga if the VMM was `Destroyed` (or `Failed`).
	let vmm_needs_update =
	result.vmm_updated && state.vmm_state.state.is_terminal();

	let state = datastore
	.instance_fetch_all(&opctx, &authz_instance)
	.await
	.map_err(saga_action_failed)?;

	} else if let Some(g) = active_vmm_update_disposition_gen
	&& snapshot.instance.intended_state
	== InstanceIntendedState::Running
	{
	info!(
	log,
	"instance update: VMM was stopped due to its sled being \
	evacuated for an update. Setting new runtime instance \
	state to SledEvacuating.";
	"instance_id" => %instance_id,
	"update_disposition_generation" => ?g
	);
	InstanceState::SledEvacuating

	// Generate a new ID and attempt to inherit the lock from the start saga.
	builder.append(Node::action(
	INSTANCE_LOCK_ID,
	"GenerateInstanceLockId",
	ACTION_GENERATE_ID.as_ref(),
	));
	builder.append(become_updater_action());

	let UpdatesRequired {
	new_runtime: _,
	new_intent: _,
	destroy_active_vmm,
	update_sled_reservations_for_migration_success,
	destroy_target_vmm,
	deprovision,
	network_config,
	} = &params.update;

	// If a network config update is required, do that.
	if let Some(update) = network_config {
	builder.append(const_node(NETWORK_CONFIG_UPDATE, update)?);
	builder.append(update_network_config_action());
	}

	// If the instance now has no active VMM, release its virtual
	// provisioning resources and unassign its Oximeter producer.
	if deprovision.is_some() {
	builder.append(release_virtual_provisioning_action());
	builder.append(unassign_oximeter_producer_action());
	}

	// Once we've finished mutating everything owned by the instance, we can
	// write back the updated state and release the instance lock.
	builder.append(commit_instance_updates_action());

	let mut state = self
	.db_datastore
	.instance_fetch_with_vmm(opctx, &authz_instance)
	.await?;
	state.instance = self
	.db_datastore
	.instance_set_intended_state(
	opctx,
	&authz_instance,
	IntendedState::Stopped,
	)
	.await?;

	InstanceError::Terminating => {
	HttpError::for_client_error(
	Some(NO_SUCH_INSTANCE.to_string()),
	ClientErrorStatusCode::GONE,

Fix possible race when stopping sled evacuating instance during reincarnation #11706

Description

Activity

davepacheco commented on Nov 5, 2026

karencfv commented on Nov 5, 2026

davepacheco commented on Nov 5, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions