FrankChen021 commented on code in PR #19624:
URL: https://github.com/apache/druid/pull/19624#discussion_r3466974530
##########
server/src/main/java/org/apache/druid/server/ServerManager.java:
##########
@@ -639,6 +639,7 @@ public Sequence<T> run(QueryPlus<T> queryPlus,
ResponseContext responseContext)
segmentsBundle,
segmentMapFn
);
+
queryPlus.getQueryMetrics().reportLocalSegmentCount(segmentReferences.size()).emit(emitter);
Review Comment:
[P2] Register segment references before emitting the new metric
This emits after getOrLoadBundleSegments returns but before
closer.registerAll(segmentReferences). If reportLocalSegmentCount or the
emitter throws, the catch block closes only the still-empty closer, so the
acquired SegmentReference objects are leaked. Register the references with the
closer before emitting this metric, or otherwise guarantee the references are
closed on emission failure.
##########
docs/operations/metrics.md:
##########
@@ -140,6 +141,7 @@ to represent the task ID are deprecated and will be removed
in a future release.
|Metric|Description|Dimensions|Normal value|
|------|-----------|----------|------------|
|`query/time`|Milliseconds taken to complete a query.|<p>Common: `dataSource`,
`type`, `interval`, `hasFilters`, `duration`, `context`, `remoteAddress`, `id`,
`statusCode`.</p><p> Aggregation Queries: `numMetrics`,
`numComplexMetrics`.</p><p> GroupBy: `numDimensions`.</p><p> TopN: `threshold`,
`dimension`.</p>|< 1s|
+|`query/segment/count`|Number of segments this Peon scans for a query. This is
local to this data node; the Broker metric `query/segments/count` reports the
distributed query plan.|<p>Common: `dataSource`, `type`, `interval`,
`hasFilters`, `duration`, `context`, `remoteAddress`, `id`.</p><p> Aggregation
Queries: `numMetrics`, `numComplexMetrics`.</p><p> GroupBy: `numDimensions`.
</p><p>TopN: `threshold`, `dimension`.</p>|Varies|
Review Comment:
[P2] Peon metric is documented but not emitted
The PR documents query/segment/count for Peons, but the only new emission is
in ServerManager. Peon/realtime queries route through PeonAppenderatorsManager
or UnifiedIndexerAppenderatorsManager into SinkQuerySegmentWalker, which still
only emits segment time, wait time, and segmentAndCache time. As a result the
new metric is absent for Real-time/Peon data nodes despite the docs and emitter
allowlists; add equivalent emission in the sink walker or remove the Peon entry.
--
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]
