refactor(l2): use spawned for monitor #3635
Conversation
Lines of code reportTotal lines added: Detailed view |
| task_set.spawn(async move { | ||
| let mut finished = false; | ||
| while !finished { | ||
| let message = ethrex_monitor | ||
| .call(monitor::app::CallInMessage::Finished) | ||
| .await | ||
| .map_err(MonitorError::GenServerError)?; | ||
| if let monitor::app::OutMessage::ShouldQuit(should_quit) = message { | ||
| finished = should_quit; | ||
| } | ||
|
|
||
| tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; | ||
| } | ||
|
|
||
| Ok(()) | ||
| }); |
There was a problem hiding this comment.
Do we need this part? If so, we should consider moving to wait on the CancellationToken of the GenServer, like this:
There was a problem hiding this comment.
The idea is, that after the genserver stops (the user pressed Shift + Q) we want to finish the whole process.
I'll try to implement it with a cancellation token, ideally we should have a way to check if a genserver is running, instead of having the custom ShouldQuit message
| .inspect_err(|err| error!("Monitor Error: {err}")); | ||
| if !state.widget.should_quit { | ||
| send_after( | ||
| Duration::from_millis(1), |
There was a problem hiding this comment.
This should use the tick_rate, right?
There was a problem hiding this comment.
On the monitor function we do this event poll
Duration::from_millis(widget.tick_rate).saturating_sub(widget.last_tick.elapsed());
if !event::poll(timeout)?
So that we are able to both render the widget and process events, so if a long time has passed since the last render, we prioritize rendering over events.
If we sent a message only after tick_rate ms, the timeout will always be near 0, and so no events would be processed
|
|
||
| let timeout = | ||
| Duration::from_millis(widget.tick_rate).saturating_sub(widget.last_tick.elapsed()); | ||
| if !event::poll(timeout)? { |
There was a problem hiding this comment.
This event::poll is blocking. This breaks with the current runtime's requirement for cooperative multitasking. We should either 1) use an async event poll 2) change the genserver to be start_blocking or 3) change the structure so we poll outside the genserver (recommended if possible)
There was a problem hiding this comment.
For the time being i start the genserver with start_blocking
Motivation
Refactor monitor so that it uses spawned crate
Description
Currently the monitor is a tokio task, refactor it to use spawned
How to test
--monitorto theinit-l2-no-metricstarget incrates/l2/Makefile.make restartincrates/l2).make init-proverincrates/l2.make testincrates/l2.Depends on #3622
Closes #3515