runtime/session_info: avoid heavy loop when introduced on a live chain #2099
Conversation
ordian
added
A2-insubstantial
| @@ -253,4 +256,26 @@ mod tests { | |||
| assert_eq!(session.needed_approvals, 42); | |||
| }) | |||
| } | |||
|
|
|||
| #[test] | |||
| fn session_pruning_avoids_heavy_loop() { | |||
There was a problem hiding this comment.
I can't figure out how this test ensures that we don't call Sessions::remove for every n < start.
There was a problem hiding this comment.
My laptop froze when running this test without the fix (on 1046e95).
It calls SessionInfo::initializer_on_new_session(¬ification) on a very high session index.
There was a problem hiding this comment.
Maybe relying on a cargo test timeout is not the most elegant way, but this test shouldn't normally fail.
There was a problem hiding this comment.
Maybe we could add an internal timeout with a few seconds' max time? Like wrap the whole closure in async_std::task::block_on(async move { ... }.timeout(Duration::from_secs(5)), using subsystem_util::TimeoutExt. It would mean adding async_std as a dev-dependency, but I don't think that should be a huge deal.
ordian
added
B0-silent
#2099) * session_info: a heavy loop test * session_info: fix a typo * session_info: fix heavy loop * session_info: crank the iterations all the way up
#2099) * session_info: a heavy loop test * session_info: fix a typo * session_info: fix heavy loop * session_info: crank the iterations all the way up
Addresses part 2 of #2093: