ji-seung-ryu commented on PR #19786: URL: https://github.com/apache/kafka/pull/19786#issuecomment-2907597412
> @ji-seung-ryu Thanks for the changes. As I mentioned in this [comment](https://github.com/apache/kafka/pull/19786#pullrequestreview-2863310322), we also need to change `testReleaseSessionCompletesDelayedShareFetchRequest` to mock the behaviour for `releaseAcquiredRecords` for `tp3`. Example - `when(sp3.releaseAcquiredRecords(ArgumentMatchers.eq(memberId))).thenReturn(CompletableFuture.completedFuture(null));` Simply adding that kinds of line is not sufficient. Since tp3 is not in sharePartitionManager.partitionCache, but returned in cachedTopicIdPartitionsInShareSession. Currently, In Following codes of SharePartitionManager.java, exceptions occurs. List<TopicIdPartition> topicIdPartitions = cachedTopicIdPartitionsInShareSession( groupId, memberIdUuid); // tp3 is included. ... topicIdPartitions.forEach(topicIdPartition -> { SharePartitionKey sharePartitionKey = sharePartitionKey(groupId, topicIdPartition); SharePartition sharePartition = partitionCache.get(sharePartitionKey); // In partitionCache, no tp3 if (sharePartition == null) { // this exception is called. log.error("No share partition found for groupId {} topicPartition {} while releasing acquired topic partitions", groupId, topicIdPartition);' There are two options we can choose. 1. add tp3 in sharePartitionManager.partitionCache and make mock behaviors for it. 2. do nothing and see if exception is detected and handled well. I guess 2 is the better choice, since I see this is intentional like testReleaseSessionSuccess. -- 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
