Skip to content

Commit

Permalink
Fix slow consumergroup reconciliation under load (#3293)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
pierDipi authored Aug 21, 2023
1 parent 21c92e7 commit 887cad0
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ func (cg *ConsumerGroup) MarkReconcileConsumersFailedCondition(condition *apis.C
condition.GetMessage(),
)

return fmt.Errorf("consumers aren't ready, %v: %v", condition.GetReason(), condition.GetMessage())
// 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
}

func (cg *ConsumerGroup) MarkReconcileConsumersSucceeded() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func TestReconcileKind(t *testing.T) {
}, nil
}),
},
WantErr: true,
WantErr: false,
WantCreates: []runtime.Object{
NewConsumer(1,
ConsumerSpec(NewConsumerSpec(
Expand Down Expand Up @@ -465,7 +465,6 @@ func TestReconcileKind(t *testing.T) {
},
WantEvents: []string{
finalizerUpdatedEvent,
"Warning InternalError consumers aren't ready, ConsumerBinding: failed to bind resource to pod: EOF",
},
},
{
Expand Down

0 comments on commit 887cad0

Please sign in to comment.