minor: Add segment/allocated/count metric#19674
Conversation
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 2 |
| P2 | 0 |
| P3 | 1 |
| Total | 3 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 2 |
| P2 | 0 |
| P3 | 1 |
| Total | 3 |
Reviewed 3 of 3 changed files.
This is an automated review by Codex GPT-5.6-Sol
| identifier = tryAllocateSubsequentSegment(toolbox, task, rowInterval, usedSegmentsForRow.iterator().next()); | ||
| } | ||
| if (identifier != null) { | ||
| emitSuccessMetric(identifier, task, toolbox); |
There was a problem hiding this comment.
[P1] Emit the metric for batched allocations
LocalTaskActionClient selects performAsync whenever batching is enabled, and TaskLockConfig enables batching by default. That path completes directly through SegmentAllocationQueue, so this new call in perform is bypassed and the metric is absent under the default configuration.
| { | ||
| final ServiceMetricEvent.Builder metricBuilder = new ServiceMetricEvent.Builder(); | ||
| IndexTaskUtils.setTaskDimensions(metricBuilder, task); | ||
| if (task instanceof SeekableStreamIndexTask<?,?,?>) { |
There was a problem hiding this comment.
[P1] Fix the new checkstyle violations
SeekableStreamIndexTask<?,?,?> violates the configured WhitespaceAfter check, while changing line 309 to .toList() leaves java.util.stream.Collectors unused and violates UnusedImports; validation will fail until both are corrected.
| |`segment/added/bytes`|Size in bytes of new segments created.| `dataSource`, `taskId`, `taskType`, `groupId`, `interval`, `tags`|Varies| | ||
| |`segment/moved/bytes`|Size in bytes of segments moved/archived via the Move Task.| `dataSource`, `taskId`, `taskType`, `groupId`, `interval`, `tags`|Varies| | ||
| |`segment/nuked/bytes`|Size in bytes of segments deleted via the Kill Task.| `dataSource`, `taskId`, `taskType`, `groupId`, `interval`, `tags`|Varies| | ||
| |`segment/allocated/count`|Number of segments successfully allocated by the Overlord to an append (realtime or batch) task. May be emitted multiple times for a single allocated segment ID if multiple tasks request an allocation with the same parameters.|`id`, `dataSource`, `taskId`, `taskType`, `groupId`, tags`|Always 1| |
There was a problem hiding this comment.
[P3] Document the emitted supervisor dimension
The implementation adds supervisorId for seekable-stream tasks, but this dimension is omitted from the metric table. The same row also has an unmatched backtick around tags.
2 participants
Related to #19651 which adds metrics for upgrade of pending segments.
Changes
segment/allocated/countto track IDs of allocated pending segmentsThis PR has: