kafka-ch-dispatcher doesn't reuse outgoing connections

[maschmid]

Describe the bug
Having an MT Broker backed by KafkaChannels, and just a single application sending to the broker, I can see the number of TCP connections in kafka-ch-dispatcher rise with each event sent:

Most of these (around 4000) are connections to broker-filter:

oc rsh -n knative-eventing kafka-ch-dispatcher-8664d75457-sfcgg
cat /proc/net/tcp | grep "AF321EAC:0050" | wc -l
3959

oc get svc --all-namespaces -o wide   | grep 172.30.50.175
knative-eventing                                   broker-filter                              ClusterIP      172.30.50.175    <none>                                                                   80/TCP,9092/TCP                     39h     eventing.knative.dev/brokerRole=filter

Expected behavior
Outgoing connections should be pooled, with a reasonable maximum of connections to the same address (e.g. 100)

To Reproduce
Send many events to an mt-broker backed by KafkaChannel

Knative release version
0.19.2

Additional context

[slinkydeveloper]

assign

This is weird, it should not happen with knative/eventing#4465, and I see the proper code in 0.19.3 https://github.com/knative-sandbox/eventing-kafka/blob/release-0.19/vendor/knative.dev/eventing/pkg/kncloudevents/http_client.go#L43

[maschmid]

Is ConfigureConnectionArgs invoked anywhere in kafka-ch-dispatcher?

[slinkydeveloper]

I don't think so, but let me check. In any case, are you modifying the map that contains those values?

[maschmid]

OK, I think the problem might be that I have misunderstood the the semantics of defaults. We only set MaxIdleConnsPerHost, not MaxConnsPerHost , so if the connections were all still busy, they are still allowed to grow up for infinity...

[slinkydeveloper]

We only set MaxIdleConnsPerHost, not MaxConnsPerHost , so if the connections were all still busy, they are still allowed to grow up for infinity...

If you set MaxConnsPerHost, does it grows indefinitely?

[maschmid]

MaxConnsPerHost is 0 by default AFAIK, so it doesn't limit it. If we'd set it, it would block during Dial if the limit would be breached...

However, I am still trying to understand the difference between imc-dispatcher and kafka-consolidated-dispatcher.

In IMC, we have https://github.com/knative/eventing/pull/4465/files#diff-793cc23b5cb3cde6c57ad48d4e44b3252912e0e14ea50abb32fbd3904b053846R111 , which configures this from a cm, but I don't think we have the same in kafka-consolidated-dispatcher? (Which means, we just use golang defaults?)

[maschmid]

I think I have a theory on why it this problem occuring in kafka dispatcher, but not imc-dispatcher.

As we don't set anything, we keep the golang defaults, which are

MaxIdleConns = 100
MaxIdleConnsPerHost = 0 , which means "DefaultMaxIdleConnsPerHost" is used per host, which == 2.

This actually causes much more connections to be busy, because with 2 idle connections per host it is hardly re-using any at all, so most of the connections are busy because they need to re-connect all the time.

(With IMC, we default to

MaxIdleConns = 1000
MaxIdleConnectionsPerHost = 100

[slinkydeveloper]

Are you proposing we default to the same defaults of IMC?

[maschmid]

Yes, but it's not just the defaults, it's also the ability to configure those values via a configmap, like we added to IMC dispatcher in https://github.com/knative/eventing/pull/4465/files#diff-793cc23b5cb3cde6c57ad48d4e44b3252912e0e14ea50abb32fbd3904b053846R109-R114

(at least I don't see anything like that in eventing-kafka 0.19)

[slinkydeveloper]

@maschmid in imc dispatcher we removed the ability to configure those values through CM, now they're injected in as env vars knative/eventing#4543

In any case, I'll PR the new defaults

[slinkydeveloper]

/assign