adixitconfluent commented on code in PR #17539:
URL: https://github.com/apache/kafka/pull/17539#discussion_r1826562798
##########
core/src/main/java/kafka/server/share/DelayedShareFetch.java:
##########
@@ -146,18 +149,47 @@ public void onComplete() {
*/
@Override
public boolean tryComplete() {
- topicPartitionDataFromTryComplete = acquirablePartitions();
+ if (anySharePartitionNoLongerExists()) {
Review Comment:
Hi @junrao, I am not actually too sure if I definitely need this check.
Ideally, I don't think there can be any value in `sharePartitions` which can be
null, but this
[TODO](https://github.com/apache/kafka/blob/trunk/core/src/main/java/kafka/server/share/SharePartitionManager.java#L629)
in `SharePartitionManager` is confusing me, plus there is this JIRA
https://issues.apache.org/jira/browse/KAFKA-17510 where we will be refactoring
share partition initialization. So, this can act as a safety check for now, and
I can remove this in a future PR once the refactor is complete, and we are sure
we don't send null share partitions. What do you think?
cc - @apoorvmittal10
--
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]
