xiangfu0 commented on code in PR #19872:
URL: https://github.com/apache/pinot/pull/19872#discussion_r4278069102
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -49,9 +49,7 @@ private CommonConstants() {
public static final String CONFIG_OF_BROKER_EVENT_LISTENER_CLASS_NAME =
"factory.className";
public static final String CONFIG_OF_REQUEST_CONTEXT_TRACKED_HEADER_KEYS =
"request.context.tracked.header.keys";
public static final String DEFAULT_METRICS_FACTORY_CLASS_NAME =
- //"org.apache.pinot.plugin.metrics.compound.CompoundPinotMetricsFactory";
- "org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory";
- //"org.apache.pinot.plugin.metrics.dropwizard.DropwizardMetricsFactory";
+ "org.apache.pinot.plugin.metrics.dropwizard.DropwizardMetricsFactory";
Review Comment:
Addressed in
[f8533e7e49](https://github.com/apache/pinot/commit/f8533e7e493bbae65439bd73de61bad9fdee5a14).
Timer creation now uses `MetricRegistry.timer(name)`, whose default
exponentially decaying reservoir is bounded at 1,028 samples with alpha 0.015,
matching Yammer's sampling parameters.
Added a regression check for 10,000 updates: the full count is preserved and
retained samples stay within the bound. A before/after probe confirmed 10,000
retained samples before the fix and 1,028 afterward.
All 1,511 focused tests and the four required precommit checks passed. The
real JMX Java-agent scrape smoke also passed with the existing Prometheus
names, labels, and millisecond timer values.
--
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]
