ShivsundarR commented on code in PR #19295:
URL: https://github.com/apache/kafka/pull/19295#discussion_r2016349699
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ShareConsumerImpl.java:
##########
@@ -644,8 +645,12 @@ private ShareFetch<K, V> collect(Map<TopicIdPartition,
NodeAcknowledgements> ack
if (currentFetch.isEmpty()) {
final ShareFetch<K, V> fetch = fetchCollector.collect(fetchBuffer);
if (fetch.isEmpty()) {
+ // Check for any acknowledgements which could have come from
control records (GAP) and include them.
+ Map<TopicIdPartition, NodeAcknowledgements>
combinedAcknowledgements = new LinkedHashMap<>(acknowledgementsMap);
+
combinedAcknowledgements.putAll(fetch.takeAcknowledgedRecords());
+
// Fetch more records and send any waiting acknowledgements
- applicationEventHandler.add(new
ShareFetchEvent(acknowledgementsMap));
+ applicationEventHandler.add(new
ShareFetchEvent(combinedAcknowledgements));
Review Comment:
@chia7712 So we have ShareFetch and `ShareInFlightBatch`.
`ShareFetch` has a `Map<TopicIdPartition, ShareInFlightBatch<K, V>>` as a
member.
`ShareFetch::isEmpty` does
```
public boolean isEmpty() {
return numRecords() == 0;
}
```
which iterates through the batches and checks ShareInFlightBatch::numRecords
which is
```
int numRecords() {
return inFlightRecords.size();
}
```
The code you shared is from `ShareInFlightBatch::isEmpty()` where it checks
both the `InFlightRecords` and `Acknowledgements`.
```
public boolean isEmpty() {
return inFlightRecords.isEmpty() && acknowledgements.isEmpty();
}
```
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ShareConsumerImpl.java:
##########
@@ -644,8 +645,12 @@ private ShareFetch<K, V> collect(Map<TopicIdPartition,
NodeAcknowledgements> ack
if (currentFetch.isEmpty()) {
final ShareFetch<K, V> fetch = fetchCollector.collect(fetchBuffer);
if (fetch.isEmpty()) {
+ // Check for any acknowledgements which could have come from
control records (GAP) and include them.
+ Map<TopicIdPartition, NodeAcknowledgements>
combinedAcknowledgements = new LinkedHashMap<>(acknowledgementsMap);
+
combinedAcknowledgements.putAll(fetch.takeAcknowledgedRecords());
+
// Fetch more records and send any waiting acknowledgements
- applicationEventHandler.add(new
ShareFetchEvent(acknowledgementsMap));
+ applicationEventHandler.add(new
ShareFetchEvent(combinedAcknowledgements));
Review Comment:
@chia7712 So we have `ShareFetch` and `ShareInFlightBatch`.
`ShareFetch` has a `Map<TopicIdPartition, ShareInFlightBatch<K, V>>` as a
member.
`ShareFetch::isEmpty` does
```
public boolean isEmpty() {
return numRecords() == 0;
}
```
which iterates through the batches and checks ShareInFlightBatch::numRecords
which is
```
int numRecords() {
return inFlightRecords.size();
}
```
The code you shared is from `ShareInFlightBatch::isEmpty()` where it checks
both the `InFlightRecords` and `Acknowledgements`.
```
public boolean isEmpty() {
return inFlightRecords.isEmpty() && acknowledgements.isEmpty();
}
```
--
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
