Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -25,6 +25,7 @@
import org.apache.kafka.common.Metric;
import org.apache.kafka.common.MetricName;
import org.apache.kafka.common.TopicPartition;
import org.apache.kafka.common.errors.TimeoutException;
import org.apache.kafka.common.metrics.JmxReporter;
import org.apache.kafka.common.metrics.MetricsContext;
import org.apache.kafka.common.metrics.KafkaMetricsContext;
Expand Down Expand Up @@ -1247,7 +1248,12 @@ public Map<String, Map<Integer, LagInfo>> allLocalStorePartitionLags() {
}

log.debug("Current changelog positions: {}", allChangelogPositions);
final Map<TopicPartition, ListOffsetsResultInfo> allEndOffsets = fetchEndOffsets(allPartitions, adminClient);
final Map<TopicPartition, ListOffsetsResultInfo> allEndOffsets;
try {
allEndOffsets = fetchEndOffsets(allPartitions, adminClient);
} catch (final TimeoutException e) {
throw new StreamsException("Timed out obtaining end offsets from kafka", e);
}
log.debug("Current end offsets :{}", allEndOffsets);

for (final Map.Entry<TopicPartition, ListOffsetsResultInfo> entry : allEndOffsets.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,21 @@
import org.apache.kafka.clients.admin.ListOffsetsResult.ListOffsetsResultInfo;
import org.apache.kafka.clients.admin.OffsetSpec;
import org.apache.kafka.clients.consumer.Consumer;
import org.apache.kafka.common.KafkaException;
import org.apache.kafka.common.KafkaFuture;
import org.apache.kafka.common.Metric;
import org.apache.kafka.common.MetricName;
import org.apache.kafka.common.TopicPartition;
import org.apache.kafka.common.errors.TimeoutException;
import org.apache.kafka.streams.StreamsConfig;
import org.apache.kafka.streams.errors.StreamsException;
import org.apache.kafka.streams.processor.TaskId;

import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -95,19 +99,60 @@ public static Map<MetricName, Metric> producerMetrics(final Collection<StreamsPr
return result;
}

public static Map<TopicPartition, ListOffsetsResultInfo> fetchEndOffsets(final Collection<TopicPartition> partitions,
final Admin adminClient) {
final Map<TopicPartition, ListOffsetsResultInfo> endOffsets;
/**
* @throws StreamsException if the consumer throws an exception
* @throws org.apache.kafka.common.errors.TimeoutException if the request times out
*/
public static Map<TopicPartition, Long> fetchCommittedOffsets(final Set<TopicPartition> partitions,
final Consumer<byte[], byte[]> consumer) {
if (partitions.isEmpty()) {
return Collections.emptyMap();
}

final Map<TopicPartition, Long> committedOffsets;
try {
final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> future = adminClient.listOffsets(
partitions.stream().collect(Collectors.toMap(Function.identity(), tp -> OffsetSpec.latest())))
.all();
endOffsets = future.get();
// those which do not have a committed offset would default to 0
committedOffsets = consumer.committed(partitions).entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue() == null ? 0L : e.getValue().offset()));
} catch (final TimeoutException e) {
LOG.warn("The committed offsets request timed out, try increasing the consumer client's default.api.timeout.ms", e);
throw e;
} catch (final KafkaException e) {
LOG.warn("The committed offsets request failed.", e);
throw new StreamsException(String.format("Failed to retrieve end offsets for %s", partitions), e);
}

return committedOffsets;
}

public static KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> fetchEndOffsetsFuture(final Collection<TopicPartition> partitions,
final Admin adminClient) {
return adminClient.listOffsets(
partitions.stream().collect(Collectors.toMap(Function.identity(), tp -> OffsetSpec.latest())))
.all();
}

/**
* A helper method that wraps the {@code Future#get} call and rethrows any thrown exception as a StreamsException
* @throws StreamsException if the admin client request throws an exception
*/
public static Map<TopicPartition, ListOffsetsResultInfo> getEndOffsets(final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> endOffsetsFuture) {
try {
return endOffsetsFuture.get();
} catch (final RuntimeException | InterruptedException | ExecutionException e) {
LOG.warn("listOffsets request failed.", e);
LOG.warn("The listOffsets request failed.", e);
throw new StreamsException("Unable to obtain end offsets from kafka", e);
}
return endOffsets;
}

