knative-extensions / eventing-kafka

Kafka integrations with Knative Eventing.
Apache License 2.0
77 stars 82 forks source link

Limit single-tenant Kafka source use of resources #1008

Closed aslom closed 2 years ago

aslom commented 2 years ago

Problem A short explanation of the problem, including relevant restrictions.

Today single-tenant Kafka source when running has no resource limits set - effectively taking as much (or as little) CPU as available in Kubernetes running the node.

Persona: Which persona is this feature for?

System Integrator System Operator

Exit Criteria A measurable (binary) test that would indicate that the problem has been resolved.

I have run tests and verified that without resource limits pod will be taking as much resources as it can get.

After fixing the problem we should see that pod resource consumption is limited.

Time Estimate (optional): How many developer-days do you think this may take to resolve?

Additional context (optional) Add any other context about the feature request here.

aslom commented 2 years ago

@lionelvillard WDYT?

lionelvillard commented 2 years ago

autoscaling does not work without limits, so +1.

aslom commented 2 years ago

It seems that using the same limits as other KnE components would make sense such as IMC dispatcher:

https://github.com/knative/eventing/blob/c820f4239730dacf5b2e0e5eed67a496bdc9b424/pkg/reconciler/inmemorychannel/controller/resources/dispatcher.go#L82

                            Resources: corev1.ResourceRequirements{
                                Requests: corev1.ResourceList{
                                    corev1.ResourceCPU:    resource.MustParse("125m"),
                                    corev1.ResourceMemory: resource.MustParse("64Mi"),
                                },
                                Limits: corev1.ResourceList{
                                    corev1.ResourceCPU:    resource.MustParse("2200m"),
                                    corev1.ResourceMemory: resource.MustParse("2048Mi"),
                                },
                            },