ijuma commented on code in PR #13213:
URL: https://github.com/apache/kafka/pull/13213#discussion_r1098706079


##########
storage/src/main/java/org/apache/kafka/storage/internals/log/AbstractIndex.java:
##########
@@ -64,8 +65,7 @@ private enum SearchResultType {
      */
     private volatile int maxEntries;
     /** The number of entries in this index */
-    private volatile int entries;
-
+    private final AtomicInteger entries;

Review Comment:
   This PR is different from the others in that it's only a rename and it 
affects a _lot_ of files. The others were rewriting a smaller set of files from 
Scala to Java too.
   
   > But I do not think we should have a contract for the caller to invoke in a 
lock because we have a volatile primtive to be incremented. This can be 
discussed in another PR.
   
   Debatable, the `incrementEntries` method is `protected` and only used by the 
two subclasses. The locking protocol of the class is a lot more complicated and 
I don't see why this particular field is the concern given everything else that 
is happening in the class.



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