Jackie-Jiang commented on code in PR #13953:
URL: https://github.com/apache/pinot/pull/13953#discussion_r1750726919
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3722,8 +3722,12 @@ public String startReplaceSegments(String
tableNameWithType, List<String> segmen
// Trigger the proactive segment clean up if needed. Once the lineage is
updated in the property store, it
// is safe to physically delete segments.
if (!segmentsToCleanUp.isEmpty()) {
- LOGGER.info("Cleaning up the segments while startReplaceSegments: {}",
segmentsToCleanUp);
- deleteSegments(tableNameWithType, segmentsToCleanUp);
+ // Only keep the segments that within the table for the proactive
clean-up.
+ segmentsToCleanUp.retainAll(getSegmentsFor(tableNameWithType, false));
Review Comment:
I don't follow this. Why do we want to skip segments not in IS? One possible
scenario is that segments are not yet pushed to IS, and we don't want to leave
them orphan.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
