ocadaruma commented on code in PR #19972:
URL: https://github.com/apache/kafka/pull/19972#discussion_r2159867183
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogSegment.java:
##########
@@ -192,8 +192,13 @@ public void sanityCheck(boolean timeIndexFileNewlyCreated)
throws IOException {
* the time index).
*/
public TimestampOffset readMaxTimestampAndOffsetSoFar() throws IOException
{
- if (maxTimestampAndOffsetSoFar == TimestampOffset.UNKNOWN)
Review Comment:
> We can pre-load the maxTimestampAndOffsetSoFar at startup for the active
segment
That's right, but there are multiple paths which adds new segment to the
segments (e.g. in LocalLog#roll, LocalLog#createAndDeleteSegment) and we need
to do pre-load carefully in all possible paths (including future code changes),
which is error-prone.
As commented in
https://github.com/apache/kafka/pull/19972#issuecomment-2979526801, the lock
introduced in this PR might not cause new lock contention because
potentially-blocking timeIndex materialization already has a lock.
So IMO lock-approach might be better.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
