junrao commented on code in PR #20131:
URL: https://github.com/apache/kafka/pull/20131#discussion_r2205387026


##########
storage/src/main/java/org/apache/kafka/storage/internals/log/AbstractIndex.java:
##########
@@ -48,7 +47,10 @@ private enum SearchResultType {
 
     private static final Logger log = 
LoggerFactory.getLogger(AbstractIndex.class);
 
-    // Serializes all index operations that mutate internal state
+    // Serializes all index operations that mutate internal state.
+    // Clients only read committed data and are not affected by concurrent 
appends/truncates.
+    // In the rare case, when the data is truncated, the follower could read 
inconsistent data.
+    // The follower has the logic to ignore the inconsistent data through crc 
and leader epoch.

Review Comment:
   Actually, I meant to only replace the first item, not the whole thing. Could 
we use the following?
   
   ```
       // Serializes all index operations that mutate internal state.
       // Readers do not need to acquire this lock because:
       //  1) MappedByteBuffer provides direct access to the OS-level buffer 
cache,
       //     which allows concurrent reads in practice.
       //  2) Clients only read committed data and are not affected by 
concurrent appends/truncates.
       //     In the rare case when the data is truncated, the follower could 
read inconsistent data.
       //     The follower has the logic to ignore the inconsistent data 
through crc and leader epoch.
       //  3) Read and remap operations are coordinated via remapLock to ensure 
visibility of the
              underlying mmap.
   ```



##########
server-common/src/main/java/org/apache/kafka/server/util/LockUtils.java:
##########
@@ -115,7 +70,7 @@ public static <T, E extends Exception> T inLockThrows(Lock 
lock, ThrowingSupplie
      * @throws E if an exception occurs during the execution of the runnable
      * @throws NullPointerException if either {@code lock} or {@code runnable} 
is null
      */
-    public static <E extends Exception> void inLockThrows(Lock lock, 
ThrowingRunnable<E> runnable) throws E {
+    public static <E extends Exception> void inLock(Lock lock, 
ThrowingRunnable<E> runnable) throws E {

Review Comment:
   Just to clarify, if the caller doesn't throw an exception, what does E bind 
to? `RuntimeException`?



-- 
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