It's possible to load a torn blueprint / sitrep / inventory

[smklein]

Original context: #9552 (comment) - specifically, the issue @hawkw mentioned in #9552 (comment)

The implementation of the "blueprint loader" and the "sitrep loader" both use the following approach:

• Identify the target (by UUID)
• Load that target {blueprint, sitrep} by querying individual tables
• Return the in-memory structure, constructed from the underlying tables

This setup does not play nice with concurrent deletion:

• If we start loading target foobar
• ... and the target is changed to something else, and foobar is deleted
• ... then the "loading" will happen concurrently with foobar being deleted

This means: It'll be possible to read a {blueprint, sitrep} with any number of rows missing.

In practice, we are not deleting these objects frequently enough to hit this, but if we had a "laggy Nexus", or a rapid turnaround for deletion, this seems possible.

[smklein]

As identified in the comment I linked above, there is a (currently) misleading comment in blueprint_read about this circumstance:

omicron/nexus/db-queries/src/db/datastore/deployment.rs

Lines 2140 to 2145 in 0c0bdf4

	// The blueprint for the current target cannot be deleted while it is
	// the current target, but it's possible someone else (a) made a new
	// blueprint the target and (b) deleted the blueprint pointed to by our
	// `target` between the above query and the below query. In such a case,
	// this query will fail with an "unknown blueprint ID" error. This
	// should be rare in practice.

It's possible that the blueprint target change + blueprint deletion happens in the body of blueprint_read, which would not return an "unknown blueprint ID" necessarily. It could just result in a torn blueprint, with many rows of data missing.

As a reasonable fix: We should consider making "load blueprint" and "delete blueprint" transactions.

Blueprints are inherently immutable, so the only time we would experience contention would be this case, where we are actually doing a concurrent read and write simultaneously -- there should otherwise be minimal impact on the common case, where we're "just doing reads" or "just doing a delete, but no one is reading".

[smklein]

This issue also exists for inventory collections: Although inventory_delete_collection uses a transaction, inventory_collection_read_batched does not. This means we could be part-way through reading an inventory collection, and it could be concurrently deleted from underneath the Nexus which is trying to read it.

This is also a scenario where "wrapping read in a transaction" seems like it should be fine; it should be reading data which is immutable, except for this concurrent deletion case.

[smklein]

Discussed in the 1/6/2026 Update Sync.

The transactional approach doesn't always avoid contention - it's possible that if loading rows from the database don't "SELECT ... WHERE blueprint_id = ", but rather, the loads perform "SELECT ... WHERE ... IN [ ]", it's possible that concurrent INSERT operations could be blocked by a concurrent transactional read.

Caveats: This is theorizing, I haven't tested it, and haven't really confirmed that these queries exist specifically in the loading of blueprints / inventory (though @jgallagher mentioned HW baseboard loading doing something along these lines).

The other strategy, of "load top-level metadata row, load all inner rows, load top-level metadata row again to check for a torn read" should also work, as an alternative variant here.

[hawkw]

The transactional approach doesn't always avoid contention - it's possible that if loading rows from the database don't "SELECT ... WHERE blueprint_id = ", but rather, the loads perform "SELECT ... WHERE ... IN [ ]", it's possible that concurrent INSERT operations could be blocked by a concurrent transactional read.

FWIW, the sitrep-loading code never does WHERE ... IN [] queries, but I'm not sure about inventory/blueprint.

The other strategy, of "load top-level metadata row, load all inner rows, load top-level metadata row again to check for a torn read" should also work, as an alternative variant here.

The downside of this (which I'm calling the "lock-free" strategy) is, as we discussed yesterday, that the in-memory representation based on the potentially torn read is still constructed before being discarded. This is only a problem if however we represent these structures in memory can't cope with unexpectedly missing data. For example, if we read a bunch of records from some table A which have a foreign key referencing records in some table B within the blueprinventorep,¹ and then expect that a map we already constructed of the records from table B to contain entries for those records, we might be unpleasantly surprised if that blueprinventorep's records had already been deleted from table B when it was loaded. If we expect(...) lookups from the map of table B records, that would be pretty sad. On the other hand, we probably should just either ignore the record whose corresponding record doesn't exist, or return an error in that case (and I've tried to make sure all the sitrep-loading code does that).

Footnotes

1. Should we be trying to come up with a name for this broad class of "immutable datastructure which we store across a bunch of tables in CRDB in a semi-formalized way"? ↩

[davepacheco]

As identified in the comment I linked above, there is a (currently) misleading comment in blueprint_read about this circumstance:

omicron/nexus/db-queries/src/db/datastore/deployment.rs

Lines 2140 to 2145 in 0c0bdf4
// The blueprint for the current target cannot be deleted while it is
// the current target, but it's possible someone else (a) made a new
// blueprint the target and (b) deleted the blueprint pointed to by our
// target between the above query and the below query. In such a case,
// this query will fail with an "unknown blueprint ID" error. This
// should be rare in practice.

It's possible that the blueprint target change + blueprint deletion happens in the body of blueprint_read, which would not return an "unknown blueprint ID" necessarily. It could just result in a torn blueprint, with many rows of data missing.

Agreed: I think this comment is correct (and describes a different, related problem), but it's not anticipating the behavior that blueprint_read() itself could return a torn blueprint. It assumes (reasonably, I'd say) that blueprint_read either returns a complete blueprint or a NotFound error, never a torn one.

[davepacheco]

Discussed in the 1/6/2026 Update Sync.

The transactional approach doesn't always avoid contention - it's possible that if loading rows from the database don't "SELECT ... WHERE blueprint_id = ", but rather, the loads perform "SELECT ... WHERE ... IN [ ]", it's possible that concurrent INSERT operations could be blocked by a concurrent transactional read.

Possibly related: cockroachdb/docs#7815

Note that this kind of contention is not the only problem with large, long-running transactions. In PostgreSQL, this can lead to bloat. I'm not sure if there are consequences like this in CockroachDB.

The other strategy, of "load top-level metadata row, load all inner rows, load top-level metadata row again to check for a torn read" should also work, as an alternative variant here.

In the case of blueprint_read, since insertion and deletion are transactional, I think it should be sufficient to just load the blueprint row last. There's no need to load it up front. I haven't checked the inventory case.