Repository navigation
[controller] fix depth metrics - #3253
Conversation
|
/assign @evankanderson @Cali0707 |
knative-prow
Bot
added
the
size/S
knative-prow
Bot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso, evankanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick release-1.19 |
|
@dprotaso: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3253 +/- ##
==========================================
- Coverage 75.09% 75.08% -0.01%
==========================================
Files 187 187
Lines 10243 10244 +1
==========================================
Hits 7692 7692
- Misses 2312 2313 +1
Partials 239 239 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@dprotaso: new pull request created: #3254 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This change replaces the two lane queue with a small wrapper around the controller-runtime priorityqueue. Having a single queue for both guarantees correct deduplication and avoid metrics issues like describes in knative#3253. It is also much simpler.
This change replaces the two lane queue with a small wrapper around the controller-runtime priorityqueue. Having a single queue for both guarantees correct deduplication and avoid metrics issues like describes in knative#3253. It is also much simpler. Signed-off-by: Alvaro Aleman <alvaroaleman@noreply.com>
4 participants
Fixes: #3242
I think the bug occurs when we have something in the consumer work queue and then something is added to the fast queue. We skip the
addtime recording on the second entry (and depth increment) and we end up with multipleGetcalls that decremented the queue depth by too much.Thus right now the depth metric is 'best-effort' since we're not really counting duplicate entries in the queue (that can't be deduped)
To address this we'd probably have to fork the client-go code to better instrument the metrics, or write our own queue implementation