artemlivshits commented on code in PR #12462:
URL: https://github.com/apache/kafka/pull/12462#discussion_r954487697
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java:
##########
@@ -273,26 +273,29 @@ public RecordAppendResult append(String topic,
// check if we have an in-progress batch
Deque<ProducerBatch> dq =
topicInfo.batches.computeIfAbsent(effectivePartition, k -> new ArrayDeque<>());
+ RecordAppendResult appendResult;
synchronized (dq) {
// After taking the lock, validate that the partition
hasn't changed and retry.
if
(topicInfo.builtInPartitioner.isPartitionChanged(partitionInfo)) {
log.trace("Partition {} for topic {} switched by a
concurrent append, retrying",
partitionInfo.partition(), topic);
continue;
}
- RecordAppendResult appendResult = tryAppend(timestamp,
key, value, headers, callbacks, dq, nowMs);
- if (appendResult != null) {
+ appendResult = tryAppend(timestamp, key, value, headers,
callbacks, dq, nowMs);
+ if (appendResult != null && !appendResult.newBatchCreated)
{
topicInfo.builtInPartitioner.updatePartitionInfo(partitionInfo,
appendResult.appendedBytes, cluster);
return appendResult;
}
}
- // we don't have an in-progress record batch try to allocate a
new batch
- if (abortOnNewBatch) {
- // Return a result that will cause another call to append.
+ // either 1. current topicPartition producerBatch is full -
return and prepare for another batch/partition.
+ // 2. no producerBatch existed for this topicPartition, create
a new producerBatch.
+ if (appendResult == null && abortOnNewBatch) {
Review Comment:
> if there are no batches in current partition, are there cases we would
still want to switch partitions?
It could happen that batch got sent out by the the time next record arrives.
When linger.ms=0 (the default), the sender would try to send the batch out as
soon as possible. So the sequence of events could look like this:
0. batch queue is empty.
1. append record: create a new batch, but don't switch partitions because
queue is empty
2. sender drains the queue, so now queue is empty
3. append record: create a new batch, but don't switch partitions because
queue is empty
...
--
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]
