Skip to content
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fd0e06b
KAFKA-4390: Replace MessageSet usage with client-side alternatives
hachikuji Oct 28, 2016
0ac2a15
Let ByteBufferLogEntry cache Record instance
hachikuji Dec 5, 2016
a4b13ad
Use last offset as offsetOfMaxTimestamp for compressed message sets
hachikuji Dec 5, 2016
a90f11c
Fix compresion rate calculation bugs in MemoryRecordsBuilder
hachikuji Dec 5, 2016
3694555
Rename Record.size and LogEntry.size to sizeInBytes for consistency
hachikuji Dec 5, 2016
36068ea
Fill in some of the missing javadocs for Record
hachikuji Dec 5, 2016
eef01d6
Always use shallow offset of max timestamp for compressed messages
hachikuji Dec 6, 2016
a525530
Remove Message.toFormatVersion and enhance Record conversion tests
hachikuji Dec 6, 2016
7bff341
GroupMetadataManager should use configured timestamp type for offsets…
hachikuji Dec 6, 2016
4968693
Always load file record data lazily
hachikuji Dec 6, 2016
1dbeadf
Ensure producer buffers are deallocated using the initial allocation …
hachikuji Dec 6, 2016
cf9b0fd
Various improvements per Guozhang's comments
hachikuji Dec 6, 2016
27ac124
Minor tweaks and doc improvements
hachikuji Dec 6, 2016
4f06bef
More naming/doc tweaks
hachikuji Dec 6, 2016
4e8cf94
Make LogEntry extension constructors private
hachikuji Dec 7, 2016
a382de5
Forgotten renamings, cleanup, and buggy offset/timestamp calculation …
hachikuji Dec 12, 2016
ef03d29
Missed one renaming
hachikuji Dec 12, 2016
ffff0bf
FileRecords positions should be integers
hachikuji Dec 12, 2016
a817a76
Cleanup and expand Records javadoc
hachikuji Dec 12, 2016
ae0866b
Fix some stale references to log buffer
hachikuji Dec 13, 2016
a96750f
Minor cleanup and tightening
hachikuji Dec 13, 2016
620ff2c
Fix Records comment on log entry fields
hachikuji Dec 13, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions checkstyle/import-control.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
<allow pkg="net.jpountz" />
<allow pkg="org.apache.kafka.common.record" />
<allow pkg="org.apache.kafka.common.network" />
<allow pkg="org.apache.kafka.common.errors" />
</subpackage>

<subpackage name="requests">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -686,11 +687,13 @@ private PartitionRecords<K, V> parseFetchedData(CompletedFetch completedFetch) {
}

