sled-agent: Move IPCC calls off of tokio worker threads?

[jgallagher]

In #9720, we encountered an OS bug that caused IPCC ioctls to hang indefinitely. In sled-agent, we call these directly from tokio worker threads, which led to several worker threads getting stuck, and eventually hitting #9619 where the entire runtime blocked (even though some worker threads were still parked / idle). #9619 proposes we implement the general workaround where we periodically spawn a new task into the runtime, which will unstick a runtime stuck because the one thread responsible for I/O is blocked polling the future that caused it to wake up. However, it seems unlikely this would have helped much in the #9720 case - we probably would have only delayed sled-agent hanging, because eventually we would have issued enough IPCC calls to hang all the worker threads.

I'm inclined to say we should treat IPCC calls as "blocking I/O" calls - that seems pretty accurate, since we're doing I/O over a uart to the SP (and/or RoT, depending on the IPCC command) - and put them in spawn_blocking. But I'm not sure what that would do in a case like #9720 - if every IPCC call hangs, would we eventually exhaust the spawn_blocking pool? Presumably sled-agent would remain generally responsive (except in paths that depended on those IPCC calls?), but what would happen in the limit?

[andrewjstone]

I'm not sure how viable this is. It is likely possible, but it should likely be done inside sprockets, since that's where the IPCC calls occur. This all happens inside a trait implementation used by rustls. I did wonder about how much of an isssue this would be early on, but also I reasoned that tokio was a work stealing scheduler and so only one thread would be blocked.

The failure modes here are utterly gross and we should probably just fix tokio when we can.

[jgallagher]

The failure modes here are utterly gross and we should probably just fix tokio when we can.

I don't think there's a tokio fix here. We already had 7 tokio workers hung before the runtime as a whole hang; wouldn't we have eventually hung all the worker threads anyway?

[andrewjstone]

The failure modes here are utterly gross and we should probably just fix tokio when we can.

I don't think there's a tokio fix here. We already had 7 tokio workers hung before the runtime as a whole hang; wouldn't we have eventually hung all the worker threads anyway?

You are right. I misread your post. We would need at least 32 workers so that we can have 31 ipcc connections blocked for concurrent connections ongoing.

If we fix this host bug then we may delay everything by the length of the IPCC call if it ends up on the IO thread, which is pretty bad also, but obviously not catastrophic. In that case a fix to tokio would do what we want.

However, presumably we want to guard against other bugs in IPCC. That's totally reasonable, but has to be balanced against existing work and testing.

[andrewjstone]

Thinking about this a bit more, I wonder if a better solution would be to put all IPCC requests on a queue and have single thread issue them. That would only block that one thread. We could even put it outside of tokio and have it owned by sprockets.

[hawkw]

Thinking about this a bit more, I wonder if a better solution would be to put all IPCC requests on a queue and have single thread issue them. That would only block that one thread. We could even put it outside of tokio and have it owned by sprockets.

IMO, this seems like the right move, especially because it gives us an easy way to shed load instead of using unbounded RAM in the case that we have an IPCC call that's hung forever.

[jgallagher]

Seems reasonable, although I'm less sure about it being owned by sprockets. Are all the other IPCC uses from the OS and outside sled-agent entirely? Grepping through omicron looks like maybe that's true - the only other caller is installinator, which I guess isn't really relevant here.

[andrewjstone]

Seems reasonable, although I'm less sure about it being owned by sprockets. Are all the other IPCC uses from the OS and outside sled-agent entirely? Grepping through omicron looks like maybe that's true - the only other caller is installinator, which I guess isn't really relevant here.

Sorry if I wasn't clear. I meant that all the sprockets specific requests for IPCC usage would be queued themselves by sprockets. Other requests outside sprockets can do their own thing. This allows trust quorum related cancellation and removal from the queue to be separate from anything else.

I suppose we could have a general purpose mechanism, but that seem harder, especially since IPCC can be used by multiple processes in theory.

[jgallagher]

I suppose we could have a general purpose mechanism, but that seem harder, especially since IPCC can be used by multiple processes in theory.

The kernel already serializes all requests anyway (whether those are from multiple threads or multiple processes). I think the concern here is just if within a process we're going to make a bunch of (potentially concurrent) requests, which yeah seems like just a localized "sprockets serializes TQ-related requests" would be fine.