kamalcph commented on code in PR #16959:
URL: https://github.com/apache/kafka/pull/16959#discussion_r1729655598
##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -1254,6 +1255,20 @@ void cleanupExpiredRemoteLogSegments() throws
RemoteStorageException, ExecutionE
canProcess = false;
continue;
}
+
+ if
(RemoteLogSegmentState.COPY_SEGMENT_STARTED.equals(metadata.state())) {
+ // get the current segment state here to avoid the
race condition that before the loop, it's under copying process,
+ // but then completed. In this case,
segmentIdsBeingCopied will not contain this id, so we might
+ // delete this segment unexpectedly.
+ Optional<RemoteLogSegmentMetadata> curMetadata =
remoteLogMetadataManager.remoteLogSegmentMetadata(
Review Comment:
There can be overlapping remote-log-segments and the
RLMM#remoteLogSegmentMetadata might return a different RLSM than the requested
one. We might be skipping the deletion for the dangling segment.
Instead of deleting the dangling segments in the same iteration, Can we note
down the dangling segments in the current iteration and delete them in the next
iteration of `cleanupExpiredRemoteLogSegments`?
--
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
