smjn commented on code in PR #22479:
URL: https://github.com/apache/kafka/pull/22479#discussion_r3379838146


##########
server/src/main/java/org/apache/kafka/server/share/dlq/ShareGroupDLQStateManager.java:
##########
@@ -653,6 +684,80 @@ private void handleProduceResponse(ClientResponse 
response) {
                     requestErrorResponse(clientResponseError.exception());
             }
         }
+
+        private void maybeFetchRecordData() {
+            if (cacheHelper.isShareGroupDlqCopyRecordEnabled(param.groupId())) 
{
+                // A non-null originalRecordData indicates that the data for 
the offsets was
+                // already fetched at a previous time. This could happen in 
case there was
+                // a retriable exception in a previous produce request, and it 
is being re-sent.
+                // This optimization will help in reducing LogReader.read 
calls. Note that an
+                // empty (but non-null) map means a previous fetch found no 
records in range
+                // (e.g. all offsets compacted away), so we still skip 
re-fetching in that case.
+                if (originalRecordData != null) {
+                    return;
+                }

Review Comment:
   When the dlq call is made by the dlqManager - it implies all records being 
DLQ'ed have already present in the topic which is read only. So if a previous 
call has indeed found some data related to the dlqManager call, the next time 
that call happens (maybe retry), the data can only reduce (perhaps compaction) 
but not increase.



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