Exclude basic-authorship-proposer from the continuous tasks alert #7484
Conversation
tomaka
added
A0-please_review
There was a problem hiding this comment.
I am in favor of excluding specific alerts instead of including specific ones which I initially proposed in #7250.
The worst that can happen when excluding tasks it that one gets a false positive alert. The worst that can happen when including specifc tasks is a missed alert. I favor the former.
|
From a higher level perspective I see this alert as a good hack, but not as a long term (/perfect) solution. In my eyes, when a long running essential task stops, the whole node should gracefully shutdown. That would eventually trigger the standard |
We do indeed have this concept of "essential task" in the code already, but it's not actually used except by GrandPa I believe. |
|
bot merge |
|
Waiting for commit status. |
|
Checks failed; merge aborted. |
|
bot merge |
|
Waiting for commit status. |
|
Checks failed; merge aborted. |
|
CI was failing because of an ICE during the Rust code docs generation. Clearly not related to this PR. |
|
I wonder whether CI still deploys the altered alerting rule to our Prometheus server, even on test failures. Let's check back in a couple of minutes. |
|
For the record, the update is now deployed on our Prometheus server. |
#7328 introduced a new task named
basic-authorship-proposer.This task is spawned whenever a slot is ready for the node to create a slot, and ends once the block has been created.
Unfortunately in #7250 we made the assumption that a task that starts once, then ends once, then doesn't start again for 10 minutes, was actually probably supposed either to not end or to start again, and thus we raise an alert.
This means that if more than 10 minutes pass between the first time a node has a slot available to it and the second time a node has a slot available to it, an alert will be triggered.
In this PR I propose to special-case
basic-authorship-proposeras a task that doesn't trigger this alert.