ankitsultana commented on code in PR #11561:
URL: https://github.com/apache/pinot/pull/11561#discussion_r1321678344
##########
pinot-spi/src/main/java/org/apache/pinot/spi/stream/PartitionGroupMetadataFetcher.java:
##########
@@ -63,8 +65,8 @@ public Exception getException() {
@Override
public Boolean call()
throws Exception {
- String clientId = PartitionGroupMetadataFetcher.class.getSimpleName() + "-"
- + _streamConfig.getTableNameWithType() + "-" + _topicName;
+ String clientId = PartitionGroupMetadataFetcher.class.getSimpleName() +
"-" + _topicName + "-try" + Integer
+ .toString(++_callRetries);
Review Comment:
1. This is adding Kafka client specific code to
`PartitionGroupMetadataFetcher` which is agnostic of the stream technology used.
2. If we have consumers for multiple kafka partitions on the same host then
all of them would get the same clientId? Not that the duplicate clientId
related MBean exceptions only get logged and don't throw, so the consumer still
gets created though it is missing MBeans which could impact monitoring.
3. Do we even need this? The `AppInfoParser.registerAppInfo` call is the
*last* call made in the `KafkaConsumer` constructor.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
