Add metrics timing message passing from OverseerSubsystemContext to Overseer::route_message#2201
Conversation
This timer only exists / logs a fraction of the time (configurable by `MESSAGE_TIMER_METRIC_CAPTURE_RATE`). When it exists, it tracks the span between the `OverSubsystemContext` receiving the message and its receipt in `Overseer::run`.
This should be more accurate; it ensures that the timer runs at least as long as that function. As `route_message` is async, it may not actually run for some time after it is called (or ever).
The previous impl using `from_entropy` depends on the `getrandom` crate, which uses the system entropy source, and which does not work on `wasm32-unknown-unknown` because it wants to fall back to a JS implementation which we can't assume exists. This impl depends only on `rand::thread_rng`, which has no documentation stating that it's similarly limited.
This deserves a bit of explanation, as the motivating issue explicitly requested randomness. In short, it's hard to get randomness to compile for `wasm32-unknown-unknown` because that is explicitly intended to be as deterministic as practical. Additionally, even though it would never be used for consensus purposes, it still felt offputting to intentionally introduce randomness into a node's operations. Except, it wasn't really random, either: it was a deterministic PRNG varying only in its state, and getting the state to work right for that target would have required initializing from a constant. Given that it was a deterministic sequence anyway, it seemed much simpler and more explicit to simply select one of each N messages instead of attempting any kind of realistic randomness.
|
Promoting the commit message from 0ab8594:
|
coriolinus
added
B0-silent
This partially reverts commit 0ab8594. `oorandom` is much lighter than the previous `rand`-based implementation, which makes this easier to work with. This implementation gives each subsystem and each child RNG a distinct increment, which should ensure they produce distinct streams of values.
|
Note to reviewers: the |
|
bot merge |
|
Missing process info; check that the PR belongs to a project column. Merge can be attempted if:
See https://github.com/paritytech/parity-processbot#faq |
2 participants
Closes #2186.