albertobastos commented on code in PR #16310:
URL: https://github.com/apache/pinot/pull/16310#discussion_r2221466517
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java:
##########
@@ -1007,6 +1008,8 @@ public Response endReplaceSegments(
@ApiParam(value = "OFFLINE|REALTIME", required = true)
@QueryParam("type") String tableTypeStr,
@ApiParam(value = "Segment lineage entry id returned by
startReplaceSegments API", required = true)
@QueryParam("segmentLineageEntryId") String segmentLineageEntryId,
+ @ApiParam(value = "Trigger an immediate segment prune")
@QueryParam("prune") @DefaultValue("false")
Review Comment:
I tried to avoid `cleanup` because it was already used on some other places
and didn't want to mix concepts but after further review yeah, I think is
harmless to do so.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java:
##########
@@ -1022,6 +1025,9 @@ public Response endReplaceSegments(
Preconditions.checkNotNull(segmentLineageEntryId,
"'segmentLineageEntryId' should not be null");
_pinotHelixResourceManager.endReplaceSegments(tableNameWithType,
segmentLineageEntryId,
endReplaceSegmentsRequest);
+ if (pruneSegments) {
+
_pinotHelixResourceManager.invokeControllerPeriodicTask(tableNameWithType,
RetentionManager.TASK_NAME, null);
Review Comment:
> Invoking retention manager is too expensive and will touch all tables.
Is that so? Passing the `tableNameWithType` parameter it would be expected
to just affect the referred table.
> Not introduced in this PR, but we didn't check the return value of
_pinotHelixResourceManager.endReplicaSegments()
If you meant `_pinotHelixResourceManager.endReplaceSegments()` it has no
return value.
--
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]
