Fix slow consumergroup reconciliation under load #3293
Conversation
In [1], we return an error, however, that method is called in the consumer group reconciler when a consumer is not ready which is a normal state at the beginning when consumers have been just created, so we shouldn't return an error because that causes the consumer group to be reconciled again with an exponentially increasing delay causing slow time to ready. This is especially evident when scaling up with a high load = (therefore when dispatcher pod is slow to become ready). [1] https://github.com/knative-sandbox/eventing-kafka-broker/blob/5cda5463aa2fa060179674fe7b3237abb836ee06/control-plane/pkg/apis/internals/kafka/eventing/v1alpha1/consumer_group_lifecycle.go#L57-L65 Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
|
/cc @matzew |
knative-prow
bot
added
size/XS
Codecov Report
@@ Coverage Diff @@
## main #3293 +/- ##
============================================
+ Coverage 61.78% 61.80% +0.02%
- Complexity 764 766 +2
============================================
Files 181 181
Lines 12220 12223 +3
Branches 266 266
============================================
+ Hits 7550 7555 +5
Misses 4081 4081
+ Partials 589 587 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/test channel-integration-tests-ssl |
| // It is "normal" to have non-ready consumers, and we will get notified when their status change, | ||
| // so we don't need to return an error here which causes the object to be queued with an | ||
| // exponentially increasing delay. | ||
| return nil |
There was a problem hiding this comment.
thanks for the explicit comment.
It seems reasonable
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matzew, pierDipi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
|
/test channel-integration-tests-ssl |
1 similar comment
|
/test channel-integration-tests-ssl |
|
/test channel-integration-tests-sasl-ssl |
knative-prow
bot
merged commit 887cad0
into
knative-extensions:main
|
/cherry-pick release-1.11 |
|
/cherry-pick release-1.10 |
|
@pierDipi: new pull request created: #3298 In 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/test-infra repository. |
|
@pierDipi: new pull request created: #3299 In 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/test-infra repository. |
In [1], we return an error, however, that method is called in the consumer group reconciler when a consumer is not ready which is a normal state at the beginning when consumers have been just created, so we shouldn't return an error because that causes the consumer group to be reconciled again with an exponentially increasing delay causing slow time to ready.
This is especially evident when scaling up with a high load = (therefore when dispatcher pod is slow to become ready).
[1] https://github.com/knative-sandbox/eventing-kafka-broker/blob/5cda5463aa2fa060179674fe7b3237abb836ee06/control-plane/pkg/apis/internals/kafka/eventing/v1alpha1/consumer_group_lifecycle.go#L57-L65
Fixes #3046
Proposed Changes
Release Note
Docs