Skip to content

Commit 7cd7c88

Browse files
authored
server: extend API request queue's memory of prior requests (#880)
Today the server's API request queue decides whether to accept or reject incoming requests by storing a fixed disposition for each kind of request and adjusting those dispositions as new requests are queued or as a VM's state changes. This is not always enough information to maintain the proper dispositions. Consider the following example: 1. A VM begins to start. Reboot requests are denied in this state (since a VM that hasn't booted yet can't be rebooted). 2. A caller queues a request to stop the VM after it starts. All subsequent reboot requests should now be denied because the VM will halt before they can be serviced. 3. The VM successfully starts. The correct reboot disposition is still "deny," because there's a pending stop request, but there's no way to discern this from the prior reboot disposition (it was also "deny" before the stop request was queued!). To address this sort of problem, and to pave the way for better handling of Crucible VCR replacements during instance start, refactor the queue as follows: - Remember what kinds of outstanding requests have not been processed and dispose of requests based on this state and the queue's notion of the instance's overall state. - Break external API requests into "state change" and "component change" requests. This will allow them to be dequeued independently when the state driver wants to handle component changes without changing the order of state change requests. - Tweak the language the state driver uses to communicate with the queue: instead of saying "the VM is now in state X," the driver says "I handled (or failed to handle) a request of type Y." Tests: cargo test, PHD.
1 parent 9ee2e1f commit 7cd7c88

File tree

6 files changed

+1115
-395
lines changed

6 files changed

+1115
-395
lines changed

Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin/propolis-server/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ ring.workspace = true
7373
slog = { workspace = true, features = [ "max_level_trace", "release_max_level_debug" ] }
7474
expectorate.workspace = true
7575
mockall.workspace = true
76+
proptest.workspace = true
7677

7778
[features]
7879
default = []
@@ -83,7 +84,6 @@ omicron-build = ["propolis/omicron-build"]
8384

8485
# Falcon builds require corresponding bits turned on in the dependency libs
8586
falcon = ["propolis/falcon"]
86-
8787
# Testing necessitates injecting failures which should hopefully be rare or even
8888
# never occur on real otherwise-unperturbed systems. We conditionally compile
8989
# code supporting failure injection to avoid the risk of somehow injecting

bin/propolis-server/src/lib/vm/active.rs

+6-9
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ impl ActiveVm {
6363

6464
self.state_driver_queue
6565
.queue_external_request(match requested {
66-
InstanceStateRequested::Run => ExternalRequest::Start,
67-
InstanceStateRequested::Stop => ExternalRequest::Stop,
68-
InstanceStateRequested::Reboot => ExternalRequest::Reboot,
66+
InstanceStateRequested::Run => ExternalRequest::start(),
67+
InstanceStateRequested::Stop => ExternalRequest::stop(),
68+
InstanceStateRequested::Reboot => ExternalRequest::reboot(),
6969
})
7070
.map_err(Into::into)
7171
}
@@ -79,10 +79,7 @@ impl ActiveVm {
7979
websock: dropshot::WebsocketConnection,
8080
) -> Result<(), VmError> {
8181
Ok(self.state_driver_queue.queue_external_request(
82-
ExternalRequest::MigrateAsSource {
83-
migration_id,
84-
websock: websock.into(),
85-
},
82+
ExternalRequest::migrate_as_source(migration_id, websock),
8683
)?)
8784
}
8885

@@ -107,11 +104,11 @@ impl ActiveVm {
107104
) -> Result<(), VmError> {
108105
self.state_driver_queue
109106
.queue_external_request(
110-
ExternalRequest::ReconfigureCrucibleVolume {
107+
ExternalRequest::reconfigure_crucible_volume(
111108
backend_id,
112109
new_vcr_json,
113110
result_tx,
114-
},
111+
),
115112
)
116113
.map_err(Into::into)
117114
}

0 commit comments

Comments
 (0)