Validate tick_duration millisecond resolution
#178
Conversation
src/builder.rs
Outdated
| @@ -193,6 +193,11 @@ impl Builder { | |||
| panic!("Maximum message latency must be greater than minimum."); | |||
| } | |||
|
|
|||
| if self.config.tick.as_nanos() % Duration::from_millis(1).as_nanos() != 0 { | |||
| // Tick duration is used for tokio::time::sleep, which requires millisecond resolution. | |||
| panic!("Tick duration resolution is in milliseconds, but value provided would require higher.") | |||
There was a problem hiding this comment.
Consider adding the provided value in the panic message to aid in debugging.
| @@ -193,6 +193,11 @@ impl Builder { | |||
| panic!("Maximum message latency must be greater than minimum."); | |||
| } | |||
|
|
|||
| if self.config.tick.as_nanos() % Duration::from_millis(1).as_nanos() != 0 { | |||
There was a problem hiding this comment.
Would it be simpler to just check < Duration::from_millis(1)?
There was a problem hiding this comment.
I think someone could provide a value that was > 1 ms, but had a higher resolution, e.g. Duration::from_micros(1500), which I believe we would want to disallow still given that comment on tokio::time::sleep.
Though, we might want to disallow Duration::ZERO in addition I suppose?
There was a problem hiding this comment.
I don't think it needs to be this restrictive. Given that the runtime is paused we aren't actually running for the duration, we are simply advancing time by it.
Duration::ZERO is a good callout, and I'm not actually sure what would happen in that case. My guess is that nothing runs.
Hi! I noticed some strange behavior while running a simulation where the tick duration was sub-millisecond, and discovered that
tokio::time::sleepapparently uses millisecond resolution.I added some validation to the builder, to panic whenever a duration with higher than ms resolution is supplied, since it looked like some other validation was already happening here. Let me know if this seems useful.
Love the project btw!