adixitconfluent commented on code in PR #17583:
URL: https://github.com/apache/kafka/pull/17583#discussion_r1813292227


##########
core/src/main/java/kafka/server/share/DelayedShareFetch.java:
##########
@@ -149,15 +149,27 @@ public void onComplete() {
      */
     @Override
     public boolean tryComplete() {
-        log.trace("Try to complete the delayed share fetch request for group 
{}, member {}, topic partitions {}",
-            shareFetchData.groupId(), shareFetchData.memberId(),
-            shareFetchData.partitionMaxBytes().keySet());
+        // There can be multiple threads which might invoke tryComplete for 
same share fetch request
+        // hence check if delay share fetch request is already completed. If 
yes, return true.
+        // However, this check alone cannot guarantee that request is really 
completed. It is possible that
+        // tryComplete is invoked by multiple threads and state has yet not 
updated. Hence, we need to check
+        // the forceComplete response as well.
+        if (isCompleted()) {

Review Comment:
   Can we also add unit tests for the conditions to verify this line and line 
164



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

Reply via email to