[GitHub] [kafka] divijvaidya commented on a diff in pull request #12365: KAFKA-14020: Performance regression in Producer

[GitBox <gi...@apache.org>]

divijvaidya commented on code in PR #12365:
URL: https://github.com/apache/kafka/pull/12365#discussion_r912836877


##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java:
##########
@@ -297,7 +297,12 @@ public RecordAppendResult append(String topic,
                     byte maxUsableMagic = apiVersions.maxUsableProduceMagic();
                     int size = Math.max(this.batchSize, AbstractRecords.estimateSizeInBytesUpperBound(maxUsableMagic, compression, key, value, headers));
                     log.trace("Allocating a new {} byte message buffer for topic {} partition {} with remaining timeout {}ms", size, topic, partition, maxTimeToBlock);
+                    // This call may block if we exhausted buffer space.
                     buffer = free.allocate(size, maxTimeToBlock);
+                    // Update the current time in case the buffer allocation blocked above.
+                    // NOTE: getting time may be expensive, so calling it under a lock
+                    // should be avoided.
+                    nowMs = time.milliseconds();

Review Comment:
   This change has some other side-effects to the code logic:
   1. In case of retries (multiple iteration of while loop) when buffer allocation blocks, prior to this change `tryAppend` on line 283 was being called with older `nowMs`. After this change it's a more recent time. This is a positive change.
   
   2. In case of retries, when buffer allocation does not occur, prior to this change the time was computed inside `appendNewBatch` hence, was guaranteed to the latest. After this change, there might be threads blocked on `synchronized` or the time consumed by previous retry isn't factored in the `nowMs` being passed to appendNewBatch. Hence, `nowMs` being passed to `appendNewBatch` might be stale by some amount (depending on how long threads were waiting to acquire the block). Is that acceptable?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org