tarun11Mavani commented on code in PR #18813:
URL: https://github.com/apache/pinot/pull/18813#discussion_r3449898217
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/BaseSingleSegmentConversionExecutor.java:
##########
@@ -210,19 +209,17 @@ public SegmentConversionResult
executeTask(PinotTaskConfig pinotTaskConfig)
throw new UnsupportedOperationException("Unrecognized push mode: "
+ pushType);
}
} catch (Exception e) {
- uploadSuccessful = false;
_minionMetrics.addMeteredTableValue(tableNameWithType,
MinionMeter.SEGMENT_UPLOAD_FAIL_COUNT, 1L);
LOGGER.error("Segment upload failed for segment {}, table {}",
segmentName, tableNameWithType, e);
_eventObserver.notifyTaskError(_pinotTaskConfig, e);
- }
- if (!FileUtils.deleteQuietly(convertedTarredSegmentFile)) {
- LOGGER.warn("Failed to delete tarred converted segment: {}",
convertedTarredSegmentFile.getAbsolutePath());
- }
-
- if (uploadSuccessful) {
- LOGGER.info("Done executing {} on table: {}, segment: {}", taskType,
tableNameWithType, segmentName);
+ throw e;
Review Comment:
Good catch — fixed in 9863fcc.
`uploadSegmentWithMetadata` now deletes the staged tar from the output
PinotFS if the metadata send fails, before rethrowing, so a retry re-stages
cleanly instead of hitting "Output file already exists".
I went with cleanup-on-failure rather than forcing `overwriteOutput=true`,
so the existing guard against unrelated collisions in the output dir stays
intact — we only remove the tar when it's our own stale partial from a failed
attempt.
Added `testExecuteTaskCleansUpStagedTarWhenMetadataPushFails`, verified it
fails without the cleanup.
--
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]
