Propolis zone cleanup could happen outside of `InstanceRunner`

Open
#5,237 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
45/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Stale
Tech stack
rust

Research direction

Start in sled-agent/src/instance.rs, especially the state handling around lines 374-389 and 600-615 and termination around 759-793. Trace how VMM shutdown currently collects the zone bundle, tears down the zone, deregisters the instance, and publishes state; done means the runner can return promptly while deferring cleanup without breaking instance state updates or resource teardown.

Written by the indexing model from the issue text.

Description

Sled Agent virtualization

Every instance managed by a sled agent has a "runner" task and a "monitor" task. All requests to do anything with the instance (change its state, forcibly remove it from the sled agent, etc.) need to execute on the runner task, which handles these requests sequentially. This includes the processing of state change messages from Propolis, which are sent from the monitor task to the runner task and processed here: https://github.com/oxidecomputer/omicron/blob/8697f39aac3323ffda3f33f4b5aa44fd7ad857e3/sled-agent/src/instance.rs#L374-L389

If Propolis indicated that it shut down, InstanceRunner::observe_state will call InstanceRunner::terminate before returning and allowing new state to be published to Nexus: https://github.com/oxidecomputer/omicron/blob/8697f39aac3323ffda3f33f4b5aa44fd7ad857e3/sled-agent/src/instance.rs#L600-L615

Terminating an instance this way collects a zone bundle from the runner thread before removing the instance from the sled agent's InstanceManager and tearing down the zone: https://github.com/oxidecomputer/omicron/blob/8697f39aac3323ffda3f33f4b5aa44fd7ad857e3/sled-agent/src/instance.rs#L759-L793

Creating a zone bundle may be an expensive operation, since it (potentially) has to copy and compress many different log files and command outputs. This causes a couple of problems:

  • Instance stop will appear to take much longer than it actually does, because the relevant state transition doesn't go to Nexus until the zone is destroyed.
  • More importantly, the instance remains in the sled agent's instance table, but the InstanceRunner is completely blocked while all this work is going on. This means that sled agent API calls targeting an instance in this state are highly likely to time out waiting for the runner to respond. If the caller is Nexus and the request is to change the instance's state, this can cause instances to be marked as Failed (due to the error conversion rules described in #3238) even though they would correctly go to Stopped if left alone.

The reason I made these operations happen in this order (zone bundle collection -> zone teardown -> deregister instance -> publish to Nexus) was to try to mitigate #3325. I suspect, though, that that issue is not as much of a problem now that every newly-started instance gets a fresh Propolis ID (not the case prior to #4194), such that every incarnation of an instance on a sled will get a distinct zone name. If that's so, then it should be possible to mitigate these problems by changing what happens on VMM shutdown: the runner can remove the instance from the table, publish the new VMM state to Nexus, and then hand the defunct zone off to some other task to be cleaned up.[^1]

[^1]: Cleaning up the defunct zone outside of the InstanceRunner task allows that task to return immediately and produce a "runner task closed" error message for anyone who happened to have requested something of the runner while it was in the "observe state change" arm of its request handler. If zone cleanup happens on the runner, Nexus's instance state will still update right away, but calls that land in this window are much more likely to time out than to get a "clean" instance gone error.

Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from oxidecomputer/omicron

All issues in oxidecomputer/omicron

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.