AndrewJSchofield commented on code in PR #19045:
URL: https://github.com/apache/kafka/pull/19045#discussion_r1973984099


##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -1291,13 +1303,30 @@ public boolean maybeAcquireFetchLock() {
         if (stateNotActive()) {
             return false;
         }
-        return fetchLock.compareAndSet(false, true);
+        boolean acquired = fetchLock.compareAndSet(false, true);
+        if (acquired) {
+            long currentTime = time.hiResClockMs();
+            fetchLockAcquiredTimeMs = currentTime;
+            timeSinceLastLockAcquisitionMs = timeSinceLastLockAcquisitionMs != 
0 ? currentTime - timeSinceLastLockAcquisitionMs : 0;

Review Comment:
   Could we please have some defensive code here to guard against time running 
backwards? Of course, what I mean is perhaps the time gets adjusted and 
`currentTime < timeSinceLastLockAcquisitionMs`. I don't think we want a 
negative value.



##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -1326,6 +1355,42 @@ int leaderEpoch() {
         return leaderEpoch;
     }
 
+    // Visible for testing
+    void recordFetchLockRatioMetric(long acquiredTime) {

Review Comment:
   Javadoc comment here please, in particular for the parameter.



##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -1326,6 +1355,42 @@ int leaderEpoch() {
         return leaderEpoch;
     }
 
+    // Visible for testing
+    void recordFetchLockRatioMetric(long acquiredTime) {
+        // Update the total fetch lock acquired time.
+        double fetchLockToTotalTime;
+        if (acquiredTime + timeSinceLastLockAcquisitionMs == 0) {
+            // If the total time is 0 then the ratio is 1 i.e. the fetch lock 
was acquired for the complete time.
+            fetchLockToTotalTime = 1;

Review Comment:
   nit: `1.0`. We are switching between floating point and integers here so I 
would be explicit about this. I know it makes no practical difference.



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