mjsax commented on code in PR #17021:
URL: https://github.com/apache/kafka/pull/17021#discussion_r1779165638
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -547,6 +551,8 @@ static KafkaAdminClient createInternal(
MetricsContext metricsContext = new KafkaMetricsContext(JMX_PREFIX,
config.originalsWithPrefix(CommonClientConfigs.METRICS_CONTEXT_PREFIX));
metrics = new Metrics(metricConfig, reporters, time,
metricsContext);
+ clientTelemetryReporter =
CommonClientConfigs.telemetryReporter(clientId, config);
+ clientTelemetryReporter.ifPresent(telemetryReporter ->
telemetryReporter.contextChange(metricsContext));
Review Comment:
Kafka Streams uses the admin client to also send "delete record" request for
repartition topics. For this case, the connection should be long lived, as we
send a "delete record" request after each commit by default (it's actually
configurable).
For the (more rare) case that there is no repartition topic, it could be a
slightly different story, but IIRC, @bbejeck did some tests already, verifying
that the connection is not closed by long lived, even for this case. So I think
we are good?
We could run more test before the 4.0 release to double check, and if we
really find an issue, we could still update the KIP, to let KS use different
connection timeout default, to make sure that the connection will be long lived.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
