[GitHub] [kafka] satishd commented on a diff in pull request #13561: KAFKA-14888: Added remote log segments retention functionality based on time and size.

["satishd (via GitHub)" <gi...@apache.org>]

satishd commented on code in PR #13561:
URL: https://github.com/apache/kafka/pull/13561#discussion_r1181520975


##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -581,11 +588,18 @@ public void run() {
                 if (isLeader()) {
                     // Copy log segments to remote storage
                     copyLogSegmentsToRemote();
+                    // Cleanup/delete expired remote log segments
+                    cleanupExpiredRemoteLogSegments();

Review Comment:
   This is different from local log deletion. It requires the deletion of segments from local storage which need to really delete the files. But incase of remote storages, it does not wait for the data to be deleted but it marks the file or object for deletion in their respective metadata stores. Respective garbage collectors in those storages will take care of deleting the data asynchronously. There is no perf impact for these delete calls as they take much shorter time than copying segments. It is very unlikely that copying segments get affected because of deletion of segments. Deletion checks are happening in every iteration so there will not be many segments that need to be deleted. Anyways, we can discuss this separately in a separate JIRA. On another note, all this logic will go to UnifiedLog in future.
   



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org