showuon commented on code in PR #16932:
URL: https://github.com/apache/kafka/pull/16932#discussion_r1724493687
##########
core/src/main/scala/kafka/log/UnifiedLog.scala:
##########
@@ -1601,7 +1601,7 @@ class UnifiedLog(@volatile var logStartOffset: Long,
shouldDelete
}
- deleteOldSegments(shouldDelete, RetentionSizeBreach(this,
remoteLogEnabled()))
+ deleteOldSegments(shouldDelete, RetentionSizeBreach(this,
remoteLogEnabledAndRemoteCopyEnabled()))
}
private def deleteLogStartOffsetBreachedSegments(): Int = {
Review Comment:
We don't need to change `deleteLogStartOffsetBreachedSegments`.
The delete log start offset breached will happen in cases like user sends
RPC to delete old records. So the log start offset will be incremented. (ex:
delete records to offset 10, so we increment log start offset to 10). And in
`deleteLogStartOffsetBreachedSegments`, we'll check if the incremented log
start offset need to delete segments, ex: segment 1 contains offset 0 ~ 8, so
segment 1 can be deleted... etc.
If remote storage is enabled (no matter remote copy enable or not), we will
always check local log start offset, because there will be another thread in
remote log manager to check the log start offset breach or not.
--
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
