snleee commented on code in PR #9249:
URL: https://github.com/apache/pinot/pull/9249#discussion_r950487153
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeConsumptionRateManager.java:
##########
@@ -131,4 +144,22 @@ public void throttle(int numMsgs) {
return _rate;
}
}
+
+ @VisibleForTesting
+ @FunctionalInterface
+ interface PartitionCountFetcher {
+ Integer fetch(StreamConfig streamConfig);
+ }
+
+ @VisibleForTesting
+ static final PartitionCountFetcher DEFAULT_PARTITION_COUNT_FETCHER =
streamConfig -> {
+ String clientId = streamConfig.getTopicName() +
"-consumption.rate.manager";
+ StreamConsumerFactory factory =
StreamConsumerFactoryProvider.create(streamConfig);
+ try (StreamMetadataProvider streamMetadataProvider =
factory.createStreamMetadataProvider(clientId)) {
+ return
streamMetadataProvider.fetchPartitionCount(/*maxWaitTimeMs*/10_000);
+ } catch (Exception e) {
+ LOGGER.warn("Error fetching metadata for topic " +
streamConfig.getTopicName(), e);
Review Comment:
Do we have any metrics for Kafka connection failure on the server side
consuming segment or the controller side?
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeConsumptionRateManager.java:
##########
@@ -85,17 +88,27 @@ public ConsumptionRateLimiter
createRateLimiter(StreamConfig streamConfig, Strin
return new RateLimiterImpl(partitionRateLimit);
}
- private static LoadingCache<StreamConfig, Integer> buildCache() {
- return
CacheBuilder.newBuilder().expireAfterWrite(CACHE_ENTRY_EXPIRATION_TIME_IN_MINUTES,
TimeUnit.MINUTES)
+ @VisibleForTesting
+ static LoadingCache<StreamConfig, Integer> buildCache(PartitionCountFetcher
partitionCountFetcher,
+ long duration, TimeUnit unit) {
+ return CacheBuilder.newBuilder().refreshAfterWrite(duration, unit)
.build(new CacheLoader<StreamConfig, Integer>() {
@Override
- public Integer load(StreamConfig streamConfig)
+ public Integer load(StreamConfig key)
Review Comment:
Why don't we use the table name as the key instead of the entire
`streamConfig`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
