kfaraz commented on code in PR #18709:
URL: https://github.com/apache/druid/pull/18709#discussion_r2488535375
##########
server/src/main/java/org/apache/druid/server/metrics/MonitorsConfig.java:
##########
@@ -79,14 +80,44 @@ public String toString()
'}';
}
- public static Map<String, String[]> mapOfDatasourceAndTaskID(final String
datasource, final String taskId)
Review Comment:
Instead of adding a new method, let's do the following instead:
- use the old method itself to emit the task ID under both the `id`
dimension and `taskId` dimension
- call out the `id` dimension as deprecated in the docs and release notes
and remove it in D37
- update the docs for the existing metrics to indicate that they emit
`taskId` dimension too.
##########
docs/operations/metrics.md:
##########
@@ -131,13 +131,13 @@ Most metric values reset each emission period, as
specified in `druid.monitoring
|`query/failed/count`|Number of failed queries.|This metric is only available
if the `QueryCountStatsMonitor` module is included.||
|`query/interrupted/count`|Number of queries interrupted due to
cancellation.|This metric is only available if the `QueryCountStatsMonitor`
module is included.||
|`query/timeout/count`|Number of timed out queries.|This metric is only
available if the `QueryCountStatsMonitor` module is included.||
-|`mergeBuffer/pendingRequests`|Number of requests waiting to acquire a batch
of buffers from the merge buffer pool.|This metric is only available if the
`GroupByStatsMonitor` module is included.|Should be ideally 0, though a higher
number isn't representative of a problem.|
-|`mergeBuffer/used`|Number of merge buffers used from the merge buffer
pool.|This metric is only available if the `GroupByStatsMonitor` module is
included.|Depends on the number of groupBy queries needing merge buffers.|
-|`mergeBuffer/queries`|Number of groupBy queries that acquired a batch of
buffers from the merge buffer pool.|This metric is only available if the
`GroupByStatsMonitor` module is included.|Depends on the number of groupBy
queries needing merge buffers.|
-|`mergeBuffer/acquisitionTimeNs`|Total time in nanoseconds to acquire merge
buffer for groupBy queries.|This metric is only available if the
`GroupByStatsMonitor` module is included.|Varies|
-|`groupBy/spilledQueries`|Number of groupBy queries that have spilled onto the
disk.|This metric is only available if the `GroupByStatsMonitor` module is
included.|Varies|
-|`groupBy/spilledBytes`|Number of bytes spilled on the disk by the groupBy
queries.|This metric is only available if the `GroupByStatsMonitor` module is
included.|Varies|
-|`groupBy/mergeDictionarySize`|Size of on-heap merge dictionary in bytes.|This
metric is only available if the `GroupByStatsMonitor` module is
included.|Varies|
+|`mergeBuffer/pendingRequests`|Number of requests waiting to acquire a batch
of buffers from the merge buffer pool.|`dataSource`, `taskId`. This metric is
only available if the `GroupByStatsMonitor` module is included.|Should be
ideally 0, though a higher number isn't representative of a problem.|
Review Comment:
same change might be needed in other lines.
##########
docs/operations/metrics.md:
##########
@@ -131,13 +131,13 @@ Most metric values reset each emission period, as
specified in `druid.monitoring
|`query/failed/count`|Number of failed queries.|This metric is only available
if the `QueryCountStatsMonitor` module is included.||
|`query/interrupted/count`|Number of queries interrupted due to
cancellation.|This metric is only available if the `QueryCountStatsMonitor`
module is included.||
|`query/timeout/count`|Number of timed out queries.|This metric is only
available if the `QueryCountStatsMonitor` module is included.||
-|`mergeBuffer/pendingRequests`|Number of requests waiting to acquire a batch
of buffers from the merge buffer pool.|This metric is only available if the
`GroupByStatsMonitor` module is included.|Should be ideally 0, though a higher
number isn't representative of a problem.|
-|`mergeBuffer/used`|Number of merge buffers used from the merge buffer
pool.|This metric is only available if the `GroupByStatsMonitor` module is
included.|Depends on the number of groupBy queries needing merge buffers.|
-|`mergeBuffer/queries`|Number of groupBy queries that acquired a batch of
buffers from the merge buffer pool.|This metric is only available if the
`GroupByStatsMonitor` module is included.|Depends on the number of groupBy
queries needing merge buffers.|
-|`mergeBuffer/acquisitionTimeNs`|Total time in nanoseconds to acquire merge
buffer for groupBy queries.|This metric is only available if the
`GroupByStatsMonitor` module is included.|Varies|
-|`groupBy/spilledQueries`|Number of groupBy queries that have spilled onto the
disk.|This metric is only available if the `GroupByStatsMonitor` module is
included.|Varies|
-|`groupBy/spilledBytes`|Number of bytes spilled on the disk by the groupBy
queries.|This metric is only available if the `GroupByStatsMonitor` module is
included.|Varies|
-|`groupBy/mergeDictionarySize`|Size of on-heap merge dictionary in bytes.|This
metric is only available if the `GroupByStatsMonitor` module is
included.|Varies|
+|`mergeBuffer/pendingRequests`|Number of requests waiting to acquire a batch
of buffers from the merge buffer pool.|`dataSource`, `taskId`. This metric is
only available if the `GroupByStatsMonitor` module is included.|Should be
ideally 0, though a higher number isn't representative of a problem.|
Review Comment:
```suggestion
|`mergeBuffer/pendingRequests`|Number of requests waiting to acquire a batch
of buffers from the merge buffer pool. This metric is only available if the
`GroupByStatsMonitor` module is included.| `dataSource`, `taskId`|Should be
ideally 0, though a higher number isn't representative of a problem.|
```
--
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]
