Skip to content

Commit

Permalink
Fix slow consumergroup reconciliation under load
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 and knative-prow-robot committed Aug 22, 2023
1 parent e1b49ad commit 98cc914
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

Check warning on line 67 in control-plane/pkg/apis/internals/kafka/eventing/v1alpha1/consumer_group_lifecycle.go

View check run for this annotation

Codecov / codecov/patch

control-plane/pkg/apis/internals/kafka/eventing/v1alpha1/consumer_group_lifecycle.go#L64-L67

Added lines #L64 - L67 were not covered by tests
}

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 98cc914

Please sign in to comment.