showuon commented on code in PR #16959:
URL: https://github.com/apache/kafka/pull/16959#discussion_r1732102612


##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -1254,7 +1265,10 @@ void cleanupExpiredRemoteLogSegments() throws 
RemoteStorageException, ExecutionE
                         canProcess = false;
                         continue;
                     }
-                    if 
(RemoteLogSegmentState.DELETE_SEGMENT_FINISHED.equals(metadata.state())) {
+                    // skip the COPY_SEGMENT_STARTED segments since they might 
be the dangling segments that failed before
+                    // and blocks the normal segment deletion, ex: it failed 
`isRemoteSegmentWithinLeaderEpochs` check... etc
+                    if 
(RemoteLogSegmentState.DELETE_SEGMENT_FINISHED.equals(metadata.state()) ||
+                            
RemoteLogSegmentState.COPY_SEGMENT_STARTED.equals(metadata.state())) {

Review Comment:
   > For my understanding, why are we not changing the state from 
COPY_SEGMENT_STARTED -> DELETE_SEGMENT_STARTED -> DELETE_SEGMENT_FINISHED?
   
   Yes, why not? Created 
[KAFKA-17428](https://issues.apache.org/jira/browse/KAFKA-17428) for this 
issue. 



-- 
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-unsubscr...@kafka.apache.org

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

Reply via email to