List<ConsumerRecord<K, V>> parsed = new ArrayList<>();
for (LogEntry logEntry : partition.records) {
Iterator<LogEntry> deepIterator = partition.records.deepIterator();
while (deepIterator.hasNext()) {
LogEntry logEntry = deepIterator.next();
// Skip the messages earlier than current position.
if (logEntry.offset() >= position) {
parsed.add(parseRecord(tp, logEntry));
bytes += logEntry.size();
bytes += logEntry.sizeInBytes();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ private void freeUp(int size) {
* memory as free.
*
* @param buffer The buffer to return
* @param size The size of the buffer to mark as deallocated, note that this maybe smaller than buffer.capacity
* @param size The size of the buffer to mark as deallocated, note that this may be smaller than buffer.capacity

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we get rid of the re-allocation logic as a whole? Otherwise we cannot remove this additional check I think.

* since the buffer may re-allocate itself during in-place compression
*/
public void deallocate(ByteBuffer buffer, int size) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@
import org.apache.kafka.common.metrics.stats.Rate;
import org.apache.kafka.common.record.CompressionType;
import org.apache.kafka.common.record.MemoryRecords;
import org.apache.kafka.common.record.MemoryRecordsBuilder;
import org.apache.kafka.common.record.Record;
import org.apache.kafka.common.record.Records;
import org.apache.kafka.common.record.TimestampType;
import org.apache.kafka.common.utils.CopyOnWriteMap;
import org.apache.kafka.common.utils.Time;
import org.apache.kafka.common.utils.Utils;
Expand All @@ -49,7 +51,7 @@
import java.util.concurrent.atomic.AtomicInteger;

/**
* This class acts as a queue that accumulates records into {@link org.apache.kafka.common.record.MemoryRecords}
* This class acts as a queue that accumulates records into {@link MemoryRecords}
* instances to be sent to the server.
* <p>
* The accumulator uses a bounded amount of memory and append calls will block when that memory is exhausted, unless
Expand Down Expand Up @@ -77,7 +79,7 @@ public final class RecordAccumulator {
/**
* Create a new record accumulator
*
* @param batchSize The size to use when allocating {@link org.apache.kafka.common.record.MemoryRecords} instances
* @param batchSize The size to use when allocating {@link MemoryRecords} instances
* @param totalSize The maximum memory the record accumulator can use.
* @param compression The compression codec for the records
* @param lingerMs An artificial delay time to add before declaring a records instance that isn't full ready for
Expand Down Expand Up @@ -190,13 +192,13 @@ public RecordAppendResult append(TopicPartition tp,
free.deallocate(buffer);
return appendResult;
}
MemoryRecords records = MemoryRecords.emptyRecords(buffer, compression, this.batchSize);
RecordBatch batch = new RecordBatch(tp, records, time.milliseconds());
MemoryRecordsBuilder recordsBuilder = MemoryRecords.builder(buffer, compression, TimestampType.CREATE_TIME, this.batchSize);
RecordBatch batch = new RecordBatch(tp, recordsBuilder, time.milliseconds());
FutureRecordMetadata future = Utils.notNull(batch.tryAppend(timestamp, key, value, callback, time.milliseconds()));

dq.addLast(batch);
incomplete.add(batch);
return new RecordAppendResult(future, dq.size() > 1 || batch.records.isFull(), true);
return new RecordAppendResult(future, dq.size() > 1 || batch.isFull(), true);
}
} finally {
appendsInProgress.decrementAndGet();
Expand All @@ -212,9 +214,9 @@ private RecordAppendResult tryAppend(long timestamp, byte[] key, byte[] value, C
if (last != null) {
FutureRecordMetadata future = last.tryAppend(timestamp, key, value, callback, time.milliseconds());
if (future == null)
last.records.close();
last.close();
else
return new RecordAppendResult(future, deque.size() > 1 || last.records.isFull(), false);
return new RecordAppendResult(future, deque.size() > 1 || last.isFull(), false);
}
return null;
}
Expand All @@ -240,7 +242,7 @@ public List<RecordBatch> abortExpiredBatches(int requestTimeout, long now) {
Iterator<RecordBatch> batchIterator = dq.iterator();
while (batchIterator.hasNext()) {
RecordBatch batch = batchIterator.next();
boolean isFull = batch != lastBatch || batch.records.isFull();
boolean isFull = batch != lastBatch || batch.isFull();
// check if the batch is expired
if (batch.maybeExpire(requestTimeout, retryBackoffMs, now, this.lingerMs, isFull)) {
expiredBatches.add(batch);
Expand Down Expand Up @@ -319,7 +321,7 @@ public ReadyCheckResult ready(Cluster cluster, long nowMs) {
long waitedTimeMs = nowMs - batch.lastAttemptMs;
long timeToWaitMs = backingOff ? retryBackoffMs : lingerMs;
long timeLeftMs = Math.max(timeToWaitMs - waitedTimeMs, 0);
boolean full = deque.size() > 1 || batch.records.isFull();
boolean full = deque.size() > 1 || batch.isFull();
boolean expired = waitedTimeMs >= timeToWaitMs;
boolean sendable = full || expired || exhausted || closed || flushInProgress();
if (sendable && !backingOff) {
Expand Down Expand Up @@ -389,15 +391,15 @@ public Map<Integer, List<RecordBatch>> drain(Cluster cluster,
boolean backoff = first.attempts > 0 && first.lastAttemptMs + retryBackoffMs > now;
// Only drain the batch if it is not during backoff period.
if (!backoff) {
if (size + first.records.sizeInBytes() > maxSize && !ready.isEmpty()) {
if (size + first.sizeInBytes() > maxSize && !ready.isEmpty()) {
// there is a rare case that a single batch size is larger than the request size due
// to compression; in this case we will still eventually send this batch in a single
// request
break;
} else {
RecordBatch batch = deque.pollFirst();
batch.records.close();
size += batch.records.sizeInBytes();
batch.close();
size += batch.sizeInBytes();
ready.add(batch);
batch.drainedMs = now;
}
Expand Down Expand Up @@ -437,7 +439,7 @@ private Deque<RecordBatch> getOrCreateDeque(TopicPartition tp) {
*/
public void deallocate(RecordBatch batch) {
incomplete.remove(batch);
free.deallocate(batch.records.buffer(), batch.records.initialCapacity());
free.deallocate(batch.buffer(), batch.initialCapacity());
}

/**
Expand Down Expand Up @@ -507,7 +509,7 @@ private void abortBatches() {
Deque<RecordBatch> dq = getDeque(batch.topicPartition);
// Close the batch before aborting
synchronized (dq) {
batch.records.close();
batch.close();
dq.remove(batch);
}
batch.done(-1L, Record.NO_TIMESTAMP, new IllegalStateException("Producer is closed forcefully."));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@
*/
package org.apache.kafka.clients.producer.internals;

import java.util.ArrayList;
import java.util.List;

import org.apache.kafka.clients.producer.Callback;
import org.apache.kafka.clients.producer.RecordMetadata;
import org.apache.kafka.common.TopicPartition;
import org.apache.kafka.common.errors.TimeoutException;
import org.apache.kafka.common.record.MemoryRecords;
import org.apache.kafka.common.record.MemoryRecordsBuilder;
import org.apache.kafka.common.record.Record;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;

/**
* A batch of records that is or will be sent.
*
Expand All @@ -39,21 +41,21 @@ public final class RecordBatch {
public final long createdMs;
public long drainedMs;
public long lastAttemptMs;
public final MemoryRecords records;
public final TopicPartition topicPartition;
public final ProduceRequestResult produceFuture;
public long lastAppendTime;
private final List<Thunk> thunks;
private long offsetCounter = 0L;
private boolean retry;
private final MemoryRecordsBuilder recordsBuilder;

public RecordBatch(TopicPartition tp, MemoryRecords records, long now) {
public RecordBatch(TopicPartition tp, MemoryRecordsBuilder recordsBuilder, long now) {
this.createdMs = now;
this.lastAttemptMs = now;
this.records = records;
this.recordsBuilder = recordsBuilder;
this.topicPartition = tp;
this.produceFuture = new ProduceRequestResult();
this.thunks = new ArrayList<Thunk>();
this.thunks = new ArrayList<>();
this.lastAppendTime = createdMs;
this.retry = false;
}
Expand All @@ -64,10 +66,10 @@ public RecordBatch(TopicPartition tp, MemoryRecords records, long now) {
* @return The RecordSend corresponding to this record or null if there isn't sufficient room.
*/
public FutureRecordMetadata tryAppend(long timestamp, byte[] key, byte[] value, Callback callback, long now) {
if (!this.records.hasRoomFor(key, value)) {
if (!recordsBuilder.hasRoomFor(key, value)) {
return null;
} else {
long checksum = this.records.append(offsetCounter++, timestamp, key, value);
long checksum = this.recordsBuilder.append(offsetCounter++, timestamp, key, value);
this.maxRecordSize = Math.max(this.maxRecordSize, Record.recordSize(key, value));
this.lastAppendTime = now;
FutureRecordMetadata future = new FutureRecordMetadata(this.produceFuture, this.recordCount,
Expand All @@ -94,9 +96,8 @@ public void done(long baseOffset, long timestamp, RuntimeException exception) {
baseOffset,
exception);
// execute callbacks
for (int i = 0; i < this.thunks.size(); i++) {
for (Thunk thunk : thunks) {
try {
Thunk thunk = this.thunks.get(i);
if (exception == null) {
// If the timestamp returned by server is NoTimestamp, that means CreateTime is used. Otherwise LogAppendTime is used.
RecordMetadata metadata = new RecordMetadata(this.topicPartition, baseOffset, thunk.future.relativeOffset(),
Expand Down Expand Up @@ -156,7 +157,7 @@ public boolean maybeExpire(int requestTimeoutMs, long retryBackoffMs, long now,
}

if (expire) {
this.records.close();
close();
this.done(-1L, Record.NO_TIMESTAMP,
new TimeoutException("Expiring " + recordCount + " record(s) for " + topicPartition + " due to " + errorMessage));
}
Expand All @@ -177,4 +178,37 @@ public boolean inRetry() {
public void setRetry() {
this.retry = true;
}

public MemoryRecords records() {
return recordsBuilder.build();
}

public int sizeInBytes() {
return recordsBuilder.sizeInBytes();
}

public double compressionRate() {
return recordsBuilder.compressionRate();
}

public boolean isFull() {
return recordsBuilder.isFull();
}

public void close() {
recordsBuilder.close();
}

public ByteBuffer buffer() {
return recordsBuilder.buffer();
}

public int initialCapacity() {
return recordsBuilder.initialCapacity();
}

public boolean isWritable() {
return !recordsBuilder.isClosed();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ private void sendProduceRequest(long now, int destination, short acks, int timeo
final Map<TopicPartition, RecordBatch> recordsByPartition = new HashMap<>(batches.size());
for (RecordBatch batch : batches) {
TopicPartition tp = batch.topicPartition;
produceRecordsByPartition.put(tp, batch.records);
produceRecordsByPartition.put(tp, batch.records());
recordsByPartition.put(tp, batch);
}

Expand Down Expand Up @@ -505,17 +505,17 @@ public void updateProduceRequestMetrics(Map<Integer, List<RecordBatch>> batches)
// per-topic bytes send rate
String topicByteRateName = "topic." + topic + ".bytes";
Sensor topicByteRate = Utils.notNull(this.metrics.getSensor(topicByteRateName));
topicByteRate.record(batch.records.sizeInBytes());
topicByteRate.record(batch.sizeInBytes());

// per-topic compression rate
String topicCompressionRateName = "topic." + topic + ".compression-rate";
Sensor topicCompressionRate = Utils.notNull(this.metrics.getSensor(topicCompressionRateName));
topicCompressionRate.record(batch.records.compressionRate());
topicCompressionRate.record(batch.compressionRate());

// global metrics
this.batchSizeSensor.record(batch.records.sizeInBytes(), now);
this.batchSizeSensor.record(batch.sizeInBytes(), now);
this.queueTimeSensor.record(batch.drainedMs - batch.createdMs, now);
this.compressionRateSensor.record(batch.records.compressionRate());
this.compressionRateSensor.record(batch.compressionRate());
this.maxRecordSizeSensor.record(batch.maxRecordSize, now);
records += batch.recordCount;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
* <p/>
* http://www.apache.org/licenses/LICENSE-2.0
* <p/>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/
package org.apache.kafka.common.record;

import org.apache.kafka.common.utils.AbstractIterator;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

public abstract class AbstractRecords implements Records {

@Override
public boolean hasMatchingShallowMagic(byte magic) {
Iterator<? extends LogEntry> iterator = shallowIterator();
while (iterator.hasNext())
if (iterator.next().magic() != magic)
return false;
return true;
}

/**
* Convert this message set to use the specified message format.
*/
@Override
public Records toMessageFormat(byte toMagic) {
List<LogEntry> converted = new ArrayList<>();
Iterator<LogEntry> deepIterator = deepIterator();
while (deepIterator.hasNext()) {
LogEntry entry = deepIterator.next();
converted.add(LogEntry.create(entry.offset(), entry.record().convert(toMagic)));
}

if (converted.isEmpty()) {
// This indicates that the message is too large, which indicates that the buffer is not large
// enough to hold a full log entry. We just return all the bytes in the file message set.
// Even though the message set does not have the right format version, we expect old clients
// to raise an error to the user after reading the message size and seeing that there
// are not enough available bytes in the response to read the full message.
return this;
} else {
// We use the first message to determine the compression type for the resulting message set.
// This could result in message sets which are either larger or smaller than the original size.
// For example, it could end up larger if most messages were previously compressed, but
// it just so happens that the first one is not. There is also some risk that this can
// cause some timestamp information to be lost (e.g. if the timestamp type was changed) since
// we are essentially merging multiple message sets. However, currently this method is only
// used for down-conversion, so we've ignored the problem.
CompressionType compressionType = shallowIterator().next().record().compressionType();
return MemoryRecords.withLogEntries(compressionType, converted);
}
}

public static int estimatedSize(CompressionType compressionType, Iterable<LogEntry> entries) {
int size = 0;
for (LogEntry entry : entries)
size += entry.sizeInBytes();
// NOTE: 1024 is the minimum block size for snappy encoding
return compressionType == CompressionType.NONE ? size : Math.min(Math.max(size / 2, 1024), 1 << 16);
}

/**
* Get the records from this log buffer (note this requires "deep" iteration into the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the reference to "log buffer " still valid?

* compressed message sets.
* @return An iterator over the records
*/
public Iterator<Record> records() {
return new AbstractIterator<Record>() {
private final Iterator<? extends LogEntry> deepEntries = deepIterator();
@Override
protected Record makeNext() {
if (deepEntries.hasNext())
return deepEntries.next().record();
return allDone();
}
};
}

}
Loading