kona-node actor design simplification #358
Conversation
No functionality changes are intended. This proposal aims to decrease complexity and increase testability. The goal is to consolidate and delete unnecessary code, keeping nearly all existing logic.
| ``` | ||
|
|
||
| #### Clarifications: | ||
| **Cancellation** - Each Actor has a cancellation token that it monitors to signal that the process is shutting down and it should stop processing. This _can_ be treated as a an inbound task but probably shouldn't since it is less of an inbound API task and more of a process-level signal. Suggested handling is demonstrated in the pseudocode example above. |
There was a problem hiding this comment.
If it was only up to me, I'd delegate the actor's cancellation process to the rollup's orchestrator (ie the task that spawns each actor). Cancelling an actor would then amount to returning from the subtask with an error. The parent task will listen for error signals from actors and shut down all the other actors if one fails.
| #### Clarifications: | ||
| **Cancellation** - Each Actor has a cancellation token that it monitors to signal that the process is shutting down and it should stop processing. This _can_ be treated as a an inbound task but probably shouldn't since it is less of an inbound API task and more of a process-level signal. Suggested handling is demonstrated in the pseudocode example above. | ||
|
|
||
| **Sequencing** - `SequencerActor` uses a timer that ticks every `block_time` to trigger it to do some work. This, however, is distinct from inbound messages, as this is SequencerActor's own logic that it runs rather than being triggered by some other party's request. In the case of sequencer actor, its inbound messages (handling admin API requests), do not at all conflict with block building and can likely be handled in parallel. |
There was a problem hiding this comment.
Are you suggesting running the block building logic in a separate task? For the sequencer (and the engine), we probably have two ways to approach the internal actor's design:
- Run the block building/task processing logic in a separate task as the inbound message handler. This would be good in terms of performances, at the cost of higher complexity (the task processing logic would share state with the inbound message handler, meaning internal communication channels within the actor)
- Run everything in a single select! match. Bias the select! to handle block building requests first. Probably the simplest solution. But there's a higher risk of starvation if branch performs a lot of work without yielding.
Wondering if we're not over-optimizing this part of the node, specially knowing that we rarely run the sequencers with RPC enabled (to avoid starving the block-building loop). In that case, the sequencer's inbound message handler becomes a no-op right?
No functionality changes are intended. This proposal aims to decrease complexity and increase testability. The goal is to consolidate and delete unnecessary code, keeping nearly all existing logic.
The motivations for this PR are that