Skip to content
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,12 @@ private Map<TopicPartition, List<ConsumerRecord<K, V>>> pollOnce(long timeout) {

// init any new fetches (won't resend pending fetches)
Cluster cluster = this.metadata.fetch();
Map<TopicPartition, List<ConsumerRecord<K, V>>> records = fetcher.fetchedRecords();
// Avoid block waiting for response if we already have data available, e.g. from another API call to commit.
if (!records.isEmpty()) {
client.poll(0);
return records;
}
fetcher.initFetches(cluster);
client.poll(timeout);
return fetcher.fetchedRecords();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.kafka.common.TopicPartition;
import org.apache.kafka.common.errors.DisconnectException;
import org.apache.kafka.common.errors.InvalidMetadataException;
import org.apache.kafka.common.errors.OffsetOutOfRangeException;
import org.apache.kafka.common.metrics.Metrics;
import org.apache.kafka.common.metrics.Sensor;
import org.apache.kafka.common.metrics.stats.Avg;
Expand Down Expand Up @@ -78,6 +79,8 @@ public class Fetcher<K, V> {
private final Deserializer<K> keyDeserializer;
private final Deserializer<V> valueDeserializer;

private final Map<TopicPartition, Long> offsetOutOfRangePartitions;

public Fetcher(ConsumerNetworkClient client,
int minBytes,
int maxWaitMs,
Expand Down Expand Up @@ -106,6 +109,7 @@ public Fetcher(ConsumerNetworkClient client,
this.valueDeserializer = valueDeserializer;

this.records = new LinkedList<PartitionRecords<K, V>>();
this.offsetOutOfRangePartitions = new HashMap<>();

this.sensors = new FetchManagerMetrics(metrics, metricGrpPrefix, metricTags);
this.retryBackoffMs = retryBackoffMs;
Expand Down Expand Up @@ -137,6 +141,7 @@ public void onFailure(RuntimeException e) {
/**
* Update the fetch positions for the provided partitions.
* @param partitions
* @throws NoOffsetForPartitionException If no offset is stored for a given partition and no reset policy is available
*/
public void updateFetchPositions(Set<TopicPartition> partitions) {
// reset the fetch position to the committed position
Expand Down Expand Up @@ -257,16 +262,43 @@ private long listOffset(TopicPartition partition, long timestamp) {
}
}

/**
* If any partition from previous fetchResponse contains OffsetOutOfRange error, throw
* OffsetOutOfRangeException and clear the partition list.
*
* @throws OffsetOutOfRangeException If there is OffsetOutOfRange error in fetchResponse
*/
private void throwIfOffsetOutOfRange() throws OffsetOutOfRangeException {
Map<TopicPartition, Long> currentOutOfRangePartitions = new HashMap<>();

// filter offsetOutOfRangePartitions to retain only the fetchable partitions
for (Map.Entry<TopicPartition, Long> entry: this.offsetOutOfRangePartitions.entrySet()) {
if (!subscriptions.isFetchable(entry.getKey())) {
log.debug("Ignoring fetched records for {} since it is no longer fetchable", entry.getKey());
continue;
}
Long consumed = subscriptions.consumed(entry.getKey());

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.

Could you add some comments here for the checking of the consumed position?

if (consumed != null && entry.getValue() == consumed)
currentOutOfRangePartitions.put(entry.getKey(), entry.getValue());
}
this.offsetOutOfRangePartitions.clear();
if (!currentOutOfRangePartitions.isEmpty())
throw new OffsetOutOfRangeException(currentOutOfRangePartitions);
}

/**
* Return the fetched records, empty the record buffer and update the consumed position.
*
* @return The fetched records per partition
* @throws OffsetOutOfRangeException If there is OffsetOutOfRange error in fetchResponse
*/
public Map<TopicPartition, List<ConsumerRecord<K, V>>> fetchedRecords() {
if (this.subscriptions.partitionAssignmentNeeded()) {
return Collections.emptyMap();

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.

Should we clear the exception here? If not, then we'll have the exception thrown after the rebalance.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we should not clear exception, because it should be handled in the same way we handle Fetcher.records -- we will still return already fetched records, instead of clearing it, after the rebalance. What do you think?

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.

Hmm... In that case, we should check that the out of range partitions are still assigned when the exception is thrown, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, good point! I missed it. Actually this is a good reason why we should store map first and create exception later. Let me update the patch.

} else {
Map<TopicPartition, List<ConsumerRecord<K, V>>> drained = new HashMap<>();
throwIfOffsetOutOfRange();

for (PartitionRecords<K, V> part : this.records) {
if (!subscriptions.isFetchable(part.partition)) {
log.debug("Ignoring fetched records for {} since it is no longer fetchable", part.partition);
Expand Down Expand Up @@ -378,8 +410,11 @@ private Map<Node, FetchRequest> createFetchRequests(Cluster cluster) {
fetchable.put(node, fetch);
}

long offset = this.subscriptions.fetched(partition);
fetch.put(partition, new FetchRequest.PartitionData(offset, this.fetchSize));
long fetched = this.subscriptions.fetched(partition);
long consumed = this.subscriptions.consumed(partition);
// Only fetch data for partitions whose previously fetched data has been consumed
if (consumed == fetched)
fetch.put(partition, new FetchRequest.PartitionData(fetched, this.fetchSize));
}
}

Expand Down Expand Up @@ -447,9 +482,12 @@ private void handleFetchResponse(ClientResponse resp, FetchRequest request) {
|| partition.errorCode == Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) {
this.metadata.requestUpdate();
} else if (partition.errorCode == Errors.OFFSET_OUT_OF_RANGE.code()) {
// TODO: this could be optimized by grouping all out-of-range partitions
long fetchOffset = request.fetchData().get(tp).offset;
if (subscriptions.hasDefaultOffsetResetPolicy())
subscriptions.needOffsetReset(tp);
else
this.offsetOutOfRangePartitions.put(tp, fetchOffset);
log.info("Fetch offset {} is out of range, resetting offset", subscriptions.fetched(tp));
subscriptions.needOffsetReset(tp);
} else if (partition.errorCode == Errors.UNKNOWN.code()) {
log.warn("Unknown error fetching data for topic-partition {}", tp);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ public void needOffsetReset(TopicPartition partition) {
needOffsetReset(partition, defaultResetStrategy);
}

public boolean hasDefaultOffsetResetPolicy() {
return defaultResetStrategy != OffsetResetStrategy.NONE;
}

public boolean isOffsetResetNeeded(TopicPartition partition) {
return assignedState(partition).awaitingReset;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,25 @@
*/
package org.apache.kafka.common.errors;

import org.apache.kafka.common.TopicPartition;
import java.util.Map;

/**
* This offset is either larger or smaller than the range of offsets the server has for the given partition.
*
*/
public class OffsetOutOfRangeException extends RetriableException {

private static final long serialVersionUID = 1L;
private Map<TopicPartition, Long> offsetOutOfRangePartitions = null;

public OffsetOutOfRangeException() {
}

public OffsetOutOfRangeException(Map<TopicPartition, Long> offsetOutOfRangePartitions) {
this.offsetOutOfRangePartitions = offsetOutOfRangePartitions;
}

public OffsetOutOfRangeException(String message) {
super(message);
}
Expand All @@ -35,4 +43,8 @@ public OffsetOutOfRangeException(String message, Throwable cause) {
super(message, cause);
}

public Map<TopicPartition, Long> offsetOutOfRangePartitions() {
return offsetOutOfRangePartitions;
}

}