kamalcph commented on code in PR #14649:
URL: https://github.com/apache/kafka/pull/14649#discussion_r1404590243
##########
core/src/main/scala/kafka/log/UnifiedLog.scala:
##########
@@ -1754,6 +1754,7 @@ class UnifiedLog(@volatile var logStartOffset: Long,
leaderEpochCache.foreach(_.clearAndFlush())
producerStateManager.truncateFullyAndStartAt(newOffset)
logStartOffset = logStartOffsetOpt.getOrElse(newOffset)
+ if (remoteLogEnabled()) maybeIncrementLocalLogStartOffset(newOffset,
LogStartOffsetIncrementReason.SegmentDeletion)
Review Comment:
> Adding it to truncateFullyAndStartAt means that anything that calls the
function does not have to worry about the updating the local log start offset
every time.
My preference is to update the local-log-start-offset in
`truncateFullyAndStartAt` function as it updates the leader-epoch-checkpoint
file, log-start-offset, high-watermark, and log-end-offset.
To avoid passing the reason in `maybeIncrementLocalLogStartOffset` method,
we can update the `local-log-start-offset` value directly in
truncateFullyAndStartAt method since it takes the same `lock`.
We followed a similar approach in our local branch:
https://sourcegraph.com/github.com/satishd/kafka@24d21694f2f0450e8a8e60e85400de3586fc7069/-/blob/core/src/main/scala/kafka/log/Log.scala?L2385
--
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
