Copilot commented on code in PR #19269:
URL: https://github.com/apache/pinot/pull/19269#discussion_r3790545135
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/MinionTaskUtils.java:
##########
@@ -356,14 +369,18 @@ public static RoaringBitmap
getValidDocIdFromServerMatchingCrc(String tableNameW
serverSegmentMetadataReader.getValidDocIdsBitmapFromServer(tableNameWithType,
segmentName, endpoint,
validDocIdsType, 60_000);
} catch (Exception e) {
+ String errorMessage =
+ "Unable to retrieve validDocIds bitmap for segment: " +
segmentName + " from endpoint: " + endpoint;
if (consensusMode == MinionConstants.ValidDocIdsConsensusMode.UNSAFE) {
- LOGGER.warn(
- "Unable to retrieve validDocIds bitmap for segment: " +
segmentName + " from endpoint: " + endpoint, e);
+ LOGGER.warn(errorMessage, e);
continue;
- } else {
- throw new IllegalStateException(
- "Unable to retrieve validDocIds bitmap for segment: " +
segmentName + " from endpoint: " + endpoint, e);
}
+ if (e instanceof NotFoundException) {
+ // Preserve the type: a 404 means the server has no validDocIds for
the segment (e.g. a not-yet-written
+ // snapshot), which callers may handle with a documented fallback,
unlike infrastructure failures.
+ throw new NotFoundException(errorMessage, e);
Review Comment:
Rethrowing the type does not restore the described fallback in Pinot's
production paths. The only production callers
(`UpsertCompactionTaskExecutor:86` and `UpsertCompactMergeTaskExecutor:118`)
let this exception escape, and `TaskFactoryRegistry:172-180` handles it like
every other failure, so a missing snapshot still marks the task failed and
enters the retry loop. Please add the intended `NotFoundException` fallback in
both executors (and exercise it in executor-level tests).
--
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]
