Skip to content

Commit

Permalink
Move Nexus notification to standalone task (#1584)
Browse files Browse the repository at this point in the history
Previously, we would spawn a new `tokio::task` in the `Downstairs` to do
an async notification to Nexus, iff the `notify-nexus` feature was
enabled. This is awkward for a few reasons:

- Spawning tasks in an otherwise-synchronous function can be surprising
(since it implicitly requires a tokio runtime)
- It requires importing a bunch of Nexus types into the `mod downstairs`
- This code was only compiled if `notify-nexus` is enabled, which means
that it's easy to have something that builds locally but fails in CI
- There's a bunch of duplicate logging, because each function notifies
Nexus separately

This PR moves Nexus notifications to a separate `notify` module, which
runs a single task to do these notifications in one place. It
communicates to the rest of Crucible through a queue; if the queue fills
up, we log and discard the notification request (since this is
best-effort).

All of this code is _always_ compiled; we check the feature (using
`cfg!(feature = "notify-nexus")` to determine whether it's actually
enabled. This module is responsible for translating into Nexus types, so
they don't leak into the rest of the codebase.
  • Loading branch information
mkeeter authored Jan 14, 2025
1 parent 2352a21 commit b10ac22
Show file tree
Hide file tree
Showing 7 changed files with 558 additions and 668 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions upstairs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ path = "src/lib.rs"

[features]
asm = ["usdt/asm"]
notify-nexus = ["nexus-client", "internal-dns", "progenitor-client", "http", "omicron-uuid-kinds"]
notify-nexus = []
integration-tests = []

[dependencies]
Expand Down Expand Up @@ -56,11 +56,9 @@ aes-gcm-siv.workspace = true
rand_chacha.workspace = true
reqwest.workspace = true
crucible-workspace-hack.workspace = true
nexus-client = { workspace = true, optional = true }
internal-dns = { workspace = true, optional = true }
omicron-uuid-kinds = { workspace = true, optional = true }
progenitor-client = { workspace = true, optional = true }
http = { workspace = true, optional = true }
nexus-client.workspace = true
omicron-uuid-kinds.workspace = true
internal-dns.workspace = true

[dev-dependencies]
expectorate.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion upstairs/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1919,7 +1919,7 @@ impl DownstairsClient {
self.client_delay_us.load(Ordering::Relaxed)
}

#[cfg(feature = "notify-nexus")]
/// Looks up the region UUID
pub(crate) fn id(&self) -> Option<Uuid> {
self.region_uuid
}
Expand Down
Loading

0 comments on commit b10ac22

Please sign in to comment.