minor: diagnostics when MSQ's on-demand segment cache can't fit an input segment - #19985
minor: diagnostics when MSQ's on-demand segment cache can't fit an input segment #19985cecemei wants to merge 4 commits into
Conversation
github-actions
Bot
added
Area - Batch Ingestion
Area - MSQ
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 1 |
| Total | 2 |
Reviewed 4 of 4 changed files.
Validation: git diff --check passed. Builds and tests not run.
This is an automated review by Codex GPT-5.6-Luna(max)
| } | ||
| } | ||
| throw DruidException.forPersona(DruidException.Persona.USER) | ||
| throw DruidException.forPersona(DruidException.Persona.OPERATOR) |
There was a problem hiding this comment.
[P2] Capacity failure changes the error persona
Changing this exception from DruidException.Persona.USER to OPERATOR changes the response persona for the normal segment-capacity failure path. That can hide the new diagnostic from the user-facing error-response strategy and alter the wire behavior of queries that cannot fit a segment. Preserve the USER persona, or add operator diagnostics without changing the response persona.
|
|
||
| Assertions.assertEquals(TaskState.FAILED, finalStatus.getStatusCode()); | ||
| Assertions.assertTrue( | ||
| finalStatus.getErrorMsg() != null && finalStatus.getErrorMsg().contains("Unable to load segment"), |
There was a problem hiding this comment.
[P3] Capacity diagnostics are not asserted
The new test only checks the pre-existing Unable to load segment prefix, so it passes even if the size, per-location capacity, and eviction diagnostics are absent or malformed. Assert the new diagnostic values or at least the added suffix so this test protects the behavior introduced by the PR.
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 4 of 4 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
| } else { | ||
| locationFailures.add( | ||
| StringUtils.format( | ||
| "location[%s]: max[%,d] available[%,d]", |
There was a problem hiding this comment.
[P2] Free-space failures are reported as capacity failures
When freeSpacePercent leaves insufficient physical disk space, reservation can fail even though configured available capacity is sufficient. This branch reports only max/available capacity and later claims the segment exceeded every location's capacity, misleading operators. Include the physical free-space constraint or distinguish the failure reason.
3 participants
Description
MSQ workers read input segments through a virtual-storage cache (SegmentLocalCacheManager/StorageLocation) sized as a fraction of tmpStorageBytesPerTask. If a single segment exceeds that cache's capacity, it fails unconditionally (nothing can be evicted to make room), and the resulting error gives no numbers, just "ensure enough disk space has been allocated." The actual size/capacity data was already computed for a debug log line but never surfaced. This PR is diagnostics-only, no behavior change.
This PR has: