sim-all: Allow running the simulator without it writing on disk #146
Conversation
|
Could we override |
|
Do you mean using zero as a special case? That could work but it may also introduce potential foot shots. Using a special, non documented, flag means that you need to be pretty aware of what you're doing. This makes me wonder though, what would happen currently if we set that to zero? |
I looked at this and zero means no flushing, meaning that the data will only be written to disk when the simulation ends. Should we discourage that? It feels like we should not allow data to pile in memory. |
I mean if the flag is not set, we don't start the report writer process. If the flag is set to any value including zero, we start the report writer |
yeah we could restrict min value to 1 for the flag |
Well that's not really how it should be, should it? If the flag is not set, we default to whatever we consider to be a reasonable batch size, which currently is 500. The flag only signals whether we want to be less disk I/O intensive. Zero, on the other side, could mean do not write to disk, but I think that has risky side-effects as previously mentioned.
Agreed |
|
I've added the |
sr-gi
force-pushed
the
20231018-no-results
branch
from
aab8df4 to
5a1b073
Compare
Ack. okay with me if we don't overload |
sr-gi
force-pushed
the
20231018-no-results
branch
2 times, most recently
from
365746b to
acce1e1
Compare
sr-gi
force-pushed
the
20231018-no-results
branch
2 times, most recently
from
97de166 to
fee45be
Compare
|
Rebased and squashed |
sr-gi
force-pushed
the
20231018-no-results
branch
2 times, most recently
from
1bd098f to
3c34db8
Compare
sim-lib/src/lib.rs
Outdated
| loop { | ||
| tokio::select! { | ||
| biased; | ||
| _ = listener.clone() => { | ||
| log::debug!("Simulation results consumer received shutdown signal."); | ||
| break writer.flush().map_err(|_| SimulationError::FileError) | ||
| return writer.map(|ref mut w| w.flush().map_err(|_| SimulationError::FileError)).unwrap_or(Ok(())); |
There was a problem hiding this comment.
Not sure if this is canonical, but is map_or a good fit here?
writer.map_or(Ok(()), |mut w| w.flush().map_err(|_| SimulationError::FileError))
Non-blocking, just 🦀 nit.
sim-lib/src/lib.rs
Outdated
| }, | ||
| None => { | ||
| break writer.flush().map_err(|_| SimulationError::FileError) | ||
| return writer.map(|ref mut w| w.flush().map_err(|_| SimulationError::FileError)).unwrap_or(Ok(())); | ||
|
|
There was a problem hiding this comment.
Unnecessary newline (curses on select and its interference with fmt).
sr-gi
force-pushed
the
20231018-no-results
branch
3 times, most recently
from
6c36b58 to
ff48db3
Compare
It is rather annoying to run the simulator and having to delete the output files when we are doing testing/dev. This allows us to do so without the file being created.
sr-gi
force-pushed
the
20231018-no-results
branch
from
ff48db3 to
cf92f52
Compare
|
Addressed nits |
It is rather annoying to run the simulator and have to delete the output files when we are doing testing/dev. This allows us to do so without the file being created.