/**
* @throws StreamsException if the admin client request throws an exception
*/
public static Map<TopicPartition, ListOffsetsResultInfo> fetchEndOffsets(final Collection<TopicPartition> partitions,
final Admin adminClient) {
if (partitions.isEmpty()) {
return Collections.emptyMap();
}
return getEndOffsets(fetchEndOffsetsFuture(partitions, adminClient));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,13 @@ public Set<InternalTopicConfig> nonSourceChangelogTopics() {
return topicConfigs;
}

/**
* Returns the topic names for any optimized source changelogs
*/
public Set<String> sourceTopicChangelogs() {
return sourceTopics.stream().filter(stateChangelogTopics::containsKey).collect(Collectors.toSet());
}

@Override
public boolean equals(final Object o) {
if (o instanceof TopicsInfo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import java.util.Set;
import java.util.stream.Collectors;

import static org.apache.kafka.streams.processor.internals.ClientUtils.fetchCommittedOffsets;

/**
* ChangelogReader is created and maintained by the stream thread and used for both updating standby tasks and
* restoring active tasks. It manages the restore consumer, including its assigned partitions, when to pause / resume
Expand Down Expand Up @@ -546,23 +548,14 @@ private void restoreChangelog(final ChangelogMetadata changelogMetadata) {
}

private Map<TopicPartition, Long> committedOffsetForChangelogs(final Set<TopicPartition> partitions) {
if (partitions.isEmpty())
return Collections.emptyMap();

Comment thread
vvcephei marked this conversation as resolved.
Outdated
final Map<TopicPartition, Long> committedOffsets;
try {
// those do not have a committed offset would default to 0
committedOffsets = mainConsumer.committed(partitions).entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue() == null ? 0L : e.getValue().offset()));
committedOffsets = fetchCommittedOffsets(partitions, mainConsumer);
} catch (final TimeoutException e) {
// if it timed out we just retry next time.
// if it timed out we just retry next time
return Collections.emptyMap();
} catch (final KafkaException e) {
throw new StreamsException(String.format("Failed to retrieve end offsets for %s", partitions), e);
}

lastUpdateOffsetTime = time.milliseconds();

return committedOffsets;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
import org.apache.kafka.common.Cluster;
import org.apache.kafka.common.Configurable;
import org.apache.kafka.common.KafkaException;
import org.apache.kafka.common.KafkaFuture;
import org.apache.kafka.common.Node;
import org.apache.kafka.common.PartitionInfo;
import org.apache.kafka.common.TopicPartition;
import org.apache.kafka.common.errors.TimeoutException;
import org.apache.kafka.common.utils.LogContext;
import org.apache.kafka.common.utils.Time;
import org.apache.kafka.common.utils.Utils;
Expand Down Expand Up @@ -72,7 +74,8 @@

import static java.util.Comparator.comparingLong;
import static java.util.UUID.randomUUID;
import static org.apache.kafka.streams.processor.internals.ClientUtils.fetchEndOffsets;
import static org.apache.kafka.streams.processor.internals.ClientUtils.fetchCommittedOffsets;
import static org.apache.kafka.streams.processor.internals.ClientUtils.fetchEndOffsetsFuture;
import static org.apache.kafka.streams.processor.internals.assignment.StreamsAssignmentProtocolVersions.EARLIEST_PROBEABLE_VERSION;
import static org.apache.kafka.streams.processor.internals.assignment.StreamsAssignmentProtocolVersions.LATEST_SUPPORTED_VERSION;
import static org.apache.kafka.streams.processor.internals.assignment.StreamsAssignmentProtocolVersions.UNKNOWN;
Expand Down Expand Up @@ -630,12 +633,13 @@ private void checkAllPartitions(final Set<String> allSourceTopics,
}

/**
* Resolve changelog topic metadata and create them if necessary. Fills in the changelogsByStatefulTask map
* and returns the set of changelogs which were newly created.
* Resolve changelog topic metadata and create them if necessary. Fills in the changelogsByStatefulTask map and
* the optimizedSourceChangelogs set and returns the set of changelogs which were newly created.
*/
private Set<String> prepareChangelogTopics(final Map<Integer, TopicsInfo> topicGroups,
final Map<Integer, Set<TaskId>> tasksForTopicGroup,
final Map<TaskId, Set<TopicPartition>> changelogsByStatefulTask) {
final Map<TaskId, Set<TopicPartition>> changelogsByStatefulTask,
final Set<String> optimizedSourceChangelogs) {
// add tasks to state change log topic subscribers
final Map<String, InternalTopicConfig> changelogTopicMetadata = new HashMap<>();
for (final Map.Entry<Integer, TopicsInfo> entry : topicGroups.entrySet()) {
Expand Down Expand Up @@ -671,6 +675,8 @@ private Set<String> prepareChangelogTopics(final Map<Integer, TopicsInfo> topicG
topicConfig.setNumberOfPartitions(numPartitions);
changelogTopicMetadata.put(topicConfig.name(), topicConfig);
}

optimizedSourceChangelogs.addAll(topicsInfo.sourceTopicChangelogs());
}

final Set<String> newlyCreatedTopics = internalTopicManager.makeReady(changelogTopicMetadata);
Expand All @@ -692,11 +698,19 @@ private boolean assignTasksToClients(final Set<String> allSourceTopics,
populateTasksForMaps(taskForPartition, tasksForTopicGroup, allSourceTopics, partitionsForTask, fullMetadata);

final Map<TaskId, Set<TopicPartition>> changelogsByStatefulTask = new HashMap<>();
final Set<String> newlyCreatedChangelogs = prepareChangelogTopics(topicGroups, tasksForTopicGroup, changelogsByStatefulTask);
final Set<String> optimizedSourceChangelogs = new HashSet<>();
final Set<String> newlyCreatedChangelogs =
prepareChangelogTopics(topicGroups, tasksForTopicGroup, changelogsByStatefulTask, optimizedSourceChangelogs);

final Map<UUID, ClientState> clientStates = new HashMap<>();
final boolean lagComputationSuccessful =
populateClientStatesMap(clientStates, clientMetadataMap, taskForPartition, changelogsByStatefulTask, newlyCreatedChangelogs);
populateClientStatesMap(clientStates,
clientMetadataMap,
taskForPartition,
changelogsByStatefulTask,
newlyCreatedChangelogs,
optimizedSourceChangelogs
);

final Set<TaskId> allTasks = partitionsForTask.keySet();
final Set<TaskId> statefulTasks = changelogsByStatefulTask.keySet();
Expand Down Expand Up @@ -747,7 +761,8 @@ private boolean populateClientStatesMap(final Map<UUID, ClientState> clientState
final Map<UUID, ClientMetadata> clientMetadataMap,
final Map<TopicPartition, TaskId> taskForPartition,
final Map<TaskId, Set<TopicPartition>> changelogsByStatefulTask,
final Set<String> newlyCreatedChangelogs) {
final Set<String> newlyCreatedChangelogs,
final Set<String> optimizedSourceChangelogs) {
boolean fetchEndOffsetsSuccessful;
Map<TaskId, Long> allTaskEndOffsetSums;
try {
Expand All @@ -756,18 +771,36 @@ private boolean populateClientStatesMap(final Map<UUID, ClientState> clientState
.flatMap(Collection::stream)
.collect(Collectors.toList());

final Collection<TopicPartition> allPreexistingChangelogPartitions = new ArrayList<>(allChangelogPartitions);
allPreexistingChangelogPartitions.removeIf(partition -> newlyCreatedChangelogs.contains(partition.topic()));
final Set<TopicPartition> preexistingChangelogPartitions = new HashSet<>();
final Set<TopicPartition> preexistingSourceChangelogPartitions = new HashSet<>();
final Set<TopicPartition> newlyCreatedChangelogPartitions = new HashSet<>();
for (final TopicPartition changelog : allChangelogPartitions) {
if (newlyCreatedChangelogs.contains(changelog.topic())) {
newlyCreatedChangelogPartitions.add(changelog);
} else if (optimizedSourceChangelogs.contains(changelog.topic())) {
preexistingSourceChangelogPartitions.add(changelog);
} else {
preexistingChangelogPartitions.add(changelog);
}
}

// Make the listOffsets request first so it can fetch the offsets for non-source changelogs
// asynchronously while we use the blocking Consumer#committed call to fetch source-changelog offsets
final KafkaFuture<Map<TopicPartition, ListOffsetsResultInfo>> endOffsetsFuture =
fetchEndOffsetsFuture(preexistingChangelogPartitions, adminClient);

final Collection<TopicPartition> allNewlyCreatedChangelogPartitions = new ArrayList<>(allChangelogPartitions);
allNewlyCreatedChangelogPartitions.removeAll(allPreexistingChangelogPartitions);
final Map<TopicPartition, Long> sourceChangelogEndOffsets =
fetchCommittedOffsets(preexistingSourceChangelogPartitions, taskManager.mainConsumer());

final Map<TopicPartition, ListOffsetsResultInfo> endOffsets =
fetchEndOffsets(allPreexistingChangelogPartitions, adminClient);
final Map<TopicPartition, ListOffsetsResultInfo> endOffsets = ClientUtils.getEndOffsets(endOffsetsFuture);

allTaskEndOffsetSums = computeEndOffsetSumsByTask(endOffsets, changelogsByStatefulTask, allNewlyCreatedChangelogPartitions);
allTaskEndOffsetSums = computeEndOffsetSumsByTask(
changelogsByStatefulTask,
endOffsets,
sourceChangelogEndOffsets,
newlyCreatedChangelogPartitions);
fetchEndOffsetsSuccessful = true;
} catch (final StreamsException e) {
} catch (final StreamsException | TimeoutException e) {

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.

@vvcephei I've been wondering if maybe we should only catch the TimeoutException, and interpret a StreamsException as fatal (like IllegalStateException for example). This is how we were using Consumer#committed in the StoreChangelogReader, and AFAICT that only throws KafkaException on "unrecoverable errors" (quoted from javadocs)
But I can't tell whether the Admin's listOffsets might throw on transient errors, so I'm leaning towards catching both just to be safe. WDYT?

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.

That sounds reasonable, but I think if you throw an exception in the assignor, it just calls the assignor again in a tight loop, which seems worse than backing off and trying again later.

If you want to propose this change, maybe you can verify what exactly happens if we throw.

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.

if you throw an exception in the assignor, it just calls the assignor again in a tight loop

Wouldn't the leader thread just die? Not saying that that's ideal, either. But it's at least in line with how exceptions thrown by other admin client requests in the assignment are currently handled.

allTaskEndOffsetSums = changelogsByStatefulTask.keySet().stream().collect(Collectors.toMap(t -> t, t -> UNKNOWN_OFFSET_SUM));
fetchEndOffsetsSuccessful = false;
}
Expand All @@ -784,13 +817,16 @@ private boolean populateClientStatesMap(final Map<UUID, ClientState> clientState
}

/**
* @param endOffsets the listOffsets result from the adminClient, or null if the request failed
* @param changelogsByStatefulTask map from stateful task to its set of changelog topic partitions
* @param endOffsets the listOffsets result from the adminClient
* @param sourceChangelogEndOffsets the end (committed) offsets of optimized source changelogs
* @param newlyCreatedChangelogPartitions any changelogs that were just created duringthis assignment
*
* @return Map from stateful task to its total end offset summed across all changelog partitions
*/
private Map<TaskId, Long> computeEndOffsetSumsByTask(final Map<TopicPartition, ListOffsetsResultInfo> endOffsets,
final Map<TaskId, Set<TopicPartition>> changelogsByStatefulTask,
private Map<TaskId, Long> computeEndOffsetSumsByTask(final Map<TaskId, Set<TopicPartition>> changelogsByStatefulTask,
final Map<TopicPartition, ListOffsetsResultInfo> endOffsets,
final Map<TopicPartition, Long> sourceChangelogEndOffsets,
final Collection<TopicPartition> newlyCreatedChangelogPartitions) {
final Map<TaskId, Long> taskEndOffsetSums = new HashMap<>();
for (final Map.Entry<TaskId, Set<TopicPartition>> taskEntry : changelogsByStatefulTask.entrySet()) {
Expand All @@ -802,13 +838,13 @@ private Map<TaskId, Long> computeEndOffsetSumsByTask(final Map<TopicPartition, L
final long changelogEndOffset;
if (newlyCreatedChangelogPartitions.contains(changelog)) {
changelogEndOffset = 0L;
} else if (sourceChangelogEndOffsets.containsKey(changelog)) {
changelogEndOffset = sourceChangelogEndOffsets.get(changelog);
} else if (endOffsets.containsKey(changelog)) {
changelogEndOffset = endOffsets.get(changelog).offset();
} else {
final ListOffsetsResultInfo offsetResult = endOffsets.get(changelog);
if (offsetResult == null) {
log.debug("Fetched end offsets did not contain the changelog {} of task {}", changelog, task);
throw new IllegalStateException("Could not get end offset for " + changelog);
}
changelogEndOffset = offsetResult.offset();
log.debug("Fetched offsets did not contain the changelog {} of task {}", changelog, task);
throw new IllegalStateException("Could not get end offset for " + changelog);
}
final long newEndOffsetSum = taskEndOffsetSums.get(task) + changelogEndOffset;
if (newEndOffsetSum < 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ void setMainConsumer(final Consumer<byte[], byte[]> mainConsumer) {
this.mainConsumer = mainConsumer;
}

Consumer<byte[], byte[]> mainConsumer() {
return mainConsumer;
}

public UUID processId() {
return processId;
}
Expand Down
Loading