Improve warning about notifications queue and remove spurious triggers #5512
Conversation
| ); | ||
| } | ||
| if let Some(metric) = &self.queue_size_report { | ||
| metric.observe(substream.queue_len() as f64); |
There was a problem hiding this comment.
In regards to observing even though the message is dropped: If we only observe when the message was actually sent, the histogram count metric would tell us how many messages went into the channel. Later on we could add an additional metric (a simple counter) to count the messages being dropped. What do you think @tomaka? Would that enable us to deeper understand the behavior?
There was a problem hiding this comment.
Totally fine if you see this more as a hack than a useful feature.
There was a problem hiding this comment.
The way I see it, dropping messages is something that is really not supposed to happen, and knowing how many messages are dropped isn't necessarily useful, as we have to treat 1 dropped message the same as 500 dropped messages.
Additionally #5481 would consist in reworking this code anyway.
Supercedes #5500
Improves the logging for the queue being full, and fixes the fact that the warning is being printed when we fail to immediately flush the queue.
We now only push the element, and flushing is handled by the
pollfunction.This is how this code was originally intended to behave, and the fact that we were immediately trying to flush the queue was not intended.
We also now report the queue size to Prometheus even when it's full.