Jackie-Jiang commented on code in PR #16339:
URL: https://github.com/apache/pinot/pull/16339#discussion_r2205895697
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeConsumptionRateManager.java:
##########
@@ -268,4 +306,96 @@ int emitMetric(int numMsgsConsumed, double rateLimit,
Instant now) {
return ratioPercentage;
}
}
+
+ interface MetricEmitter {
Review Comment:
I don't think we need this interface. `emitMetric()` is no longer emitting
metric for async emitter, and 2 arguments are ignored, so this abstraction is
not clear.
We can just keep 2 separate implementation, one non-thread-safe and one
thread-safe
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeConsumptionRateManager.java:
##########
@@ -59,6 +64,7 @@ public class RealtimeConsumptionRateManager {
private static final String SERVER_CONSUMPTION_RATE_METRIC_KEY_NAME =
ServerMeter.REALTIME_ROWS_CONSUMED.getMeterName();
+ private static final AtomicReference<AsyncMetricEmitter>
ASYNC_METRIC_EMITTER_REF = new AtomicReference<>();
Review Comment:
Should we make a thread-safe version of `ConsumptionRateLimiter` instead of
just the emitter? I think guava `RateLimiter` itself is thread-safe
--
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
