apoorvmittal10 commented on code in PR #16842:
URL: https://github.com/apache/kafka/pull/16842#discussion_r1823126487


##########
core/src/main/java/kafka/server/share/SharePartitionManager.java:
##########
@@ -490,6 +497,30 @@ public void acknowledgeSessionUpdate(String groupId, 
ShareRequestMetadata reqMet
         }
     }
 
+    /**
+     * The handleFetchException method is used to handle the exception that 
occurred while reading from log.
+     * The method will handle the exception for each topic-partition in the 
request. The share partition
+     * might get removed from the cache.
+     * <p>
+     * The replica read request might error out for one share partition
+     * but as we cannot determine which share partition errored out, we might 
remove all the share partitions
+     * in the request.
+     *
+     * @param groupId The group id in the share fetch request.
+     * @param topicIdPartitions The topic-partitions in the replica read 
request.
+     * @param future The future to complete with the exception.
+     * @param throwable The exception that occurred while fetching messages.
+     */
+    public void handleFetchException(
+        String groupId,
+        Set<TopicIdPartition> topicIdPartitions,
+        CompletableFuture<Map<TopicIdPartition, PartitionData>> future,
+        Throwable throwable
+    ) {
+        topicIdPartitions.forEach(topicIdPartition -> 
handleFencedSharePartitionException(sharePartitionKey(groupId, 
topicIdPartition), throwable));

Review Comment:
   Hmmm, though you are right but the problem is that ShareFetchResponse 
contains both Fetch and Acknowledgement response. If we set a top level error 
code then the acknowledgement in request will also be responded as failed 
however acknowledgement might have succeeded.
   
   The reason I was removeing the SharePartition in this condition was that 
recreating a new SharePartition is cheap. However if you think we shouldn't 
remove then I will avoid that and see waht scenarios we encounter during the 
tests (moving partitions in a cluster).



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