always use StorageMonitor, also make StorageMonitor work for MSQ tasks#19048
Conversation
capistrant
left a comment
There was a problem hiding this comment.
code looks good. should we add docs though? especially now that the monitor is always on, I think it makes sense to get the metrics into the metrics table. Perhaps not blocking, I can flip to approve if you'd like to do it as a follow up
|
i think i'd rather do as a follow-up since none of the vsf stuff is documented yet and just get this as part of that. I think its mature enough to start documenting in the next release |
capistrant
left a comment
There was a problem hiding this comment.
I'm on board with a broader follow up for all vsf related docs
| ); | ||
| .manufacturate(new File(toolbox.getIndexingTmpDir(), "segment-fetch"), true); | ||
| final SegmentManager segmentManager = new SegmentManager(cacheManager); | ||
| final StorageMonitor storageMonitor = new StorageMonitor(cacheManager, Map.of("taskId", task.getId(), "groupId", task.getGroupId())); |
There was a problem hiding this comment.
Best to pass in task::getMetricBuilder (a supplier of ServiceMetricEvent.Builder). It includes taskId and groupId, also some other stuff like dataSource.
Makes
StorageMonitor, added in #18742, be always on. The low overhead seems worth the utility. Also added support for MSQ tasks to work correctly with this monitor, emitting extrataskIdandgroupIddimensions.