apoorvmittal10 commented on code in PR #17709:
URL: https://github.com/apache/kafka/pull/17709#discussion_r1834568839
##########
core/src/main/java/kafka/server/share/DelayedShareFetch.java:
##########
@@ -198,7 +198,7 @@ Map<TopicIdPartition, FetchRequest.PartitionData>
acquirablePartitions() {
Map<TopicIdPartition, FetchRequest.PartitionData> topicPartitionData =
new LinkedHashMap<>();
sharePartitions.forEach((topicIdPartition, sharePartition) -> {
- int partitionMaxBytes =
shareFetchData.partitionMaxBytes().getOrDefault(topicIdPartition, 0);
+ int partitionMaxBytes =
shareFetch.partitionMaxBytes().getOrDefault(topicIdPartition, 0);
Review Comment:
> Should we skip erroneous partitions in shareFetch?
Here the method is iterating over `sharePartitions` which have been filled
in `SharePartitionManager` hence some of the errored share partitions might not
come here itself as already added to erroneous. You are right, here again there
could be a sharePartition which errored out so rather skipping them from fetch
we should do the second recommendation of yours which is to add them to
errorneous if received exception from `maybeAcquireFetchLock`.
There already exists a jira for same, from previous PR comments, which I am
planning to do next: https://issues.apache.org/jira/browse/KAFKA-17901
--
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
