Add timeout to endpointset metric collector #7336
Conversation
We have seen deadlocks with endpoint discovery caused by the metric collector hanging and not releasing the store labels lock. This causes the endpoint update to hang, which also makes all endpoint readers hang on acquiring a read lock for the resolved endpoints slice. This commit makes sure the Collect method on the metrics collector has a built in timeout to guard against cases where an upstream call never reads from the collection channel. Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
fpetkovski
force-pushed
the
endpoint-collect-timeout
branch
from
09d688d to
cad8f93
Compare
| @@ -277,7 +279,12 @@ func (c *endpointSetNodeCollector) Collect(ch chan<- prometheus.Metric) { | |||
| lbls = append(lbls, storeTypeStr) | |||
| } | |||
| } | |||
| ch <- prometheus.MustNewConstMetric(c.connectionsDesc, prometheus.GaugeValue, float64(occurrences), lbls...) | |||
| select { | |||
| case ch <- prometheus.MustNewConstMetric(c.connectionsDesc, prometheus.GaugeValue, float64(occurrences), lbls...): | |||
There was a problem hiding this comment.
Why this can take even 1 second? 🤔
There was a problem hiding this comment.
I don't yet understand why the send would block forever here, the only explanation is that the caller does not read from the channel. The 1s timeout is arbitrary though, it's there to make sure we do not end up in a deadlock.
We noticed queriers getting stuck occasionally and the goroutine profile showed that the mutex in this function was constantly locked.
There was a problem hiding this comment.
I think something else might be at play here, e.g., this goroutine is starved of CPU resources. Do you have a lot of goroutines running when this happens?
There was a problem hiding this comment.
|
I will merge this for now and we can dig into the root cause once we get some time. |
We have seen deadlocks with endpoint discovery caused by the metric collector hanging and not releasing the store labels lock. This causes the endpoint update to hang, which also makes all endpoint readers hang on acquiring a read lock for the resolved endpoints slice.
This commit makes sure the Collect method on the metrics collector has a built in timeout to guard against cases where an upstream call never reads from the collection channel.
Changes
Verification