snleee commented on code in PR #8838:
URL: https://github.com/apache/pinot/pull/8838#discussion_r899415424
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3163,6 +3165,7 @@ public String startReplaceSegments(String
tableNameWithType, List<String> segmen
if (!segmentsToCleanUp.isEmpty()) {
LOGGER.info("Cleaning up the segments while startReplaceSegments:
{}", segmentsToCleanUp);
deleteSegments(tableNameWithType, segmentsToCleanUp);
+ waitForSegmentsToDelete(tableNameWithType, segmentsToCleanUp,
SEGMENT_CLEANUP_TIMEOUT_MS);
Review Comment:
@klsince Essentially, because of async deletion, there can be a short
duration where 3 data snapshots (old, current, new) exist in the table and this
can fill up the disk space pretty easily. To avoid that, we would want to make
sure to finish "clean up" before we allow the new data push.
@jtao15 I actually think that we should consider making this optional. For
instance, S3 file system implementation needs to copy data and then delete the
previous one for `doMove()`. In this case, it will take a long time to delete
segments.
@klsince Have you seen any long delay in segment deletion in your
deployments? We internally use NFS so `doMove` is a very cheap operation.
--
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]
