runtime/control: refactor to pending message queue#259
Conversation
nrc
left a comment
nrc
left a comment
There was a problem hiding this comment.
some very small comments inline, happy to land with or without changes
| && !self.pending.is_empty() | ||
| { | ||
| for req in self.pending.drain(..) { | ||
| match req { |
There was a problem hiding this comment.
style nit: I'd try to refactor this into a send function on the enum
| } | ||
|
|
||
| if let Some(node) = &self.self_node | ||
| && !self.pending.is_empty() |
There was a problem hiding this comment.
nit: I don't think this is necessary since the for loop will just be a no-op
| let node = node.await; | ||
| replier.send(node) | ||
| }); | ||
| self.pending.push(PendingRequest::SelfNode(replier)); |
There was a problem hiding this comment.
These functions seem pretty repetitive and it would be nice to factor out the common 'shape', not sure if that is a good idea though, especially since they're all quite small after this refactor
There was a problem hiding this comment.
It would — I thought about it and started sketching something, but I'm not sure exactly what the shape wants to be. I was going to let it cook mentally for a bit and do some research before trying to extract the essence of it. It feels like a common enough problem that there's probably a clean actor-y way to handle it (that Erlang probably already discovered)
npry
force-pushed
the
npry/rt.control.msgqueue
branch
2 times, most recently
from
eec9960 to
d4ee0f6
Compare
Use a pending-message queue for the IPv4/IPv6/self-node messages rather than the tokio notifier in alignment with other actors that work like this. This also avoids spawning and possibly leaking tokio tasks if the control actor dies, and in the expected case (where we've already seen a self-node), no pending work needs to be enqueued at all. Signed-off-by: Nathan Perry <[email protected]> Change-Id: I0f032c94121fee97b846a4585d89fbee6a6a6964
npry
force-pushed
the
npry/rt.control.msgqueue
branch
from
d4ee0f6 to
24cd6b1
Compare
2 participants
Use a pending-message queue for the IPv4/IPv6/self-node messages rather than the tokio notifier in alignment with other actors that work like this. This also avoids spawning and possibly leaking tokio tasks if the control actor dies, and in the expected case (where we've already seen a self-node), no pending work needs to be enqueued at all (previously this would have unconditionally spawned a task).