diff --git a/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java b/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java index f7d4bcdd468d0..da2a16bf12daa 100644 --- a/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java +++ b/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java @@ -39,6 +39,7 @@ public interface Callback { * RecordBatchTooLargeException * RecordTooLargeException * UnknownServerException + * UnknownProducerIdException * * Retriable exceptions (transient, may be covered by increasing #.retries): * diff --git a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java b/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java index a5d749a4ff089..0d13b139519e4 100644 --- a/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java +++ b/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java @@ -1149,7 +1149,9 @@ public List partitionsFor(String topic) { * block forever. *

* - * @throws InterruptException If the thread is interrupted while blocked + * @throws InterruptException If the thread is interrupted while blocked. + * @throws KafkaException If a unexpected error occurs while trying to close the client, this error should be treated + * as fatal and indicate the client is no longer functionable. */ @Override public void close() { @@ -1169,7 +1171,9 @@ public void close() { * * @param timeout The maximum time to wait for producer to complete any pending requests. The value should be * non-negative. Specifying a timeout of zero means do not wait for pending send requests to complete. - * @throws InterruptException If the thread is interrupted while blocked + * @throws InterruptException If the thread is interrupted while blocked. + * @throws KafkaException If a unexpected error occurs while trying to close the client, this error should be treated + * as fatal and indicate the client is no longer functionable. * @throws IllegalArgumentException If the timeout is negative. * */ diff --git a/streams/src/main/java/org/apache/kafka/streams/errors/TaskMigratedException.java b/streams/src/main/java/org/apache/kafka/streams/errors/TaskMigratedException.java index 419543302c6a6..b8ff3fa4134e4 100644 --- a/streams/src/main/java/org/apache/kafka/streams/errors/TaskMigratedException.java +++ b/streams/src/main/java/org/apache/kafka/streams/errors/TaskMigratedException.java @@ -18,7 +18,7 @@ import org.apache.kafka.common.TopicPartition; -import org.apache.kafka.streams.processor.internals.Task; +import org.apache.kafka.streams.processor.TaskId; /** * Indicates that a task got migrated to another thread. @@ -28,43 +28,41 @@ public class TaskMigratedException extends StreamsException { private final static long serialVersionUID = 1L; - private final Task task; + private final TaskId taskId; - // this is for unit test only - public TaskMigratedException() { - super("A task has been migrated unexpectedly", null); - - this.task = null; - } - - public TaskMigratedException(final Task task, + public TaskMigratedException(final TaskId taskId, final TopicPartition topicPartition, final long endOffset, final long pos) { - super(String.format("Log end offset of %s should not change while restoring: old end offset %d, current offset %d", - topicPartition, - endOffset, - pos), - null); - - this.task = task; + this(taskId, String.format("Log end offset of %s should not change while restoring: old end offset %d, current offset %d", + topicPartition, + endOffset, + pos), null); } - public TaskMigratedException(final Task task) { - super(String.format("Task %s is unexpectedly closed during processing", task.id()), null); - - this.task = task; + public TaskMigratedException(final TaskId taskId) { + this(taskId, String.format("Task %s is unexpectedly closed during processing", taskId), null); } - public TaskMigratedException(final Task task, + public TaskMigratedException(final TaskId taskId, final Throwable throwable) { - super(String.format("Client request for task %s has been fenced due to a rebalance", task.id()), throwable); + this(taskId, String.format("Client request for task %s has been fenced due to a rebalance", taskId), throwable); + } - this.task = task; + public TaskMigratedException(final TaskId taskId, + final String message, + final Throwable throwable) { + super(message, throwable); + this.taskId = taskId; } - public Task migratedTask() { - return task; + public TaskId migratedTaskId() { + return taskId; } + // this is for unit test only + // TODO K9113: remove this after we've refactored AssignedTasksTests + public TaskMigratedException() { + this(null, "A task has been migrated unexpectedly", null); + } } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractTask.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractTask.java index a18b2b6a3590a..2ff6a27fb8789 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractTask.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractTask.java @@ -23,7 +23,6 @@ import org.apache.kafka.streams.errors.LockException; import org.apache.kafka.streams.errors.ProcessorStateException; import org.apache.kafka.streams.errors.StreamsException; -import org.apache.kafka.streams.errors.TaskMigratedException; import org.apache.kafka.streams.processor.ProcessorContext; import org.apache.kafka.streams.processor.StateStore; import org.apache.kafka.streams.processor.TaskId; @@ -43,7 +42,6 @@ public abstract class AbstractTask implements Task { final Set partitions; final Consumer consumer; final String logPrefix; - final boolean eosEnabled; final Logger log; final LogContext logContext; final StateDirectory stateDirectory; @@ -61,8 +59,8 @@ public abstract class AbstractTask implements Task { final Set partitions, final ProcessorTopology topology, final Consumer consumer, - final ChangelogReader changelogReader, final boolean isStandby, + final ProcessorStateManager stateMgr, final StateDirectory stateDirectory, final StreamsConfig config) { this.id = id; @@ -70,28 +68,14 @@ public abstract class AbstractTask implements Task { this.partitions = new HashSet<>(partitions); this.topology = topology; this.consumer = consumer; - this.eosEnabled = StreamsConfig.EXACTLY_ONCE.equals(config.getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG)); this.stateDirectory = stateDirectory; + this.stateMgr = stateMgr; + final String threadIdPrefix = String.format("stream-thread [%s] ", Thread.currentThread().getName()); this.logPrefix = threadIdPrefix + String.format("%s [%s] ", isStandby ? "standby-task" : "task", id); this.logContext = new LogContext(logPrefix); this.log = logContext.logger(getClass()); - - // create the processor state manager - try { - stateMgr = new ProcessorStateManager( - id, - partitions, - isStandby, - stateDirectory, - topology.storeToChangelogTopic(), - changelogReader, - eosEnabled, - logContext); - } catch (final IOException e) { - throw new ProcessorStateException(String.format("%sError while creating the state manager", logPrefix), e); - } } @Override @@ -135,10 +119,6 @@ public String toString() { return toString(""); } - public boolean isEosEnabled() { - return eosEnabled; - } - /** * Produces a string representation containing useful information about a Task starting with the given indent. * This is useful in debugging scenarios. @@ -169,19 +149,6 @@ public String toString(final String indent) { return sb.toString(); } - /** - * Flush all state stores owned by this task - */ - void flushState() { - try { - stateMgr.flush(); - } catch (final ProcessorStateException e) { - if (e.getCause() instanceof RecoverableClientException) { - throw new TaskMigratedException(this, e); - } - } - } - /** * Package-private for testing only * diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedStandbyTasks.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedStandbyTasks.java index 299390bfd1666..806301bfa49cf 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedStandbyTasks.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedStandbyTasks.java @@ -76,7 +76,7 @@ List closeRevokedStandbyTasks(final Map runningTasksToSus firstException.compareAndSet(null, e); try { prevActiveTasks.remove(id); - task.close(false, false); + task.close(false); } catch (final RuntimeException f) { log.error( "After suspending failed, closing the same stream task {} failed again due to the following error:", @@ -179,7 +179,7 @@ private RuntimeException closeNonRunningTasks(final Set nonRunningTasksT for (final TaskId id : nonRunningTasksToClose) { final StreamTask task = created.get(id); - firstException.compareAndSet(null, closeNonRunning(false, task, closedTaskChangelogs)); + firstException.compareAndSet(null, closeNonRunning(true, task, closedTaskChangelogs)); } return firstException.get(); @@ -192,19 +192,17 @@ RuntimeException closeRestoringTasks(final Set restoringTasksToClose, for (final TaskId id : restoringTasksToClose) { final StreamTask task = restoring.get(id); - firstException.compareAndSet(null, closeRestoring(false, task, closedTaskChangelogs)); + firstException.compareAndSet(null, closeRestoring(true, task, closedTaskChangelogs)); } return firstException.get(); } - private RuntimeException closeRunning(final boolean isZombie, - final StreamTask task) { + private RuntimeException closeRunning(final boolean clean, final StreamTask task) { removeTaskFromAllStateMaps(task, Collections.emptyMap()); try { - final boolean clean = !isZombie; - task.close(clean, isZombie); + task.close(clean); } catch (final RuntimeException e) { log.error("Failed to close the stream task {}", task.id(), e); return e; @@ -213,14 +211,14 @@ private RuntimeException closeRunning(final boolean isZombie, return null; } - private RuntimeException closeNonRunning(final boolean isZombie, + private RuntimeException closeNonRunning(final boolean clean, final StreamTask task, final List closedTaskChangelogs) { removeTaskFromAllStateMaps(task, Collections.emptyMap()); closedTaskChangelogs.addAll(task.changelogPartitions()); try { - task.close(false, isZombie); + task.close(clean); } catch (final RuntimeException e) { log.error("Failed to close the stream task {}", task.id(), e); return e; @@ -230,14 +228,13 @@ private RuntimeException closeNonRunning(final boolean isZombie, } // Since a restoring task has not had its topology initialized yet, we need only close the state manager - private RuntimeException closeRestoring(final boolean isZombie, + private RuntimeException closeRestoring(final boolean clean, final StreamTask task, final List closedTaskChangelogs) { removeTaskFromAllStateMaps(task, Collections.emptyMap()); closedTaskChangelogs.addAll(task.changelogPartitions()); try { - final boolean clean = !isZombie; task.closeStateManager(clean); } catch (final RuntimeException e) { log.error("Failed to close the restoring stream task {} due to the following error:", task.id(), e); @@ -247,13 +244,11 @@ private RuntimeException closeRestoring(final boolean isZombie, return null; } - private RuntimeException closeSuspended(final boolean isZombie, - final StreamTask task) { + private RuntimeException closeSuspended(final boolean clean, final StreamTask task) { removeTaskFromAllStateMaps(task, Collections.emptyMap()); try { - final boolean clean = !isZombie; - task.closeSuspended(clean, null); + task.closeSuspended(clean); } catch (final RuntimeException e) { log.error("Failed to close the suspended stream task {} due to the following error:", task.id(), e); return e; @@ -270,7 +265,7 @@ RuntimeException closeNotAssignedSuspendedTasks(final Set revokedTasks) final StreamTask suspendedTask = suspended.get(revokedTask); if (suspendedTask != null) { - firstException.compareAndSet(null, closeSuspended(false, suspendedTask)); + firstException.compareAndSet(null, closeSuspended(true, suspendedTask)); } else { log.debug("Revoked stream task {} could not be found in suspended, may have already been closed", revokedTask); } @@ -287,16 +282,16 @@ RuntimeException closeAllTasksAsZombies() { for (final TaskId id : allAssignedTaskIds()) { if (running.containsKey(id)) { log.debug("Closing the zombie running stream task {}.", id); - firstException.compareAndSet(null, closeRunning(true, running.get(id))); + firstException.compareAndSet(null, closeRunning(false, running.get(id))); } else if (created.containsKey(id)) { log.debug("Closing the zombie created stream task {}.", id); - firstException.compareAndSet(null, closeNonRunning(true, created.get(id), changelogs)); + firstException.compareAndSet(null, closeNonRunning(false, created.get(id), changelogs)); } else if (restoring.containsKey(id)) { log.debug("Closing the zombie restoring stream task {}.", id); - firstException.compareAndSet(null, closeRestoring(true, restoring.get(id), changelogs)); + firstException.compareAndSet(null, closeRestoring(false, restoring.get(id), changelogs)); } else if (suspended.containsKey(id)) { log.debug("Closing the zombie suspended stream task {}.", id); - firstException.compareAndSet(null, closeSuspended(true, suspended.get(id))); + firstException.compareAndSet(null, closeSuspended(false, suspended.get(id))); } } @@ -330,7 +325,7 @@ boolean maybeResumeSuspendedTask(final TaskId taskId, return true; } else { log.warn("Couldn't resume stream task {} assigned partitions {}, task partitions {}", taskId, partitions, task.partitions()); - task.closeSuspended(true, null); + task.closeSuspended(true); } } return false; diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedTasks.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedTasks.java index 125d3e315e456..77451b9e3e0d6 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedTasks.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AssignedTasks.java @@ -99,7 +99,7 @@ Collection running() { void tryCloseZombieTask(final T task) { try { - task.close(false, true); + task.close(false); } catch (final RuntimeException e) { log.warn("Failed to close zombie {} {} due to {}; ignore and proceed.", taskTypeName, task.id(), e.toString()); } @@ -279,13 +279,13 @@ void shutdown(final boolean clean) { } void closeTask(final T task, final boolean clean) { - task.close(clean, false); + task.close(clean); } private void closeUnclean(final T task) { log.info("Try to close {} {} unclean.", task.getClass().getSimpleName(), task.id()); try { - task.close(false, false); + task.close(false); } catch (final RuntimeException fatalException) { log.error("Failed while closing {} {} due to the following error:", task.getClass().getSimpleName(), diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStateManagerImpl.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStateManagerImpl.java index 18574c7e52996..965e87b355dd4 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStateManagerImpl.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStateManagerImpl.java @@ -163,7 +163,6 @@ public void reinitializeStateStoresForPartitions(final Collection partitionForTopic; - private final boolean eosEnabled; private final File baseDir; private OffsetCheckpoint checkpointFile; private final Map checkpointFileCache = new HashMap<>(); @@ -78,7 +80,6 @@ public class ProcessorStateManager implements StateManager { /** * @throws ProcessorStateException if the task directory does not exist and could not be created - * @throws IOException if any severe error happens while creating or locking the state directory */ public ProcessorStateManager(final TaskId taskId, final Collection sources, @@ -86,14 +87,11 @@ public ProcessorStateManager(final TaskId taskId, final StateDirectory stateDirectory, final Map storeToChangelogTopic, final ChangelogReader changelogReader, - final boolean eosEnabled, - final LogContext logContext) throws IOException { - this.eosEnabled = eosEnabled; - - log = logContext.logger(ProcessorStateManager.class); + final LogContext logContext) throws ProcessorStateException { + this.log = logContext.logger(ProcessorStateManager.class); this.taskId = taskId; this.changelogReader = changelogReader; - logPrefix = String.format("task [%s] ", taskId); + this.logPrefix = format("task [%s] ", taskId); partitionForTopic = new HashMap<>(); for (final TopicPartition source : sources) { @@ -108,20 +106,35 @@ public ProcessorStateManager(final TaskId taskId, baseDir = stateDirectory.directoryForTask(taskId); checkpointFile = new OffsetCheckpoint(new File(baseDir, CHECKPOINT_FILE_NAME)); - initialLoadedCheckpoints = checkpointFile.read(); - log.trace("Checkpointable offsets read from checkpoint: {}", initialLoadedCheckpoints); + try { + // load the checkpointed offsets and then delete the checkpoint file + initialLoadedCheckpoints = checkpointFile.read(); - if (eosEnabled) { - // with EOS enabled, there should never be a checkpoint file _during_ processing. - // delete the checkpoint file after loading its stored offsets. checkpointFile.delete(); checkpointFile = null; + log.trace("Checkpointable offsets read from checkpoint: {}", initialLoadedCheckpoints); + } catch (final IOException e) { + throw new ProcessorStateException(format("%sError loading and deleting checkpoint file when creating the state manager", + logPrefix), e); } log.debug("Created state store manager for task {}", taskId); } + void clearCheckpoints() throws ProcessorStateException { + try { + if (checkpointFile != null) { + checkpointFile.delete(); + checkpointFile = null; + + checkpointFileCache.clear(); + } + } catch (final IOException e) { + throw new ProcessorStateException(format("%sError while deleting the checkpoint file", logPrefix), e); + } + + } public static String storeChangelogTopic(final String applicationId, final String storeName) { @@ -140,11 +153,11 @@ public void register(final StateStore store, log.debug("Registering state store {} to its state manager", storeName); if (CHECKPOINT_FILE_NAME.equals(storeName)) { - throw new IllegalArgumentException(String.format("%sIllegal store name: %s", logPrefix, storeName)); + throw new IllegalArgumentException(format("%sIllegal store name: %s", logPrefix, storeName)); } if (registeredStores.containsKey(storeName) && registeredStores.get(storeName).isPresent()) { - throw new IllegalArgumentException(String.format("%sStore %s has already been registered.", logPrefix, storeName)); + throw new IllegalArgumentException(format("%sStore %s has already been registered.", logPrefix, storeName)); } // check that the underlying change log topic exist or not @@ -188,7 +201,6 @@ public void register(final StateStore store, public void reinitializeStateStoresForPartitions(final Collection partitions, final InternalProcessorContext processorContext) { StateManagerUtil.reinitializeStateStoresForPartitions(log, - eosEnabled, baseDir, registeredStores, storeToChangelogTopic, @@ -199,15 +211,6 @@ public void reinitializeStateStoresForPartitions(final Collection checkpointed() { updateCheckpointFileCache(emptyMap()); @@ -239,7 +242,7 @@ void updateStandbyStates(final TopicPartition storePartition, try { restoreCallback.restoreBatch(convertedRecords); } catch (final RuntimeException e) { - throw new ProcessorStateException(String.format("%sException caught while trying to restore state from %s", logPrefix, storePartition), e); + throw new ProcessorStateException(format("%sException caught while trying to restore state from %s", logPrefix, storePartition), e); } } @@ -262,9 +265,14 @@ public StateStore getStore(final String name) { return registeredStores.getOrDefault(name, Optional.empty()).orElse(null); } + /** + * @throws TaskMigratedException recoverable error sending changelog records that would cause the task to be removed + * @throws StreamsException fatal error when flushing the state store, for example sending changelog records failed + * or flushing state store get IO errors; such error should cause the thread to die + */ @Override public void flush() { - ProcessorStateException firstException = null; + RuntimeException firstException = null; // attempting to flush the stores if (!registeredStores.isEmpty()) { log.debug("Flushing all stores registered in the state manager"); @@ -276,7 +284,12 @@ public void flush() { store.flush(); } catch (final RuntimeException e) { if (firstException == null) { - firstException = new ProcessorStateException(String.format("%sFailed to flush state store %s", logPrefix, store.name()), e); + // do NOT wrap the error if it is actually caused by Streams itself + if (e instanceof StreamsException) + firstException = e; + else + firstException = new ProcessorStateException( + format("%sFailed to flush state store %s", logPrefix, store.name()), e); } log.error("Failed to flush state store {}: ", store.name(), e); } @@ -313,7 +326,7 @@ public void close(final boolean clean) throws ProcessorStateException { registeredStores.put(store.name(), Optional.empty()); } catch (final RuntimeException e) { if (firstException == null) { - firstException = new ProcessorStateException(String.format("%sFailed to close state store %s", logPrefix, store.name()), e); + firstException = new ProcessorStateException(format("%sFailed to close state store %s", logPrefix, store.name()), e); } log.error("Failed to close state store {}: ", store.name(), e); } @@ -323,15 +336,6 @@ public void close(final boolean clean) throws ProcessorStateException { } } - if (!clean && eosEnabled) { - // delete the checkpoint file if this is an unclean close - try { - clearCheckpoints(); - } catch (final IOException e) { - throw new ProcessorStateException(String.format("%sError while deleting the checkpoint file", logPrefix), e); - } - } - if (firstException != null) { throw firstException; } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollector.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollector.java index b8b99a3f7b9ba..7b708ce6f2b8e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollector.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollector.java @@ -16,6 +16,7 @@ */ package org.apache.kafka.streams.processor.internals; +import org.apache.kafka.clients.consumer.OffsetAndMetadata; import org.apache.kafka.clients.producer.Producer; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.header.Headers; @@ -26,6 +27,11 @@ public interface RecordCollector extends AutoCloseable { + /** + * Initialize the record collector + */ + void initialize(); + void send(final String topic, final K key, final V value, @@ -44,11 +50,7 @@ void send(final String topic, final Serializer valueSerializer, final StreamPartitioner partitioner); - /** - * Initialize the collector with a producer. - * @param producer the producer that should be used by this collector - */ - void init(final Producer producer); + void commit(final Map offsets); /** * Flush the internal {@link Producer}. @@ -70,6 +72,8 @@ void send(final String topic, /** * A supplier of a {@link RecordCollectorImpl} instance. */ + // TODO: after we have done KAFKA-9088 we should just add this function + // to InternalProcessorContext interface interface Supplier { /** * Get the record collector. diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollectorImpl.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollectorImpl.java index 2451afcdf2c14..721fd1c2349f0 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollectorImpl.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/RecordCollectorImpl.java @@ -16,10 +16,11 @@ */ package org.apache.kafka.streams.processor.internals; -import org.apache.kafka.clients.producer.Callback; +import org.apache.kafka.clients.consumer.CommitFailedException; +import org.apache.kafka.clients.consumer.Consumer; +import org.apache.kafka.clients.consumer.OffsetAndMetadata; import org.apache.kafka.clients.producer.Producer; import org.apache.kafka.clients.producer.ProducerRecord; -import org.apache.kafka.clients.producer.RecordMetadata; import org.apache.kafka.common.KafkaException; import org.apache.kafka.common.PartitionInfo; import org.apache.kafka.common.TopicPartition; @@ -27,6 +28,7 @@ import org.apache.kafka.common.errors.AuthorizationException; import org.apache.kafka.common.errors.InvalidTopicException; import org.apache.kafka.common.errors.OffsetMetadataTooLarge; +import org.apache.kafka.common.errors.OutOfOrderSequenceException; import org.apache.kafka.common.errors.ProducerFencedException; import org.apache.kafka.common.errors.RetriableException; import org.apache.kafka.common.errors.SecurityDisabledException; @@ -38,10 +40,15 @@ import org.apache.kafka.common.metrics.Sensor; import org.apache.kafka.common.serialization.Serializer; import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.streams.StreamsConfig; import org.apache.kafka.streams.errors.ProductionExceptionHandler; import org.apache.kafka.streams.errors.ProductionExceptionHandler.ProductionExceptionHandlerResponse; import org.apache.kafka.streams.errors.StreamsException; +import org.apache.kafka.streams.errors.TaskMigratedException; import org.apache.kafka.streams.processor.StreamPartitioner; +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl; +import org.apache.kafka.streams.processor.internals.metrics.TaskMetrics; import org.slf4j.Logger; import java.util.Collections; @@ -50,65 +57,135 @@ import java.util.Map; public class RecordCollectorImpl implements RecordCollector { + private final static String SEND_EXCEPTION_MESSAGE = "Error encountered sending record to topic %s for task %s due to:%n%s"; + private final Logger log; - private final String logPrefix; + private final TaskId taskId; + private final boolean eosEnabled; + private final String applicationId; private final Sensor droppedRecordsSensor; - private Producer producer; private final Map offsets; + private final Consumer consumer; private final ProductionExceptionHandler productionExceptionHandler; - private final static String LOG_MESSAGE = "Error sending record to topic {} due to {}; " + - "No more records will be sent and no more offsets will be recorded for this task. " + - "Enable TRACE logging to view failed record key and value."; - private final static String EXCEPTION_MESSAGE = "%sAbort sending since %s with a previous record (timestamp %d) to topic %s due to %s"; - private final static String PARAMETER_HINT = "\nYou can increase the producer configs `delivery.timeout.ms` and/or " + - "`retries` to avoid this error. Note that `retries` is set to infinite by default."; - private final static String TIMEOUT_HINT_TEMPLATE = "%nTimeout exception caught when sending record to topic %s. " + - "This might happen if the producer cannot send data to the Kafka cluster and thus, " + - "its internal buffer fills up. " + - "This can also happen if the broker is slow to respond, if the network connection to " + - "the broker was interrupted, or if similar circumstances arise. " + - "You can increase producer parameter `max.block.ms` to increase this timeout."; + // used when eosEnabled is true only + private boolean transactionInFlight = false; + private Producer producer; private volatile KafkaException sendException; - public RecordCollectorImpl(final String streamTaskId, + public RecordCollectorImpl(final TaskId taskId, + final StreamsConfig config, final LogContext logContext, - final ProductionExceptionHandler productionExceptionHandler, - final Sensor droppedRecordsSensor) { + final StreamsMetricsImpl streamsMetrics, + final Consumer consumer, + final StreamThread.ProducerSupplier producerSupplier) { + this.taskId = taskId; + this.consumer = consumer; this.offsets = new HashMap<>(); - this.logPrefix = String.format("task [%s] ", streamTaskId); this.log = logContext.logger(getClass()); - this.productionExceptionHandler = productionExceptionHandler; - this.droppedRecordsSensor = droppedRecordsSensor; + + this.applicationId = config.getString(StreamsConfig.APPLICATION_ID_CONFIG); + this.productionExceptionHandler = config.defaultProductionExceptionHandler(); + this.eosEnabled = StreamsConfig.EXACTLY_ONCE.equals(config.getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG)); + + final String threadId = Thread.currentThread().getName(); + this.droppedRecordsSensor = TaskMetrics.droppedRecordsSensorOrSkippedRecordsSensor(threadId, taskId.toString(), streamsMetrics); + + producer = producerSupplier.get(taskId); + + // TODO K9113: this should be moved to task when it transits to running from created / restoring + // then even if there's a long time between the initialization and the first txn that is also fine. + initialize(); } + /** + * @throws StreamsException fatal error that should cause the thread to die + */ @Override - public void init(final Producer producer) { - this.producer = producer; + public void initialize() { + maybeInitTxns(); } - @Override - public void send(final String topic, - final K key, - final V value, - final Headers headers, - final Long timestamp, - final Serializer keySerializer, - final Serializer valueSerializer, - final StreamPartitioner partitioner) { - Integer partition = null; + private void maybeInitTxns() { + if (eosEnabled) { + // initialize transactions if eos is turned on, which will block if the previous transaction has not + // completed yet; do not start the first transaction until the topology has been initialized later + try { + producer.initTransactions(); + } catch (final TimeoutException exception) { + final String errorMessage = "Timeout exception caught when initializing transactions for task " + taskId + ". " + + "\nThe broker is either slow or in bad state (like not having enough replicas) in responding to the request, " + + "or the connection to broker was interrupted sending the request or receiving the response. " + + "\n Consider overwriting `max.block.ms` to a larger value to avoid timeout errors"; - if (partitioner != null) { - final List partitions = producer.partitionsFor(topic); - if (partitions.size() > 0) { - partition = partitioner.partition(topic, key, value, partitions.size()); - } else { - throw new StreamsException("Could not get partition information for topic '" + topic + "'." + - " This can happen if the topic does not exist."); + // TODO K9113: we do NOT try to handle timeout exception here but throw it as a fatal error + throw new StreamsException(errorMessage, exception); + } catch (final KafkaException exception) { + throw new StreamsException("Error encountered while initializing transactions for task " + taskId, exception); + } + } + } + + private void maybeBeginTxn() { + if (eosEnabled && !transactionInFlight) { + try { + producer.beginTransaction(); + } catch (final ProducerFencedException error) { + throw new TaskMigratedException(taskId, "Producer get fenced trying to begin a new transaction", error); + } + transactionInFlight = true; + } + } + + private void maybeAbortTxn() { + if (eosEnabled && transactionInFlight) { + try { + producer.abortTransaction(); + } catch (final ProducerFencedException ignore) { + /* TODO + * this should actually never happen atm as we guard the call to #abortTransaction + * -> the reason for the guard is a "bug" in the Producer -- it throws IllegalStateException + * instead of ProducerFencedException atm. We can remove the isZombie flag after KAFKA-5604 got + * fixed and fall-back to this catch-and-swallow code + */ + + // can be ignored: transaction got already aborted by brokers/transactional-coordinator if this happens + } + transactionInFlight = false; + } + } + + public void commit(final Map offsets) { + if (eosEnabled) { + try { + maybeBeginTxn(); + + producer.sendOffsetsToTransaction(offsets, applicationId); + producer.commitTransaction(); + transactionInFlight = false; + } catch (final ProducerFencedException error) { + throw new TaskMigratedException(taskId, "Producer get fenced trying to commit a transaction", error); + } catch (final TimeoutException error) { + // TODO K9113: currently handle timeout exception as a fatal error, should discuss whether we want to handle it + throw new StreamsException("Timed out while committing transaction via producer for task " + taskId, error); + } catch (final KafkaException error) { + throw new StreamsException("Error encountered sending offsets and committing transaction " + + "via producer for task " + taskId, error); + } + } else { + try { + consumer.commitSync(offsets); + } catch (final CommitFailedException error) { + throw new TaskMigratedException(taskId, "Consumer committing offsets failed, " + + "indicating the corresponding thread is no longer part of the group.", error); + } catch (final TimeoutException error) { + // TODO K9113: currently handle timeout exception as a fatal error + throw new StreamsException("Timed out while committing offsets via consumer for task " + taskId, error); + } catch (final KafkaException error) { + throw new StreamsException("Error encountered committing offsets via consumer for task " + taskId, error); } } - send(topic, key, value, headers, partition, timestamp, keySerializer, valueSerializer); } private boolean productionExceptionIsFatal(final Exception exception) { @@ -125,41 +202,64 @@ private boolean productionExceptionIsFatal(final Exception exception) { return securityException || communicationException; } - private void recordSendError( - final K key, - final V value, - final Long timestamp, - final String topic, - final Exception exception - ) { - String errorLogMessage = LOG_MESSAGE; - String errorMessage = EXCEPTION_MESSAGE; - - if (exception instanceof TimeoutException) { - final String topicTimeoutHint = String.format(TIMEOUT_HINT_TEMPLATE, topic); - errorLogMessage += topicTimeoutHint; - errorMessage += topicTimeoutHint; - } else if (exception instanceof RetriableException) { - // There is no documented API for detecting retriable errors, so we rely on `RetriableException` - // even though it's an implementation detail (i.e. we do the best we can given what's available) - errorLogMessage += PARAMETER_HINT; - errorMessage += PARAMETER_HINT; + private void recordSendError(final String topic, final Exception exception, final ProducerRecord serializedRecord) { + String errorMessage = String.format(SEND_EXCEPTION_MESSAGE, topic, taskId, exception.toString()); + + if (productionExceptionIsFatal(exception)) { + errorMessage += "\nWritten offsets would not be recorded and no more records would be sent since this is a fatal error."; + sendException = new StreamsException(errorMessage, exception); + } else if (exception instanceof ProducerFencedException || exception instanceof OutOfOrderSequenceException) { + errorMessage += "\nWritten offsets would not be recorded and no more records would be sent since the producer is fenced, " + + "indicating the task may be migrated out."; + sendException = new TaskMigratedException(taskId, errorMessage, exception); + } else { + if (exception instanceof RetriableException) { + errorMessage += "\nThe broker is either slow or in bad state (like not having enough replicas) in responding the request, " + + "or the connection to broker was interrupted sending the request or receiving the response. " + + "\nConsider overwriting `max.block.ms` and /or " + + "`delivery.timeout.ms` to a larger value to wait longer for such scenarios and avoid timeout errors"; + } + + if (productionExceptionHandler.handle(serializedRecord, exception) == ProductionExceptionHandlerResponse.FAIL) { + errorMessage += "\nException handler choose to FAIL the processing, no more records would be sent."; + sendException = new StreamsException(errorMessage, exception); + } else { + errorMessage += "\nException handler choose to CONTINUE processing in spite of this error but written offsets would not be recorded."; + droppedRecordsSensor.record(); + } + } + + log.error(errorMessage); + } + + /** + * @throws StreamsException fatal error that should cause the thread to die + * @throws TaskMigratedException recoverable error that would cause the task to be removed + */ + @Override + public void send(final String topic, + final K key, + final V value, + final Headers headers, + final Long timestamp, + final Serializer keySerializer, + final Serializer valueSerializer, + final StreamPartitioner partitioner) { + final Integer partition; + + if (partitioner != null) { + final List partitions = producer.partitionsFor(topic); + if (partitions.size() > 0) { + partition = partitioner.partition(topic, key, value, partitions.size()); + } else { + throw new StreamsException("Could not get partition information for topic '" + topic + "' for task " + taskId + + ". This can happen if the topic does not exist."); + } + } else { + partition = null; } - log.error(errorLogMessage, topic, exception.getMessage(), exception); - - // KAFKA-7510 put message key and value in TRACE level log so we don't leak data by default - log.trace("Failed message: key {} value {} timestamp {}", key, value, timestamp); - - sendException = new StreamsException( - String.format( - errorMessage, - logPrefix, - "an error caught", - timestamp, - topic, - exception.toString() - ), - exception); + + send(topic, key, value, headers, partition, timestamp, keySerializer, valueSerializer); } @Override @@ -172,85 +272,44 @@ public void send(final String topic, final Serializer keySerializer, final Serializer valueSerializer) { checkForException(); - final byte[] keyBytes = keySerializer.serialize(topic, headers, key); - final byte[] valBytes = valueSerializer.serialize(topic, headers, value); - - final ProducerRecord serializedRecord = new ProducerRecord<>(topic, partition, timestamp, keyBytes, valBytes, headers); try { - producer.send(serializedRecord, new Callback() { - @Override - public void onCompletion(final RecordMetadata metadata, - final Exception exception) { - if (exception == null) { - if (sendException != null) { - return; - } - final TopicPartition tp = new TopicPartition(metadata.topic(), metadata.partition()); - offsets.put(tp, metadata.offset()); - } else { - if (sendException == null) { - if (exception instanceof ProducerFencedException || exception instanceof UnknownProducerIdException) { - log.warn(LOG_MESSAGE, topic, exception.getMessage(), exception); - - // KAFKA-7510 put message key and value in TRACE level log so we don't leak data by default - log.trace("Failed message: (key {} value {} timestamp {}) topic=[{}] partition=[{}]", key, value, timestamp, topic, partition); - - sendException = new RecoverableClientException( - String.format( - EXCEPTION_MESSAGE, - logPrefix, - "producer got fenced", - timestamp, - topic, - exception.toString() - ), - exception - ); - } else { - if (productionExceptionIsFatal(exception)) { - recordSendError(key, value, timestamp, topic, exception); - } else if (productionExceptionHandler.handle(serializedRecord, exception) == ProductionExceptionHandlerResponse.FAIL) { - recordSendError(key, value, timestamp, topic, exception); - } else { - log.warn( - "Error sending records topic=[{}] and partition=[{}]; " + - "The exception handler chose to CONTINUE processing in spite of this error. " + - "Enable TRACE logging to view failed messages key and value.", - topic, partition, exception - ); - - // KAFKA-7510 put message key and value in TRACE level log so we don't leak data by default - log.trace("Failed message: (key {} value {} timestamp {}) topic=[{}] partition=[{}]", key, value, timestamp, topic, partition); - - droppedRecordsSensor.record(); - } - } - } - } + maybeBeginTxn(); + + final byte[] keyBytes = keySerializer.serialize(topic, headers, key); + final byte[] valBytes = valueSerializer.serialize(topic, headers, value); + + final ProducerRecord serializedRecord = new ProducerRecord<>(topic, partition, timestamp, keyBytes, valBytes, headers); + + producer.send(serializedRecord, (metadata, exception) -> { + // if there's already an exception record, skip logging offsets or new exceptions + if (sendException != null) { + return; + } + + if (exception == null) { + final TopicPartition tp = new TopicPartition(metadata.topic(), metadata.partition()); + offsets.put(tp, metadata.offset()); + } else { + recordSendError(topic, exception, serializedRecord); + + // KAFKA-7510 only put message key and value in TRACE level log so we don't leak data by default + log.trace("Failed record: (key {} value {} timestamp {}) topic=[{}] partition=[{}]", key, value, timestamp, topic, partition); } }); } catch (final RuntimeException uncaughtException) { if (isRecoverable(uncaughtException)) { // producer.send() call may throw a KafkaException which wraps a FencedException, // in this case we should throw its wrapped inner cause so that it can be captured and re-wrapped as TaskMigrationException - throw new RecoverableClientException("Caught a recoverable exception", uncaughtException); + throw new TaskMigratedException(taskId, "Producer cannot send records anymore since it got fenced", uncaughtException); } else { - throw new StreamsException( - String.format( - EXCEPTION_MESSAGE, - logPrefix, - "an error caught", - timestamp, - topic, - uncaughtException.toString() - ), - uncaughtException); + final String errorMessage = String.format(SEND_EXCEPTION_MESSAGE, topic, taskId, uncaughtException.toString()); + throw new StreamsException(errorMessage, uncaughtException); } } } - public static boolean isRecoverable(final RuntimeException uncaughtException) { + private static boolean isRecoverable(final RuntimeException uncaughtException) { return uncaughtException instanceof KafkaException && ( uncaughtException.getCause() instanceof ProducerFencedException || uncaughtException.getCause() instanceof UnknownProducerIdException); @@ -262,28 +321,34 @@ private void checkForException() { } } + /** + * @throws StreamsException fatal error that should cause the thread to die + * @throws TaskMigratedException recoverable error that would cause the task to be removed + */ @Override public void flush() { - log.debug("Flushing producer"); - try { - producer.flush(); - } catch (final ProducerFencedException | UnknownProducerIdException e) { - throw new RecoverableClientException("Caught a recoverable exception while flushing", e); - } + log.debug("Flushing record collector"); + producer.flush(); checkForException(); } + /** + * @throws StreamsException fatal error that should cause the thread to die + * @throws TaskMigratedException recoverable error that would cause the task to be removed + */ @Override public void close() { - log.debug("Closing producer"); - if (producer != null) { + log.debug("Closing record collector"); + maybeAbortTxn(); + + if (eosEnabled) { try { producer.close(); - } catch (final ProducerFencedException | UnknownProducerIdException e) { - throw new RecoverableClientException("Caught a recoverable exception while closing", e); + } catch (final KafkaException e) { + throw new StreamsException("Caught a recoverable exception while closing", e); } - producer = null; } + checkForException(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StandbyTask.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StandbyTask.java index 72ac76a97658b..026316a285033 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StandbyTask.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StandbyTask.java @@ -62,11 +62,11 @@ public class StandbyTask extends AbstractTask { final Set partitions, final ProcessorTopology topology, final Consumer consumer, - final ChangelogReader changelogReader, final StreamsConfig config, final StreamsMetricsImpl metrics, + final ProcessorStateManager stateMgr, final StateDirectory stateDirectory) { - super(id, partitions, topology, consumer, changelogReader, true, stateDirectory, config); + super(id, partitions, topology, consumer, true, stateMgr, stateDirectory, config); closeTaskSensor = ThreadMetrics.closeTaskSensor(Thread.currentThread().getName(), metrics); processorContext = new StandbyContextImpl(id, config, stateMgr, metrics); @@ -132,11 +132,9 @@ private void flushAndCheckpointState() { * - {@link #commit()} * - close state *

-     * @param isZombie ignored by {@code StandbyTask} as it can never be a zombie
      */
     @Override
-    public void close(final boolean clean,
-                      final boolean isZombie) {
+    public void close(final boolean clean) {
         closeTaskSensor.record();
         if (!taskInitialized) {
             return;
diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StateManagerUtil.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StateManagerUtil.java
index 4568865b38d86..ef95c0e59ff1e 100644
--- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StateManagerUtil.java
+++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StateManagerUtil.java
@@ -48,7 +48,6 @@ static RecordConverter converterForStore(final StateStore store) {
     }
 
     public static void reinitializeStateStoresForPartitions(final Logger log,
-                                                            final boolean eosEnabled,
                                                             final File baseDir,
                                                             final FixedOrderMap> stateStores,
                                                             final Map storeToChangelogTopic,
@@ -64,18 +63,6 @@ public static void reinitializeStateStoresForPartitions(final Logger log,
             storesToBeReinitialized.add(changelogTopicToStore.get(topicPartition.topic()));
         }
 
-        if (!eosEnabled) {
-            try {
-                checkpointFile.write(checkpointFileCache);
-            } catch (final IOException fatalException) {
-                log.error("Failed to write offset checkpoint file to {} while re-initializing {}: {}",
-                          checkpointFile,
-                          stateStores,
-                          fatalException);
-                throw new StreamsException("Failed to reinitialize global store.", fatalException);
-            }
-        }
-
         for (final String storeName : storesToBeReinitialized) {
             if (!stateStores.containsKey(storeName)) {
                 // the store has never been registered; carry on...
diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java
index 4117355c145ee..898de11237b17 100644
--- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java
+++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java
@@ -16,24 +16,18 @@
  */
 package org.apache.kafka.streams.processor.internals;
 
-import org.apache.kafka.clients.consumer.CommitFailedException;
 import org.apache.kafka.clients.consumer.Consumer;
 import org.apache.kafka.clients.consumer.ConsumerRecord;
 import org.apache.kafka.clients.consumer.OffsetAndMetadata;
-import org.apache.kafka.clients.producer.Producer;
 import org.apache.kafka.common.KafkaException;
 import org.apache.kafka.common.TopicPartition;
 import org.apache.kafka.common.errors.AuthorizationException;
-import org.apache.kafka.common.errors.ProducerFencedException;
-import org.apache.kafka.common.errors.TimeoutException;
-import org.apache.kafka.common.errors.UnknownProducerIdException;
 import org.apache.kafka.common.errors.WakeupException;
 import org.apache.kafka.common.metrics.Sensor;
 import org.apache.kafka.common.utils.Time;
 import org.apache.kafka.streams.StreamsConfig;
 import org.apache.kafka.streams.errors.DeserializationExceptionHandler;
 import org.apache.kafka.streams.errors.ProcessorStateException;
-import org.apache.kafka.streams.errors.ProductionExceptionHandler;
 import org.apache.kafka.streams.errors.StreamsException;
 import org.apache.kafka.streams.errors.TaskMigratedException;
 import org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl.Version;
@@ -72,6 +66,12 @@ public class StreamTask extends AbstractTask implements ProcessorNodePunctuator
     static final byte LATEST_MAGIC_BYTE = 1;
 
     private final Time time;
+    private final String threadId;
+    // we want to abstract eos logic out of StreamTask, however
+    // there's still an optimization that requires this info to be
+    // leaked into this class, which is to checkpoint after committing if EOS is not enabled.
+    private final boolean eosEnabled;
+    private final StreamsConfig config;
     private final long maxTaskIdleMs;
     private final int maxBufferedSize;
     private final StreamsMetricsImpl streamsMetrics;
@@ -81,7 +81,6 @@ public class StreamTask extends AbstractTask implements ProcessorNodePunctuator
     private final Map consumedOffsets;
     private final PunctuationQueue streamTimePunctuationQueue;
     private final PunctuationQueue systemTimePunctuationQueue;
-    private final ProducerSupplier producerSupplier;
 
     private final Sensor closeTaskSensor;
     private final Sensor processLatencySensor;
@@ -91,78 +90,41 @@ public class StreamTask extends AbstractTask implements ProcessorNodePunctuator
     private final Sensor recordLatenessSensor;
 
     private long idleStartTime;
-    private Producer producer;
     private boolean commitRequested = false;
-    private boolean transactionInFlight = false;
-
-    private final String threadId;
-
-    public interface ProducerSupplier {
-        Producer get();
-    }
 
     public StreamTask(final TaskId id,
                       final Set partitions,
                       final ProcessorTopology topology,
                       final Consumer consumer,
-                      final ChangelogReader changelogReader,
-                      final StreamsConfig config,
-                      final StreamsMetricsImpl metrics,
-                      final StateDirectory stateDirectory,
-                      final ThreadCache cache,
-                      final Time time,
-                      final ProducerSupplier producerSupplier) {
-        this(id, partitions, topology, consumer, changelogReader, config, metrics, stateDirectory, cache, time, producerSupplier, null);
-    }
-
-    public StreamTask(final TaskId id,
-                      final Set partitions,
-                      final ProcessorTopology topology,
-                      final Consumer consumer,
-                      final ChangelogReader changelogReader,
                       final StreamsConfig config,
                       final StreamsMetricsImpl streamsMetrics,
                       final StateDirectory stateDirectory,
                       final ThreadCache cache,
                       final Time time,
-                      final ProducerSupplier producerSupplier,
+                      final ProcessorStateManager stateMgr,
                       final RecordCollector recordCollector) {
-        super(id, partitions, topology, consumer, changelogReader, false, stateDirectory, config);
+        super(id, partitions, topology, consumer, false, stateMgr, stateDirectory, config);
 
         this.time = time;
-        this.producerSupplier = producerSupplier;
-        this.producer = producerSupplier.get();
+        this.config = config;
         this.streamsMetrics = streamsMetrics;
+        this.recordCollector = recordCollector;
+        this.eosEnabled = StreamsConfig.EXACTLY_ONCE.equals(config.getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG));
 
-        threadId = Thread.currentThread().getName();
+        this.threadId = Thread.currentThread().getName();
+        this.closeTaskSensor = ThreadMetrics.closeTaskSensor(threadId, streamsMetrics);
         final String taskId = id.toString();
-        closeTaskSensor = ThreadMetrics.closeTaskSensor(threadId, streamsMetrics);
         if (streamsMetrics.version() == Version.FROM_0100_TO_24) {
             final Sensor parent = ThreadMetrics.commitOverTasksSensor(threadId, streamsMetrics);
-            commitSensor = TaskMetrics.commitSensor(threadId, taskId, streamsMetrics, parent);
-            enforcedProcessingSensor = TaskMetrics.enforcedProcessingSensor(threadId, taskId, streamsMetrics, parent);
-        } else {
-            commitSensor = TaskMetrics.commitSensor(threadId, taskId, streamsMetrics);
-            enforcedProcessingSensor = TaskMetrics.enforcedProcessingSensor(threadId, taskId, streamsMetrics);
-        }
-        processLatencySensor = TaskMetrics.processLatencySensor(threadId, taskId, streamsMetrics);
-        punctuateLatencySensor = TaskMetrics.punctuateSensor(threadId, taskId, streamsMetrics);
-        recordLatenessSensor = TaskMetrics.recordLatenessSensor(threadId, taskId, streamsMetrics);
-        TaskMetrics.droppedRecordsSensor(threadId, taskId, streamsMetrics);
-
-        final ProductionExceptionHandler productionExceptionHandler = config.defaultProductionExceptionHandler();
-
-        if (recordCollector == null) {
-            this.recordCollector = new RecordCollectorImpl(
-                taskId,
-                logContext,
-                productionExceptionHandler,
-                TaskMetrics.droppedRecordsSensorOrSkippedRecordsSensor(threadId, taskId, streamsMetrics)
-            );
+            this.commitSensor = TaskMetrics.commitSensor(threadId, taskId, streamsMetrics, parent);
+            this.enforcedProcessingSensor = TaskMetrics.enforcedProcessingSensor(threadId, taskId, streamsMetrics, parent);
         } else {
-            this.recordCollector = recordCollector;
+            this.commitSensor = TaskMetrics.commitSensor(threadId, taskId, streamsMetrics);
+            this.enforcedProcessingSensor = TaskMetrics.enforcedProcessingSensor(threadId, taskId, streamsMetrics);
         }
-        this.recordCollector.init(this.producer);
+        this.processLatencySensor = TaskMetrics.processLatencySensor(threadId, taskId, streamsMetrics);
+        this.punctuateLatencySensor = TaskMetrics.punctuateSensor(threadId, taskId, streamsMetrics);
+        this.recordLatenessSensor = TaskMetrics.recordLatenessSensor(threadId, taskId, streamsMetrics);
 
         streamTimePunctuationQueue = new PunctuationQueue();
         systemTimePunctuationQueue = new PunctuationQueue();
@@ -199,12 +161,10 @@ public StreamTask(final TaskId id,
         partitionGroup = new PartitionGroup(partitionQueues, recordLatenessSensor);
 
         stateMgr.registerGlobalStateStores(topology.globalStateStores());
+    }
 
-        // initialize transactions if eos is turned on, which will block if the previous transaction has not
-        // completed yet; do not start the first transaction until the topology has been initialized later
-        if (eosEnabled) {
-            initializeTransactions();
-        }
+    public boolean isEosEnabled() {
+        return eosEnabled;
     }
 
     @Override
@@ -287,15 +247,6 @@ public boolean initializeStateStores() {
     public void initializeTopology() {
         initTopology();
 
-        if (eosEnabled) {
-            try {
-                this.producer.beginTransaction();
-            } catch (final ProducerFencedException | UnknownProducerIdException e) {
-                throw new TaskMigratedException(this, e);
-            }
-            transactionInFlight = true;
-        }
-
         processorContext.initialize();
 
         taskInitialized = true;
@@ -313,20 +264,7 @@ public void initializeTopology() {
     @Override
     public void resume() {
         log.debug("Resuming");
-        if (eosEnabled) {
-            if (producer != null) {
-                throw new IllegalStateException("Task producer should be null.");
-            }
-            producer = producerSupplier.get();
-            initializeTransactions();
-            recordCollector.init(producer);
-
-            try {
-                stateMgr.clearCheckpoints();
-            } catch (final IOException e) {
-                throw new ProcessorStateException(format("%sError while deleting the checkpoint file", logPrefix), e);
-            }
-        }
+        stateMgr.clearCheckpoints();
         initializeMetadata();
     }
 
@@ -395,7 +333,7 @@ public boolean process() {
                 consumer.resume(singleton(partition));
             }
         } catch (final RecoverableClientException e) {
-            throw new TaskMigratedException(this, e);
+            throw new TaskMigratedException(id, e);
         } catch (final KafkaException e) {
             final String stackTrace = getStacktraceString(e);
             throw new StreamsException(format("Exception caught in process. taskId=%s, " +
@@ -445,7 +383,7 @@ public void punctuate(final ProcessorNode node, final long timestamp, final Punc
         try {
             maybeMeasureLatency(() -> node.punctuate(timestamp, punctuator), time, punctuateLatencySensor);
         } catch (final RecoverableClientException e) {
-            throw new TaskMigratedException(this, e);
+            throw new TaskMigratedException(id, e);
         } catch (final KafkaException e) {
             throw new StreamsException(String.format("%sException caught while punctuating processor '%s'", logPrefix, node.name()), e);
         } finally {
@@ -464,36 +402,51 @@ private void updateProcessorContext(final StampedRecord record, final ProcessorN
         processorContext.setCurrentNode(currNode);
     }
 
+    private Map extractPartitionTimes() {
+        final Map partitionTimes = new HashMap<>();
+        for (final TopicPartition partition : partitionGroup.partitions()) {
+            partitionTimes.put(partition, partitionTime(partition));
+        }
+        return partitionTimes;
+    }
+
     /**
-     * 
-     * - flush state and producer
-     * - if(!eos) write checkpoint
-     * - commit offsets and start new transaction
-     * 
- * * @throws TaskMigratedException if committing offsets failed (non-EOS) * or if the task producer got fenced (EOS) */ @Override public void commit() { - commit(true, extractPartitionTimes()); + log.debug("Committing"); + + commitState(); + + // this is an optimization for non-EOS only + if (!eosEnabled) { + stateMgr.checkpoint(checkpointableOffsets()); + } } /** + *
+     * the following order must be followed:
+     *  1. flush the state, send any left changelog records
+     *  2. then flush the record collector
+     *  3. then commit the record collector -- for EOS this is the synchronization barrier
+     * 
+ * * @throws TaskMigratedException if committing offsets failed (non-EOS) * or if the task producer got fenced (EOS) */ - // visible for testing - void commit(final boolean startNewTransaction, final Map partitionTimes) { + private void commitState() { final long startNs = time.nanoseconds(); - log.debug("Committing"); - flushState(); + stateMgr.flush(); - if (!eosEnabled) { - stateMgr.checkpoint(activeTaskCheckpointableOffsets()); - } + recordCollector.flush(); + // we need to preserve the original partitions times before calling commit + // because all partition times are reset to -1 during close + final Map partitionTimes = extractPartitionTimes(); final Map consumedOffsetsAndMetadata = new HashMap<>(consumedOffsets.size()); for (final Map.Entry entry : consumedOffsets.entrySet()) { final TopicPartition partition = entry.getKey(); @@ -501,29 +454,18 @@ void commit(final boolean startNewTransaction, final Map p final long partitionTime = partitionTimes.get(partition); consumedOffsetsAndMetadata.put(partition, new OffsetAndMetadata(offset, encodeTimestamp(partitionTime))); } - - try { - if (eosEnabled) { - producer.sendOffsetsToTransaction(consumedOffsetsAndMetadata, applicationId); - producer.commitTransaction(); - transactionInFlight = false; - if (startNewTransaction) { - producer.beginTransaction(); - transactionInFlight = true; - } - } else { - consumer.commitSync(consumedOffsetsAndMetadata); - } - } catch (final CommitFailedException | ProducerFencedException | UnknownProducerIdException error) { - throw new TaskMigratedException(this, error); - } + recordCollector.commit(consumedOffsetsAndMetadata); commitNeeded = false; commitRequested = false; commitSensor.record(time.nanoseconds() - startNs); } - private Map activeTaskCheckpointableOffsets() { + /** + * Return all the checkpointable offsets(written + consumed) to the state manager. + * Currently only changelog topic offsets need to be checkpointed. + */ + private Map checkpointableOffsets() { final Map checkpointableOffsets = new HashMap<>(recordCollector.offsets()); for (final Map.Entry entry : consumedOffsets.entrySet()) { checkpointableOffsets.putIfAbsent(entry.getKey(), entry.getValue()); @@ -531,17 +473,6 @@ private Map activeTaskCheckpointableOffsets() { return checkpointableOffsets; } - @Override - protected void flushState() { - log.trace("Flushing state and producer"); - super.flushState(); - try { - recordCollector.flush(); - } catch (final RecoverableClientException e) { - throw new TaskMigratedException(this, e); - } - } - Map purgableOffsets() { final Map purgableConsumedOffsets = new HashMap<>(); for (final Map.Entry entry : consumedOffsets.entrySet()) { @@ -581,98 +512,49 @@ private void initTopology() { */ public void suspend() { log.debug("Suspending"); - suspend(true, false); + suspend(true); } /** *
-     * - close topology
-     * - if (clean) {@link #commit()}
-     *   - flush state and producer
-     *   - if (!eos) write checkpoint
-     *   - commit offsets
+     * the following order must be followed:
+     *  1. first close topology to make sure all cached records in the topology are processed
+     *  2. then flush the state, send any left changelog records
+     *  3. then flush the record collector
+     *  4. then commit the record collector -- for EOS this is the synchronization barrier
+     *  5. then checkpoint the state manager -- even if we crash before this step, EOS is still guaranteed
      * 
* * @throws TaskMigratedException if committing offsets failed (non-EOS) * or if the task producer got fenced (EOS) */ - // visible for testing - void suspend(final boolean clean, - final boolean isZombie) { - // this is necessary because all partition times are reset to -1 during close - // we need to preserve the original partitions times before calling commit - final Map partitionTimes = extractPartitionTimes(); + private void suspend(final boolean clean) { try { - closeTopology(); // should we call this only on clean suspend? - } catch (final RuntimeException fatal) { - if (clean) { - throw fatal; - } - } + // If the suspension is from unclean shutdown, then only need to close topology and flush state to make sure that when later + // closing the states, there's no records triggering any processing anymore; also swallow all caught exceptions + closeTopology(); - if (clean) { - TaskMigratedException taskMigratedException = null; - try { - commit(false, partitionTimes); - } finally { - if (eosEnabled) { - stateMgr.checkpoint(activeTaskCheckpointableOffsets()); - - try { - recordCollector.close(); - } catch (final RecoverableClientException e) { - taskMigratedException = new TaskMigratedException(this, e); - } finally { - producer = null; - } - } - } - if (taskMigratedException != null) { - throw taskMigratedException; - } - } else { - // In the case of unclean close we still need to make sure all the stores are flushed before closing any - super.flushState(); - - if (eosEnabled) { - maybeAbortTransactionAndCloseRecordCollector(isZombie); + if (clean) { + commitState(); + // whenever we have successfully committed state during suspension, it is safe to checkpoint + // the state as well no matter if EOS is enabled or not + stateMgr.checkpoint(checkpointableOffsets()); + } else { + stateMgr.flush(); } - } - } - - private void maybeAbortTransactionAndCloseRecordCollector(final boolean isZombie) { - if (!isZombie) { - try { - if (transactionInFlight) { - producer.abortTransaction(); - } - transactionInFlight = false; - } catch (final ProducerFencedException ignore) { - /* TODO - * this should actually never happen atm as we guard the call to #abortTransaction - * -> the reason for the guard is a "bug" in the Producer -- it throws IllegalStateException - * instead of ProducerFencedException atm. We can remove the isZombie flag after KAFKA-5604 got - * fixed and fall-back to this catch-and-swallow code - */ - - // can be ignored: transaction got already aborted by brokers/transactional-coordinator if this happens + } catch (final RuntimeException error) { + if (clean) { + throw error; } } - try { - recordCollector.close(); - } catch (final Throwable e) { - log.error("Failed to close producer due to the following error:", e); - } finally { - producer = null; - } + // we should also clear any buffered records of a task when suspending it + partitionGroup.clear(); } private void closeTopology() { log.trace("Closing processor topology"); - partitionGroup.clear(); - // close the processors // make sure close() is called for each node even when there is a RuntimeException RuntimeException exception = null; @@ -694,61 +576,57 @@ private void closeTopology() { } } - // helper to avoid calling suspend() twice if a suspended task is not reassigned and closed - void closeSuspended(final boolean clean, RuntimeException firstException) { + // TODO K9113: we should let the task itself to decide, based on the state, whether to close as suspended or as running + void closeSuspended(final boolean clean) { + try { closeStateManager(clean); - } catch (final RuntimeException e) { - if (firstException == null) { - firstException = e; + } catch (final RuntimeException error) { + if (clean) { + throw error; } - log.error("Could not close state manager due to the following error:", e); - } - - partitionGroup.close(); - streamsMetrics.removeAllTaskLevelSensors(threadId, id.toString()); - - closeTaskSensor.record(); + } finally { + partitionGroup.close(); + recordCollector.close(); + closeTaskSensor.record(); - if (firstException != null) { - throw firstException; + streamsMetrics.removeAllTaskLevelSensors(threadId, id.toString()); } } /** *
-     * - {@link #suspend(boolean, boolean) suspend(clean)}
-     *   - close topology
-     *   - if (clean) {@link #commit()}
-     *     - flush state and producer
-     *     - commit offsets
-     * - close state
-     *   - if (clean) write checkpoint
-     * - if (eos) close producer
+     * the following order must be followed:
+     *  1. first close topology to make sure all cached records in the topology are processed
+     *  2. then flush the state, send any left changelog records
+     *  3. then flush the record collector
+     *  4. then commit the record collector -- for EOS this is the synchronization barrier
+     *  5. then checkpoint the state manager -- even if we crash before this step, EOS is still guaranteed
      * 
* * @param clean shut down cleanly (ie, incl. flush and commit) if {@code true} -- * otherwise, just close open resources - * @param isZombie {@code true} is this task is a zombie or not (this will repress {@link TaskMigratedException} * @throws TaskMigratedException if committing offsets failed (non-EOS) * or if the task producer got fenced (EOS) */ @Override - public void close(boolean clean, - final boolean isZombie) { + public void close(final boolean clean) { log.debug("Closing"); - RuntimeException firstException = null; try { - suspend(clean, isZombie); - } catch (final RuntimeException e) { - clean = false; - firstException = e; - log.error("Could not close task due to the following error:", e); + // If it is an unclean close, then the suspend would never + // throw since it would swallow all exception; otherwise if suspension + // throws we do not need to proceed to further steps and should just notify + // the caller thread. Therefore it is always safe to proceed without try-catch + suspend(clean); + + closeSuspended(clean); + } catch (final RuntimeException error) { + if (clean) { + throw error; + } } - closeSuspended(clean, firstException); - taskClosed = true; } @@ -886,44 +764,11 @@ boolean commitRequested() { return commitRequested; } - // visible for testing only - RecordCollector recordCollector() { - return recordCollector; - } - - // used for testing - long streamTime() { - return partitionGroup.streamTime(); - } - - // used for testing + // visible for testing long partitionTime(final TopicPartition partition) { return partitionGroup.partitionTimestamp(partition); } - Producer getProducer() { - return producer; - } - - private void initializeTransactions() { - try { - producer.initTransactions(); - } catch (final TimeoutException retriable) { - log.error( - "Timeout exception caught when initializing transactions for task {}. " + - "This might happen if the broker is slow to respond, if the network connection to " + - "the broker was interrupted, or if similar circumstances arise. " + - "You can increase producer parameter `max.block.ms` to increase this timeout.", - id, - retriable - ); - throw new StreamsException( - format("%sFailed to initialize task %s due to timeout.", logPrefix, id), - retriable - ); - } - } - // visible for testing String encodeTimestamp(final long partitionTime) { final ByteBuffer buffer = ByteBuffer.allocate(9); @@ -949,11 +794,8 @@ long decodeTimestamp(final String encryptedString) { } } - private Map extractPartitionTimes() { - final Map partitionTimes = new HashMap<>(); - for (final TopicPartition partition : partitionGroup.partitions()) { - partitionTimes.put(partition, partitionTime(partition)); - } - return partitionTimes; + // ---- visible for testing only below --- // + RecordCollector recordCollector() { + return recordCollector; } } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java index 5abdef1f451ed..cd0f4c8bbe265 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java @@ -254,6 +254,11 @@ void setRebalanceException(final Throwable rebalanceException) { this.rebalanceException = rebalanceException; } + // public for testing purposes + public interface ProducerSupplier { + Producer get(final TaskId id); + } + static abstract class AbstractTaskCreator { final String applicationId; final InternalTopologyBuilder builder; @@ -307,15 +312,14 @@ Collection createTasks(final Consumer consumer, } abstract T createTask(final Consumer consumer, final TaskId id, final Set partitions); - - public void close() {} } static class TaskCreator extends AbstractTaskCreator { - private final ThreadCache cache; - private final KafkaClientSupplier clientSupplier; private final String threadId; + private final ThreadCache cache; private final Producer threadProducer; + private final KafkaClientSupplier clientSupplier; + private final ProducerSupplier producerSupplier; private final Sensor createTaskSensor; TaskCreator(final InternalTopologyBuilder builder, @@ -326,7 +330,6 @@ static class TaskCreator extends AbstractTaskCreator { final ThreadCache cache, final Time time, final KafkaClientSupplier clientSupplier, - final Producer threadProducer, final String threadId, final Logger log) { super( @@ -337,11 +340,22 @@ static class TaskCreator extends AbstractTaskCreator { storeChangelogReader, time, log); + + final boolean eosEnabled = StreamsConfig.EXACTLY_ONCE.equals(config.getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG)); + if (!eosEnabled) { + final Map producerConfigs = config.getProducerConfigs(getThreadProducerClientId(threadId)); + log.info("Creating thread producer client"); + threadProducer = clientSupplier.getProducer(producerConfigs); + } else { + threadProducer = null; + } + + this.cache = cache; - this.clientSupplier = clientSupplier; - this.threadProducer = threadProducer; this.threadId = threadId; - createTaskSensor = ThreadMetrics.createTaskSensor(threadId, streamsMetrics); + this.clientSupplier = clientSupplier; + this.producerSupplier = new TaskProducerSupplier(); + this.createTaskSensor = ThreadMetrics.createTaskSensor(threadId, streamsMetrics); } @Override @@ -350,33 +364,60 @@ StreamTask createTask(final Consumer consumer, final Set partitions) { createTaskSensor.record(); + final String threadIdPrefix = String.format("stream-thread [%s] ", Thread.currentThread().getName()); + final String logPrefix = threadIdPrefix + String.format("%s [%s] ", "task", taskId); + final LogContext logContext = new LogContext(logPrefix); + + final ProcessorTopology topology = builder.build(taskId.topicGroupId); + + final ProcessorStateManager stateManager = new ProcessorStateManager( + taskId, + partitions, + false, + stateDirectory, + topology.storeToChangelogTopic(), + storeChangelogReader, + logContext); + + final RecordCollector recordCollector = new RecordCollectorImpl( + taskId, + config, + logContext, + streamsMetrics, + consumer, + producerSupplier); + return new StreamTask( taskId, partitions, - builder.build(taskId.topicGroupId), + topology, consumer, - storeChangelogReader, config, streamsMetrics, stateDirectory, cache, time, - () -> createProducer(taskId)); + stateManager, + recordCollector); } - private Producer createProducer(final TaskId id) { - // eos - if (threadProducer == null) { - final Map producerConfigs = config.getProducerConfigs(getTaskProducerClientId(threadId, id)); - log.info("Creating producer client for task {}", id); - producerConfigs.put(ProducerConfig.TRANSACTIONAL_ID_CONFIG, applicationId + "-" + id); - return clientSupplier.getProducer(producerConfigs); + private class TaskProducerSupplier implements ProducerSupplier { + @Override + public Producer get(final TaskId id) { + if (threadProducer == null) { + // create one producer per task for EOS + // TODO: after KIP-447 this would be removed + final Map producerConfigs = config.getProducerConfigs(getTaskProducerClientId(threadId, id)); + producerConfigs.put(ProducerConfig.TRANSACTIONAL_ID_CONFIG, applicationId + "-" + id); + log.info("Creating producer client for task {}", id); + + return clientSupplier.getProducer(producerConfigs); + } else { + return threadProducer; + } } - - return threadProducer; } - @Override public void close() { if (threadProducer != null) { try { @@ -416,17 +457,30 @@ StandbyTask createTask(final Consumer consumer, final Set partitions) { createTaskSensor.record(); + final String threadIdPrefix = String.format("stream-thread [%s] ", Thread.currentThread().getName()); + final String logPrefix = threadIdPrefix + String.format("%s [%s] ", "standby-task", taskId); + final LogContext logContext = new LogContext(logPrefix); + final ProcessorTopology topology = builder.build(taskId.topicGroupId); if (!topology.stateStores().isEmpty() && !topology.storeToChangelogTopic().isEmpty()) { + final ProcessorStateManager stateManager = new ProcessorStateManager( + taskId, + partitions, + true, + stateDirectory, + topology.storeToChangelogTopic(), + storeChangelogReader, + logContext); + return new StandbyTask( taskId, partitions, topology, consumer, - storeChangelogReader, config, streamsMetrics, + stateManager, stateDirectory); } else { log.trace( @@ -469,9 +523,9 @@ StandbyTask createTask(final Consumer consumer, // package-private for testing final ConsumerRebalanceListener rebalanceListener; - final Producer producer; final Consumer restoreConsumer; final Consumer consumer; + final Producer producer; final InternalTopologyBuilder builder; public static StreamThread create(final InternalTopologyBuilder builder, @@ -499,17 +553,9 @@ public static StreamThread create(final InternalTopologyBuilder builder, final Duration pollTime = Duration.ofMillis(config.getLong(StreamsConfig.POLL_MS_CONFIG)); final StoreChangelogReader changelogReader = new StoreChangelogReader(restoreConsumer, pollTime, userStateRestoreListener, logContext); - Producer threadProducer = null; - final boolean eosEnabled = StreamsConfig.EXACTLY_ONCE.equals(config.getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG)); - if (!eosEnabled) { - final Map producerConfigs = config.getProducerConfigs(getThreadProducerClientId(threadId)); - log.info("Creating shared producer client"); - threadProducer = clientSupplier.getProducer(producerConfigs); - } - final ThreadCache cache = new ThreadCache(logContext, cacheSizeBytes, streamsMetrics); - final AbstractTaskCreator activeTaskCreator = new TaskCreator( + final TaskCreator activeTaskCreator = new TaskCreator( builder, config, streamsMetrics, @@ -518,10 +564,9 @@ public static StreamThread create(final InternalTopologyBuilder builder, cache, time, clientSupplier, - threadProducer, threadId, log); - final AbstractTaskCreator standbyTaskCreator = new StandbyTaskCreator( + final StandbyTaskCreator standbyTaskCreator = new StandbyTaskCreator( builder, config, streamsMetrics, @@ -560,7 +605,7 @@ public static StreamThread create(final InternalTopologyBuilder builder, return new StreamThread( time, config, - threadProducer, + activeTaskCreator.threadProducer, restoreConsumer, consumer, originalReset, @@ -614,9 +659,9 @@ public StreamThread(final Time time, this.log = logContext.logger(StreamThread.class); this.rebalanceListener = new StreamsRebalanceListener(time, taskManager, this, this.log); this.taskManager = taskManager; - this.producer = producer; this.restoreConsumer = restoreConsumer; this.consumer = consumer; + this.producer = producer; this.originalReset = originalReset; this.assignmentErrorCode = assignmentErrorCode; @@ -707,8 +752,7 @@ private void runLoop() { } catch (final TaskMigratedException ignoreAndRejoinGroup) { log.warn("Detected task {} that got migrated to another thread. " + "This implies that this thread missed a rebalance and dropped out of the consumer group. " + - "Will try to rejoin the consumer group. Below is the detailed description of the task:\n{}", - ignoreAndRejoinGroup.migratedTask().id(), ignoreAndRejoinGroup.migratedTask().toString(">")); + "Will try to rejoin the consumer group.", ignoreAndRejoinGroup.migratedTaskId()); enforceRebalance(); } @@ -922,7 +966,7 @@ private void addRecordsToTasks(final ConsumerRecords records) { } else if (task.isClosed()) { log.info("Stream task {} is already closed, probably because it got unexpectedly migrated to another thread already. " + "Notifying the thread to trigger a new rebalance immediately.", task.id()); - throw new TaskMigratedException(task); + throw new TaskMigratedException(task.id()); } task.addRecords(partition, records.records(partition)); @@ -1006,7 +1050,7 @@ private void maybeUpdateStandbyTasks() { if (task.isClosed()) { log.info("Standby task {} is already closed, probably because it got unexpectedly migrated to another thread already. " + "Notifying the thread to trigger a new rebalance immediately.", task.id()); - throw new TaskMigratedException(task); + throw new TaskMigratedException(task.id()); } remaining = task.update(partition, remaining); @@ -1045,7 +1089,7 @@ private void maybeUpdateStandbyTasks() { if (task.isClosed()) { log.info("Standby task {} is already closed, probably because it got unexpectedly migrated to another thread already. " + "Notifying the thread to trigger a new rebalance immediately.", task.id()); - throw new TaskMigratedException(task); + throw new TaskMigratedException(task.id()); } final List> remaining = task.update(partition, records.records(partition)); @@ -1064,7 +1108,7 @@ private void maybeUpdateStandbyTasks() { if (task.isClosed()) { log.info("Standby task {} is already closed, probably because it got unexpectedly migrated to another thread already. " + "Notifying the thread to trigger a new rebalance immediately.", task.id()); - throw new TaskMigratedException(task); + throw new TaskMigratedException(task.id()); } log.info("Reinitializing StandbyTask {} from changelogs {}", task, recoverableException.partitions()); @@ -1228,7 +1272,7 @@ public Map producerMetrics() { // and the producer object passed in here will be null. We would then iterate through // all the active tasks and add their metrics to the output metrics map. for (final StreamTask task: taskManager.activeTasks().values()) { - final Map taskProducerMetrics = task.getProducer().metrics(); + final Map taskProducerMetrics = ((RecordCollectorImpl) task.recordCollector()).producer().metrics(); result.putAll(taskProducerMetrics); } } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/Task.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/Task.java index 13e21b90f6fdb..60b18965429a7 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/Task.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/Task.java @@ -45,8 +45,7 @@ public interface Task { void resume(); - void close(final boolean clean, - final boolean isZombie); + void close(final boolean clean); StateStore getStore(final String name); diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java index 43284c00a286c..6916f3f19e62b 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java @@ -54,7 +54,7 @@ public class TaskManager { private final ChangelogReader changelogReader; private final String logPrefix; private final Consumer restoreConsumer; - private final StreamThread.AbstractTaskCreator taskCreator; + private final StreamThread.TaskCreator taskCreator; private final StreamThread.AbstractTaskCreator standbyTaskCreator; private final StreamsMetadataState streamsMetadataState; @@ -82,8 +82,8 @@ public class TaskManager { final String logPrefix, final Consumer restoreConsumer, final StreamsMetadataState streamsMetadataState, - final StreamThread.AbstractTaskCreator taskCreator, - final StreamThread.AbstractTaskCreator standbyTaskCreator, + final StreamThread.TaskCreator taskCreator, + final StreamThread.StandbyTaskCreator standbyTaskCreator, final Admin adminClient, final AssignedStreamsTasks active, final AssignedStandbyTasks standby) { @@ -302,7 +302,6 @@ void shutdown(final boolean clean) { firstException.compareAndSet(null, fatalException); } taskCreator.close(); - standbyTaskCreator.close(); final RuntimeException fatalException = firstException.get(); if (fatalException != null) { diff --git a/streams/src/test/java/org/apache/kafka/streams/integration/ResetPartitionTimeIntegrationTest.java b/streams/src/test/java/org/apache/kafka/streams/integration/ResetPartitionTimeIntegrationTest.java index 75fa48233a061..955106c789f77 100644 --- a/streams/src/test/java/org/apache/kafka/streams/integration/ResetPartitionTimeIntegrationTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/integration/ResetPartitionTimeIntegrationTest.java @@ -93,7 +93,7 @@ public ResetPartitionTimeIntegrationTest(final boolean eosEnabled) { } @Test - public void shouldPreservePartitionTimeOnKafkaStreamRestart() throws Exception { + public void shouldPreservePartitionTimeOnKafkaStreamRestart() { final String appId = "appId"; final String input = "input"; final String outputRaw = "output-raw"; diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/AbstractTaskTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/AbstractTaskTest.java index 59573490358a5..3836a359635c8 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/AbstractTaskTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/AbstractTaskTest.java @@ -90,7 +90,7 @@ public void shouldNotAttemptToLockIfNoStores() { final Consumer consumer = EasyMock.createNiceMock(Consumer.class); EasyMock.replay(stateDirectory); - final AbstractTask task = createTask(consumer, Collections.emptyMap()); + final AbstractTask task = createTask(consumer, Collections.emptyMap()); task.registerStateStores(); @@ -205,16 +205,26 @@ private AbstractTask createTask(final Consumer consumer, storeNamesToChangelogTopics.put(e.getKey().name(), e.getValue()); } + final LogContext logContext = new LogContext("stream-task-test "); + final ProcessorStateManager stateManager = new ProcessorStateManager( + id, + storeTopicPartitions, + false, + stateDirectory, + storeNamesToChangelogTopics, + new StoreChangelogReader(consumer, + Duration.ZERO, + new MockStateRestoreListener(), + logContext), + logContext); + return new AbstractTask(id, storeTopicPartitions, withLocalStores(new ArrayList<>(stateStoresToChangelogTopics.keySet()), storeNamesToChangelogTopics), consumer, - new StoreChangelogReader(consumer, - Duration.ZERO, - new MockStateRestoreListener(), - new LogContext("stream-task-test ")), false, + stateManager, stateDirectory, config) { @@ -228,7 +238,7 @@ public void resume() {} public void commit() {} @Override - public void close(final boolean clean, final boolean isZombie) {} + public void close(final boolean clean) {} @Override public boolean initializeStateStores() { diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/AssignedStreamsTasksTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/AssignedStreamsTasksTest.java index 35db23d8446ea..cd3c2c38bfd52 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/AssignedStreamsTasksTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/AssignedStreamsTasksTest.java @@ -17,23 +17,11 @@ package org.apache.kafka.streams.processor.internals; -import org.apache.kafka.clients.consumer.MockConsumer; -import org.apache.kafka.clients.consumer.OffsetResetStrategy; -import org.apache.kafka.clients.producer.MockProducer; import org.apache.kafka.common.TopicPartition; -import org.apache.kafka.common.metrics.MetricConfig; -import org.apache.kafka.common.metrics.Metrics; -import org.apache.kafka.common.metrics.Sensor.RecordingLevel; -import org.apache.kafka.common.serialization.Deserializer; -import org.apache.kafka.common.serialization.Serdes; -import org.apache.kafka.common.serialization.Serializer; import org.apache.kafka.common.utils.LogContext; -import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.common.utils.Utils; import org.apache.kafka.streams.errors.TaskMigratedException; import org.apache.kafka.streams.processor.TaskId; -import org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl; -import org.apache.kafka.test.MockSourceNode; import org.easymock.EasyMock; import org.junit.Before; import org.junit.Test; @@ -52,7 +40,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.IsEqual.equalTo; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -191,7 +178,7 @@ public void shouldCloseUnInitializedTasksOnSuspend() { EasyMock.expect(t1.partitions()).andAnswer(Collections::emptySet).anyTimes(); EasyMock.expect(t1.changelogPartitions()).andAnswer(Collections::emptyList).anyTimes(); - t1.close(false, false); + t1.close(true); EasyMock.expectLastCall(); EasyMock.replay(t1); @@ -218,7 +205,7 @@ public void shouldCloseTaskOnSuspendWhenRuntimeException() { t1.suspend(); EasyMock.expectLastCall().andThrow(new RuntimeException("KABOOM!")); - t1.close(false, false); + t1.close(false); EasyMock.expectLastCall(); EasyMock.replay(t1); @@ -235,7 +222,7 @@ public void shouldCloseTaskOnSuspendIfTaskMigratedException() { t1.suspend(); EasyMock.expectLastCall().andThrow(new TaskMigratedException()); - t1.close(false, true); + t1.close(false); EasyMock.expectLastCall().andThrow(new RuntimeException("any exception")); EasyMock.replay(t1); @@ -248,9 +235,9 @@ public void shouldCloseTaskOnSuspendIfTaskMigratedException() { public void shouldCloseUncleanAndThenRethrowOnShutdownIfRuntimeException() { mockTaskInitialization(); - t1.close(true, false); + t1.close(true); EasyMock.expectLastCall().andThrow(new RuntimeException("any first exception")); - t1.close(false, false); + t1.close(false); EasyMock.expectLastCall().andThrow(new RuntimeException("any second exception")); EasyMock.replay(t1); addAndInitTask(); @@ -266,9 +253,9 @@ public void shouldCloseUncleanAndThenRethrowOnShutdownIfRuntimeException() { public void shouldCloseWithoutExceptionOnShutdownIfTaskMigratedException() { mockTaskInitialization(); - t1.close(true, false); + t1.close(true); EasyMock.expectLastCall().andThrow(new TaskMigratedException()); - t1.close(false, true); + t1.close(false); EasyMock.expectLastCall().andThrow(new RuntimeException("any second exception")); EasyMock.replay(t1); addAndInitTask(); @@ -486,74 +473,12 @@ public void shouldReturnNumberOfPunctuations() { EasyMock.verify(t1); } - @Test - public void shouldCloseCleanlyWithSuspendedTaskAndEOS() { - final String topic = "topic"; - - final Deserializer deserializer = Serdes.ByteArray().deserializer(); - final Serializer serializer = Serdes.ByteArray().serializer(); - - final MockConsumer consumer = - new MockConsumer<>(OffsetResetStrategy.EARLIEST); - final MockProducer producer = - new MockProducer<>(false, serializer, serializer); - - final MockSourceNode source = new MockSourceNode<>( - new String[] {"topic"}, - deserializer, - deserializer); - - final ChangelogReader changelogReader = new MockChangelogReader(); - - final ProcessorTopology topology = new ProcessorTopology( - Collections.singletonList(source), - Collections.singletonMap(topic, source), - Collections.emptyMap(), - Collections.emptyList(), - Collections.emptyList(), - Collections.emptyMap(), - Collections.emptySet()); - - final Set partitions = Collections.singleton( - new TopicPartition(topic, 1)); - - final Metrics metrics = new Metrics(new MetricConfig().recordLevel(RecordingLevel.DEBUG)); - - final StreamsMetricsImpl streamsMetrics = new MockStreamsMetrics(metrics); - - final MockTime time = new MockTime(); - - final StateDirectory stateDirectory = new StateDirectory( - StreamTaskTest.createConfig(true), - time, - true); - - final StreamTask task = new StreamTask( - new TaskId(0, 0), - partitions, - topology, - consumer, - changelogReader, - StreamTaskTest.createConfig(true), - streamsMetrics, - stateDirectory, - null, - time, - () -> producer); - - assignedTasks.addNewTask(task); - assignedTasks.initializeNewTasks(); - assertNull(assignedTasks.suspendOrCloseTasks(assignedTasks.allAssignedTaskIds(), revokedChangelogs)); - - assignedTasks.shutdown(true); - } - @Test public void shouldClearZombieCreatedTasks() { new TaskTestSuite() { @Override public void additionalSetup(final StreamTask task) { - task.close(false, true); + task.close(false); } @Override @@ -596,7 +521,7 @@ public void shouldClearZombieRunningTasks() { @Override public void additionalSetup(final StreamTask task) { task.initializeTopology(); - task.close(false, true); + task.close(false); } @Override @@ -619,7 +544,7 @@ public void shouldClearZombieSuspendedTasks() { public void additionalSetup(final StreamTask task) { task.initializeTopology(); task.suspend(); - task.closeSuspended(false, null); + task.closeSuspended(false); } @Override diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/ProcessorStateManagerTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/ProcessorStateManagerTest.java index 7b9445443deb3..b0519b431c9ca 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/ProcessorStateManagerTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/ProcessorStateManagerTest.java @@ -117,7 +117,7 @@ public void cleanup() throws IOException { } @Test - public void shouldRestoreStoreWithBatchingRestoreSpecification() throws Exception { + public void shouldRestoreStoreWithBatchingRestoreSpecification() { final TaskId taskId = new TaskId(0, 2); final MockBatchingStateRestoreListener batchingRestoreCallback = new MockBatchingStateRestoreListener(); @@ -141,7 +141,7 @@ public void shouldRestoreStoreWithBatchingRestoreSpecification() throws Exceptio } @Test - public void shouldRestoreStoreWithSinglePutRestoreSpecification() throws Exception { + public void shouldRestoreStoreWithSinglePutRestoreSpecification() { final TaskId taskId = new TaskId(0, 2); final Integer intKey = 1; @@ -164,7 +164,7 @@ public void shouldRestoreStoreWithSinglePutRestoreSpecification() throws Excepti } @Test - public void shouldConvertDataOnRestoreIfStoreImplementsTimestampedBytesStore() throws Exception { + public void shouldConvertDataOnRestoreIfStoreImplementsTimestampedBytesStore() { final TaskId taskId = new TaskId(0, 2); final Integer intKey = 1; @@ -187,7 +187,7 @@ public void shouldConvertDataOnRestoreIfStoreImplementsTimestampedBytesStore() t } @Test - public void testRegisterPersistentStore() throws IOException { + public void testRegisterPersistentStore() { final TaskId taskId = new TaskId(0, 2); final MockKeyValueStore persistentStore = getPersistentStore(); @@ -201,7 +201,6 @@ public void testRegisterPersistentStore() throws IOException { mkEntry(nonPersistentStoreName, nonPersistentStoreName) ), changelogReader, - false, logContext); try { @@ -213,7 +212,7 @@ public void testRegisterPersistentStore() throws IOException { } @Test - public void testRegisterNonPersistentStore() throws IOException { + public void testRegisterNonPersistentStore() { final MockKeyValueStore nonPersistentStore = new MockKeyValueStore(nonPersistentStoreName, false); // non persistent store final ProcessorStateManager stateMgr = new ProcessorStateManager( @@ -226,7 +225,6 @@ public void testRegisterNonPersistentStore() throws IOException { mkEntry(nonPersistentStoreName, nonPersistentStoreTopicName) ), changelogReader, - false, logContext); try { @@ -277,7 +275,6 @@ public void testChangeLogOffsets() throws IOException { stateDirectory, storeToChangelogTopic, changelogReader, - false, logContext); try { @@ -301,7 +298,7 @@ public void testChangeLogOffsets() throws IOException { } @Test - public void testGetStore() throws IOException { + public void testGetStore() { final MockKeyValueStore mockKeyValueStore = new MockKeyValueStore(nonPersistentStoreName, false); final ProcessorStateManager stateMgr = new ProcessorStateManager( new TaskId(0, 1), @@ -310,7 +307,6 @@ public void testGetStore() throws IOException { stateDirectory, emptyMap(), changelogReader, - false, logContext); try { stateMgr.register(mockKeyValueStore, mockKeyValueStore.stateRestoreCallback); @@ -341,7 +337,6 @@ public void testFlushAndClose() throws IOException { mkMap(mkEntry(persistentStoreName, persistentStoreTopicName), mkEntry(nonPersistentStoreName, nonPersistentStoreTopicName)), changelogReader, - false, logContext); try { // make sure the checkpoint file is not written yet @@ -386,7 +381,6 @@ public void shouldMaintainRegistrationOrderWhenReregistered() throws IOException mkMap(mkEntry(persistentStoreName, persistentStoreTopicName), mkEntry(nonPersistentStoreName, nonPersistentStoreTopicName)), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); stateMgr.register(nonPersistentStore, nonPersistentStore.stateRestoreCallback); @@ -407,7 +401,7 @@ public void shouldMaintainRegistrationOrderWhenReregistered() throws IOException } @Test - public void shouldRegisterStoreWithoutLoggingEnabledAndNotBackedByATopic() throws IOException { + public void shouldRegisterStoreWithoutLoggingEnabledAndNotBackedByATopic() { final ProcessorStateManager stateMgr = new ProcessorStateManager( new TaskId(0, 1), noPartitions, @@ -415,33 +409,11 @@ public void shouldRegisterStoreWithoutLoggingEnabledAndNotBackedByATopic() throw stateDirectory, emptyMap(), changelogReader, - false, logContext); stateMgr.register(nonPersistentStore, nonPersistentStore.stateRestoreCallback); assertNotNull(stateMgr.getStore(nonPersistentStoreName)); } - @Test - public void shouldNotChangeOffsetsIfAckedOffsetsIsNull() throws IOException { - final Map offsets = singletonMap(persistentStorePartition, 99L); - checkpoint.write(offsets); - - final MockKeyValueStore persistentStore = new MockKeyValueStore(persistentStoreName, true); - final ProcessorStateManager stateMgr = new ProcessorStateManager( - taskId, - noPartitions, - false, - stateDirectory, - emptyMap(), - changelogReader, - false, - logContext); - stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); - stateMgr.close(true); - final Map read = checkpoint.read(); - assertThat(read, equalTo(offsets)); - } - @Test public void shouldIgnoreIrrelevantLoadedCheckpoints() throws IOException { final Map offsets = mkMap( @@ -458,7 +430,6 @@ public void shouldIgnoreIrrelevantLoadedCheckpoints() throws IOException { stateDirectory, singletonMap(persistentStoreName, persistentStorePartition.topic()), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); @@ -483,7 +454,6 @@ public void shouldOverrideLoadedCheckpointsWithRestoredCheckpoints() throws IOEx stateDirectory, singletonMap(persistentStoreName, persistentStorePartition.topic()), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); @@ -508,7 +478,6 @@ public void shouldIgnoreIrrelevantRestoredCheckpoints() throws IOException { stateDirectory, singletonMap(persistentStoreName, persistentStorePartition.topic()), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); @@ -537,7 +506,6 @@ public void shouldOverrideRestoredOffsetsWithProcessedOffsets() throws IOExcepti stateDirectory, singletonMap(persistentStoreName, persistentStorePartition.topic()), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); @@ -568,7 +536,6 @@ public void shouldWriteCheckpointForPersistentLogEnabledStore() throws IOExcepti stateDirectory, singletonMap(persistentStore.name(), persistentStoreTopicName), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); @@ -586,7 +553,6 @@ public void shouldWriteCheckpointForStandbyReplica() throws IOException { stateDirectory, singletonMap(persistentStore.name(), persistentStoreTopicName), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); @@ -615,7 +581,6 @@ public void shouldNotWriteCheckpointForNonPersistent() throws IOException { stateDirectory, singletonMap(nonPersistentStoreName, nonPersistentStoreTopicName), changelogReader, - false, logContext); stateMgr.register(nonPersistentStore, nonPersistentStore.stateRestoreCallback); @@ -634,7 +599,6 @@ public void shouldNotWriteCheckpointForStoresWithoutChangelogTopic() throws IOEx stateDirectory, emptyMap(), changelogReader, - false, logContext); stateMgr.register(persistentStore, persistentStore.stateRestoreCallback); @@ -646,7 +610,7 @@ public void shouldNotWriteCheckpointForStoresWithoutChangelogTopic() throws IOEx } @Test - public void shouldThrowIllegalArgumentExceptionIfStoreNameIsSameAsCheckpointFileName() throws IOException { + public void shouldThrowIllegalArgumentExceptionIfStoreNameIsSameAsCheckpointFileName() { final ProcessorStateManager stateManager = new ProcessorStateManager( taskId, noPartitions, @@ -654,7 +618,6 @@ public void shouldThrowIllegalArgumentExceptionIfStoreNameIsSameAsCheckpointFile stateDirectory, emptyMap(), changelogReader, - false, logContext); try { @@ -666,7 +629,7 @@ public void shouldThrowIllegalArgumentExceptionIfStoreNameIsSameAsCheckpointFile } @Test - public void shouldThrowIllegalArgumentExceptionOnRegisterWhenStoreHasAlreadyBeenRegistered() throws IOException { + public void shouldThrowIllegalArgumentExceptionOnRegisterWhenStoreHasAlreadyBeenRegistered() { final ProcessorStateManager stateManager = new ProcessorStateManager( taskId, noPartitions, @@ -674,7 +637,6 @@ public void shouldThrowIllegalArgumentExceptionOnRegisterWhenStoreHasAlreadyBeen stateDirectory, emptyMap(), changelogReader, - false, logContext); stateManager.register(mockKeyValueStore, null); @@ -689,7 +651,7 @@ public void shouldThrowIllegalArgumentExceptionOnRegisterWhenStoreHasAlreadyBeen } @Test - public void shouldThrowProcessorStateExceptionOnFlushIfStoreThrowsAnException() throws IOException { + public void shouldThrowProcessorStateExceptionOnFlushIfStoreThrowsAnException() { final ProcessorStateManager stateManager = new ProcessorStateManager( taskId, @@ -698,7 +660,6 @@ public void shouldThrowProcessorStateExceptionOnFlushIfStoreThrowsAnException() stateDirectory, singletonMap(storeName, changelogTopic), changelogReader, - false, logContext); final MockKeyValueStore stateStore = new MockKeyValueStore(storeName, true) { @@ -718,7 +679,7 @@ public void flush() { } @Test - public void shouldThrowProcessorStateExceptionOnCloseIfStoreThrowsAnException() throws IOException { + public void shouldThrowProcessorStateExceptionOnCloseIfStoreThrowsAnException() { final ProcessorStateManager stateManager = new ProcessorStateManager( taskId, @@ -727,7 +688,6 @@ public void shouldThrowProcessorStateExceptionOnCloseIfStoreThrowsAnException() stateDirectory, singletonMap(storeName, changelogTopic), changelogReader, - false, logContext); final MockKeyValueStore stateStore = new MockKeyValueStore(storeName, true) { @@ -761,9 +721,8 @@ public void shouldLogAWarningIfCheckpointThrowsAnIOException() { stateDirectory, singletonMap(persistentStore.name(), persistentStoreTopicName), changelogReader, - false, logContext); - } catch (final IOException e) { + } catch (final ProcessorStateException e) { e.printStackTrace(); throw new AssertionError(e); } @@ -788,7 +747,7 @@ public void shouldLogAWarningIfCheckpointThrowsAnIOException() { } @Test - public void shouldFlushAllStoresEvenIfStoreThrowsException() throws IOException { + public void shouldFlushAllStoresEvenIfStoreThrowsException() { final AtomicBoolean flushedStore = new AtomicBoolean(false); final MockKeyValueStore stateStore1 = new MockKeyValueStore(storeName, true) { @@ -810,7 +769,6 @@ public void flush() { stateDirectory, singletonMap(storeName, changelogTopic), changelogReader, - false, logContext); stateManager.register(stateStore1, stateStore1.stateRestoreCallback); @@ -823,7 +781,7 @@ public void flush() { } @Test - public void shouldCloseAllStoresEvenIfStoreThrowsExcepiton() throws IOException { + public void shouldCloseAllStoresEvenIfStoreThrowsExcepiton() { final AtomicBoolean closedStore = new AtomicBoolean(false); @@ -846,7 +804,6 @@ public void close() { stateDirectory, singletonMap(storeName, changelogTopic), changelogReader, - false, logContext); stateManager.register(stateStore1, stateStore1.stateRestoreCallback); @@ -872,7 +829,6 @@ public void shouldDeleteCheckpointFileOnCreationIfEosEnabled() throws IOExceptio stateDirectory, emptyMap(), changelogReader, - true, logContext); assertFalse(checkpointFile.exists()); @@ -884,23 +840,14 @@ public void shouldDeleteCheckpointFileOnCreationIfEosEnabled() throws IOExceptio } @Test - public void shouldSuccessfullyReInitializeStateStoresWithEosDisable() throws Exception { - shouldSuccessfullyReInitializeStateStores(false); - } - - @Test - public void shouldSuccessfullyReInitializeStateStoresWithEosEnable() throws Exception { - shouldSuccessfullyReInitializeStateStores(true); - } - - private void shouldSuccessfullyReInitializeStateStores(final boolean eosEnabled) throws Exception { + public void shouldSuccessfullyReInitializeStateStores() { final String store2Name = "store2"; final String store2Changelog = "store2-changelog"; final TopicPartition store2Partition = new TopicPartition(store2Changelog, 0); final List changelogPartitions = asList(changelogTopicPartition, store2Partition); final Map storeToChangelog = mkMap( - mkEntry(storeName, changelogTopic), - mkEntry(store2Name, store2Changelog) + mkEntry(storeName, changelogTopic), + mkEntry(store2Name, store2Changelog) ); final MockKeyValueStore stateStore = new MockKeyValueStore(storeName, true); @@ -913,7 +860,6 @@ private void shouldSuccessfullyReInitializeStateStores(final boolean eosEnabled) stateDirectory, storeToChangelog, changelogReader, - eosEnabled, logContext); stateManager.register(stateStore, stateStore.stateRestoreCallback); @@ -933,7 +879,7 @@ public void register(final StateStore store, final StateRestoreCallback stateRes assertTrue(stateStore2.initialized); } - private ProcessorStateManager getStandByStateManager(final TaskId taskId) throws IOException { + private ProcessorStateManager getStandByStateManager(final TaskId taskId) { return new ProcessorStateManager( taskId, noPartitions, @@ -941,7 +887,6 @@ private ProcessorStateManager getStandByStateManager(final TaskId taskId) throws stateDirectory, singletonMap(persistentStoreName, persistentStoreTopicName), changelogReader, - false, logContext); } diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java index 3af25808cf255..869ddfd0262d8 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java @@ -16,6 +16,10 @@ */ package org.apache.kafka.streams.processor.internals; +import org.apache.kafka.clients.consumer.CommitFailedException; +import org.apache.kafka.clients.consumer.MockConsumer; +import org.apache.kafka.clients.consumer.OffsetAndMetadata; +import org.apache.kafka.clients.consumer.OffsetResetStrategy; import org.apache.kafka.clients.producer.Callback; import org.apache.kafka.clients.producer.MockProducer; import org.apache.kafka.clients.producer.ProducerRecord; @@ -23,10 +27,12 @@ import org.apache.kafka.clients.producer.internals.DefaultPartitioner; import org.apache.kafka.common.Cluster; import org.apache.kafka.common.KafkaException; +import org.apache.kafka.common.Metric; import org.apache.kafka.common.MetricName; import org.apache.kafka.common.Node; import org.apache.kafka.common.PartitionInfo; import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.errors.OutOfOrderSequenceException; import org.apache.kafka.common.errors.ProducerFencedException; import org.apache.kafka.common.errors.TimeoutException; import org.apache.kafka.common.errors.UnknownProducerIdException; @@ -35,22 +41,26 @@ import org.apache.kafka.common.header.internals.RecordHeader; import org.apache.kafka.common.header.internals.RecordHeaders; import org.apache.kafka.common.metrics.Metrics; -import org.apache.kafka.common.metrics.Sensor; -import org.apache.kafka.common.metrics.stats.WindowedSum; import org.apache.kafka.common.serialization.ByteArraySerializer; import org.apache.kafka.common.serialization.StringSerializer; import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.Utils; +import org.apache.kafka.streams.StreamsConfig; import org.apache.kafka.streams.errors.AlwaysContinueProductionExceptionHandler; -import org.apache.kafka.streams.errors.DefaultProductionExceptionHandler; import org.apache.kafka.streams.errors.StreamsException; +import org.apache.kafka.streams.errors.TaskMigratedException; import org.apache.kafka.streams.processor.StreamPartitioner; +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl; import org.apache.kafka.streams.processor.internals.testutil.LogCaptureAppender; +import org.apache.kafka.test.StreamsTestUtils; import org.junit.Test; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Properties; import java.util.concurrent.Future; import static org.hamcrest.MatcherAssert.assertThat; @@ -61,9 +71,13 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +// TODO K9113: should improve test coverage public class RecordCollectorTest { + private final TaskId taskId = new TaskId(0, 0); private final LogContext logContext = new LogContext("test "); + private final StreamsMetricsImpl streamsMetrics = new MockStreamsMetrics(new Metrics()); + private final StreamsConfig streamsConfig = new StreamsConfig(StreamsTestUtils.getStreamsConfig("test")); private final List infos = Arrays.asList( new PartitionInfo("topic1", 0, Node.noNode(), new Node[0], new Node[0]), @@ -78,25 +92,19 @@ public class RecordCollectorTest { private final ByteArraySerializer byteArraySerializer = new ByteArraySerializer(); private final StringSerializer stringSerializer = new StringSerializer(); - private final StreamPartitioner streamPartitioner = (topic, key, value, numPartitions) -> { - return Integer.parseInt(key) % numPartitions; - }; - - private final String topic1TimeoutHint = "Timeout exception caught when sending record to topic topic1." + - " This might happen if the producer cannot send data to the Kafka cluster and thus, its internal buffer fills up." + - " This can also happen if the broker is slow to respond, if the network connection to the broker was interrupted, or if similar circumstances arise." + - " You can increase producer parameter `max.block.ms` to increase this timeout."; + private final StreamPartitioner streamPartitioner = (topic, key, value, numPartitions) -> Integer.parseInt(key) % numPartitions; @Test public void testSpecificPartition() { final RecordCollectorImpl collector = new RecordCollectorImpl( - "RecordCollectorTest-TestSpecificPartition", - new LogContext("RecordCollectorTest-TestSpecificPartition "), - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") + taskId, + streamsConfig, + logContext, + streamsMetrics, + null, + id -> new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) ); - collector.init(new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer)); final Headers headers = new RecordHeaders(new Header[]{new RecordHeader("key", "value".getBytes())}); @@ -129,12 +137,13 @@ public void testSpecificPartition() { public void testStreamPartitioner() { final RecordCollectorImpl collector = new RecordCollectorImpl( - "RecordCollectorTest-TestStreamPartitioner", - new LogContext("RecordCollectorTest-TestStreamPartitioner "), - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") + taskId, + streamsConfig, + logContext, + streamsMetrics, + null, + id -> new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) ); - collector.init(new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer)); final Headers headers = new RecordHeaders(new Header[]{new RecordHeader("key", "value".getBytes())}); @@ -163,12 +172,13 @@ public void shouldNotAllowOffsetsToBeUpdatedExternally() { final TopicPartition topicPartition = new TopicPartition(topic, 0); final RecordCollectorImpl collector = new RecordCollectorImpl( - "RecordCollectorTest-TestSpecificPartition", - new LogContext("RecordCollectorTest-TestSpecificPartition "), - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") + taskId, + streamsConfig, + logContext, + streamsMetrics, + null, + id -> new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) ); - collector.init(new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer)); collector.send(topic, "999", "0", null, 0, null, stringSerializer, stringSerializer); collector.send(topic, "999", "0", null, 0, null, stringSerializer, stringSerializer); @@ -182,20 +192,21 @@ public void shouldNotAllowOffsetsToBeUpdatedExternally() { assertThat(collector.offsets().get(topicPartition), equalTo(2L)); } - @Test + @Test(expected = StreamsException.class) public void shouldThrowStreamsExceptionOnAnyExceptionButProducerFencedException() { final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + streamsConfig, logContext, - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - throw new KafkaException(); + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + throw new KafkaException(); + } } - }); + ); final StreamsException thrown = assertThrows(StreamsException.class, () -> collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner) @@ -203,20 +214,21 @@ public synchronized Future send(final ProducerRecord(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - throw new KafkaException(new ProducerFencedException("asdf")); + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + throw new KafkaException(new ProducerFencedException("asdf")); + } } - }); + ); final RecoverableClientException thrown = assertThrows(RecoverableClientException.class, () -> collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner) @@ -228,17 +240,18 @@ public synchronized Future send(final ProducerRecord(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - throw new KafkaException(new UnknownProducerIdException("asdf")); + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + throw new KafkaException(new UnknownProducerIdException("asdf")); + } } - }); + ); final RecoverableClientException thrown = assertThrows(RecoverableClientException.class, () -> collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner) @@ -250,17 +263,19 @@ public synchronized Future send(final ProducerRecord(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new ProducerFencedException("asdf")); - return null; + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new ProducerFencedException("asdf")); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); final RecoverableClientException thrown = assertThrows(RecoverableClientException.class, () -> @@ -272,17 +287,18 @@ public synchronized Future send(final ProducerRecord(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new UnknownProducerIdException("asdf")); - return null; - } - }); + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new UnknownProducerIdException("asdf")); + return null; + } + }); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); final RecoverableClientException thrown = assertThrows(RecoverableClientException.class, () -> @@ -291,22 +307,22 @@ public synchronized Future send(final ProducerRecord send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new Exception()); - return null; + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new Exception()); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); @@ -316,71 +332,60 @@ public synchronized Future send(final ProducerRecord record, fin } catch (final StreamsException expected) { /* ok */ } } - @SuppressWarnings("unchecked") @Test public void shouldNotThrowStreamsExceptionOnSubsequentCallIfASendFailsWithContinueExceptionHandler() { + final Metrics metrics = new Metrics(); + final LogCaptureAppender logCaptureAppender = LogCaptureAppender.createAndRegister(); + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.DEFAULT_PRODUCTION_EXCEPTION_HANDLER_CLASS_CONFIG, AlwaysContinueProductionExceptionHandler.class.getName()); final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + new StreamsConfig(props), logContext, - new AlwaysContinueProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new Exception()); - return null; + new MockStreamsMetrics(metrics), + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new Exception()); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); + collector.flush(); - collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); - } + final Metric metric = metrics.metrics().get(new MetricName( + "dropped-records-total", + "stream-task-metrics", + "The total number of dropped records", + Utils.mkMap(Utils.mkEntry("thread-id", Thread.currentThread().getName()), Utils.mkEntry("task-id", taskId.toString())))); + assertEquals(1.0, metric.metricValue()); - @SuppressWarnings("unchecked") - @Test - public void shouldRecordSkippedMetricAndLogWarningIfSendFailsWithContinueExceptionHandler() { - final Metrics metrics = new Metrics(); - final Sensor sensor = metrics.sensor("dropped-records"); - final LogCaptureAppender logCaptureAppender = LogCaptureAppender.createAndRegister(); - final MetricName metricName = new MetricName("name", "group", "description", Collections.emptyMap()); - sensor.add(metricName, new WindowedSum()); - final RecordCollector collector = new RecordCollectorImpl( - "test", - logContext, - new AlwaysContinueProductionExceptionHandler(), - metrics.sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new Exception()); - return null; - } - }); - collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); - assertEquals(1.0, metrics.metrics().get(metricName).metricValue()); - assertTrue(logCaptureAppender.getMessages().contains("test Error sending records topic=[topic1] and partition=[0]; The exception handler chose to CONTINUE processing in spite of this error. Enable TRACE logging to view failed messages key and value.")); + final List messages = logCaptureAppender.getMessages(); + assertTrue(messages.get(messages.size() - 1).endsWith("Exception handler choose to CONTINUE processing in spite of this error but written offsets would not be recorded.")); LogCaptureAppender.unregister(logCaptureAppender); + + collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); } - @SuppressWarnings("unchecked") @Test public void shouldThrowStreamsExceptionOnFlushIfASendFailedWithDefaultExceptionHandler() { final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + streamsConfig, logContext, - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new Exception()); - return null; + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new Exception()); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); @@ -393,63 +398,66 @@ public synchronized Future send(final ProducerRecord record, fin @Test public void shouldThrowStreamsExceptionWithTimeoutHintOnProducerTimeoutWithDefaultExceptionHandler() { final RecordCollector collector = new RecordCollectorImpl( - "test", - logContext, - new DefaultProductionExceptionHandler(), - new Metrics().sensor("skipped-records")); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new TimeoutException()); - return null; + taskId, + streamsConfig, + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new TimeoutException()); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); - final StreamsException expected = assertThrows(StreamsException.class, () -> collector.flush()); + final StreamsException expected = assertThrows(StreamsException.class, collector::flush); assertTrue(expected.getCause() instanceof TimeoutException); - assertTrue(expected.getMessage().endsWith(topic1TimeoutHint)); } - @SuppressWarnings("unchecked") @Test public void shouldNotThrowStreamsExceptionOnFlushIfASendFailedWithContinueExceptionHandler() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.DEFAULT_PRODUCTION_EXCEPTION_HANDLER_CLASS_CONFIG, AlwaysContinueProductionExceptionHandler.class.getName()); final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + new StreamsConfig(props), logContext, - new AlwaysContinueProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new Exception()); - return null; + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new Exception()); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); collector.flush(); } - @SuppressWarnings("unchecked") @Test public void shouldThrowStreamsExceptionOnCloseIfASendFailedWithDefaultExceptionHandler() { final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + streamsConfig, logContext, - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new Exception()); - return null; + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new Exception()); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); @@ -459,63 +467,281 @@ public synchronized Future send(final ProducerRecord record, fin } catch (final StreamsException expected) { /* ok */ } } - @SuppressWarnings("unchecked") @Test public void shouldNotThrowStreamsExceptionOnCloseIfASendFailedWithContinueExceptionHandler() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.DEFAULT_PRODUCTION_EXCEPTION_HANDLER_CLASS_CONFIG, AlwaysContinueProductionExceptionHandler.class.getName()); final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + new StreamsConfig(props), logContext, - new AlwaysContinueProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public synchronized Future send(final ProducerRecord record, final Callback callback) { - callback.onCompletion(null, new Exception()); - return null; + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new Exception()); + return null; + } } - }); + ); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); collector.close(); } - @SuppressWarnings("unchecked") + @Test + public void shouldThrowStreamsExceptionOnEOSInitializeTimeout() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + + assertThrows(StreamsException.class, () -> + new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public void initTransactions() { + throw new TimeoutException("test"); + } + } + ) + ); + } + + @Test + public void shouldThrowMigrateExceptionOnEOSProcessFenced() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + throw new KafkaException(new ProducerFencedException("boom")); + } + } + ); + + assertThrows(TaskMigratedException.class, () -> collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner)); + } + + @Test + public void shouldFailWithMigrateExceptionOnEOSProcessFenced() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new ProducerFencedException("boom")); + return null; + } + } + ); + + collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); + + assertThrows(TaskMigratedException.class, () -> collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner)); + } + + @Test + public void shouldFailWithMigrateExceptionOnEOSProcessUnknownPid() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public synchronized Future send(final ProducerRecord record, final Callback callback) { + callback.onCompletion(null, new OutOfOrderSequenceException("boom")); + return null; + } + } + ); + + collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); + + assertThrows(TaskMigratedException.class, + () -> collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner)); + } + + @Test + public void shouldFailWithMigrateExceptionOnCommitFailed() { + final RecordCollector collector = new RecordCollectorImpl( + taskId, + streamsConfig, + logContext, + streamsMetrics, + new MockConsumer(OffsetResetStrategy.EARLIEST) { + @Override + public void commitSync(final Map offsets) { + throw new CommitFailedException(); + } + }, + id -> new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) + ); + + assertThrows(TaskMigratedException.class, () -> collector.commit(null)); + } + + @Test + public void shouldFailWithMigrateExceptionOnCommitUnexpected() { + final RecordCollector collector = new RecordCollectorImpl( + taskId, + streamsConfig, + logContext, + streamsMetrics, + new MockConsumer(OffsetResetStrategy.EARLIEST) { + @Override + public void commitSync(final Map offsets) { + throw new KafkaException(); + } + }, + id -> new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) + ); + + assertThrows(StreamsException.class, () -> collector.commit(Collections.singletonMap(new TopicPartition("topic", 0), new OffsetAndMetadata(5L)))); + } + + @Test + public void shouldFailWithMigrateExceptionOnEOSSendOffsetFenced() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public void sendOffsetsToTransaction(final Map offsets, final String consumerGroupId) { + throw new ProducerFencedException("boom"); + } + } + ); + + assertThrows(TaskMigratedException.class, () -> collector.commit(Collections.singletonMap(new TopicPartition("topic", 0), new OffsetAndMetadata(5L)))); + } + + @Test + public void shouldFailWithMigrateExceptionOnEOSCommitFenced() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public void commitTransaction() { + throw new ProducerFencedException("boom"); + } + } + ); + + assertThrows(TaskMigratedException.class, () -> collector.commit(Collections.singletonMap(new TopicPartition("topic", 0), new OffsetAndMetadata(5L)))); + } + + @Test + public void shouldFailWithStreamsExceptionOnEOSSendOffsetUnexpected() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public void sendOffsetsToTransaction(final Map offsets, final String consumerGroupId) { + throw new KafkaException("boom"); + } + } + ); + + assertThrows(StreamsException.class, () -> collector.commit(Collections.singletonMap(new TopicPartition("topic", 0), new OffsetAndMetadata(5L)))); + } + + @Test + public void shouldFailWithMigrateExceptionOnEOSCommitUnexpected() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.PROCESSING_GUARANTEE_CONFIG, StreamsConfig.EXACTLY_ONCE); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + new StreamsConfig(props), + logContext, + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public void commitTransaction() { + throw new KafkaException("boom"); + } + } + ); + + assertThrows(StreamsException.class, () -> collector.commit(Collections.singletonMap(new TopicPartition("topic", 0), new OffsetAndMetadata(5L)))); + } + @Test(expected = StreamsException.class) public void shouldThrowIfTopicIsUnknownWithDefaultExceptionHandler() { final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + streamsConfig, logContext, - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public List partitionsFor(final String topic) { - return Collections.emptyList(); + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public List partitionsFor(final String topic) { + return Collections.emptyList(); + } } + ); - }); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); } - @SuppressWarnings("unchecked") @Test(expected = StreamsException.class) public void shouldThrowIfTopicIsUnknownWithContinueExceptionHandler() { + final Properties props = StreamsTestUtils.getStreamsConfig("test"); + props.setProperty(StreamsConfig.DEFAULT_PRODUCTION_EXCEPTION_HANDLER_CLASS_CONFIG, AlwaysContinueProductionExceptionHandler.class.getName()); final RecordCollector collector = new RecordCollectorImpl( - "test", + taskId, + new StreamsConfig(props), logContext, - new AlwaysContinueProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - collector.init(new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { - @Override - public List partitionsFor(final String topic) { - return Collections.emptyList(); + streamsMetrics, + null, + id -> new MockProducer(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer) { + @Override + public List partitionsFor(final String topic) { + return Collections.emptyList(); + } } + ); - }); collector.send("topic1", "3", "0", null, null, stringSerializer, stringSerializer, streamPartitioner); } @@ -525,15 +751,16 @@ public void testRecordHeaderPassThroughSerializer() { final CustomStringSerializer valueSerializer = new CustomStringSerializer(); keySerializer.configure(Collections.emptyMap(), true); - final RecordCollectorImpl collector = new RecordCollectorImpl( - "test", + final MockProducer mockProducer = + new MockProducer<>(cluster, true, new DefaultPartitioner(), byteArraySerializer, byteArraySerializer); + final RecordCollector collector = new RecordCollectorImpl( + taskId, + streamsConfig, logContext, - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") + streamsMetrics, + null, + id -> mockProducer ); - final MockProducer mockProducer = new MockProducer<>(cluster, true, new DefaultPartitioner(), - byteArraySerializer, byteArraySerializer); - collector.init(mockProducer); collector.send("topic1", "3", "0", new RecordHeaders(), null, keySerializer, valueSerializer, streamPartitioner); @@ -546,18 +773,6 @@ public void testRecordHeaderPassThroughSerializer() { } } - @Test - public void testShouldNotThrowNPEOnCloseIfProducerIsNotInitialized() { - final RecordCollectorImpl collector = new RecordCollectorImpl( - "NoNPE", - logContext, - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - - collector.close(); - } - private static class CustomStringSerializer extends StringSerializer { private boolean isKey; diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordQueueTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordQueueTest.java index d12481a496924..c32feb297fb41 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordQueueTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordQueueTest.java @@ -18,8 +18,6 @@ import org.apache.kafka.clients.consumer.ConsumerRecord; import org.apache.kafka.common.TopicPartition; -import org.apache.kafka.common.metrics.Metrics; -import org.apache.kafka.common.metrics.Sensor; import org.apache.kafka.common.record.TimestampType; import org.apache.kafka.common.serialization.Deserializer; import org.apache.kafka.common.serialization.IntegerDeserializer; @@ -28,7 +26,6 @@ import org.apache.kafka.common.serialization.Serializer; import org.apache.kafka.common.utils.Bytes; import org.apache.kafka.common.utils.LogContext; -import org.apache.kafka.streams.errors.DefaultProductionExceptionHandler; import org.apache.kafka.streams.errors.LogAndContinueExceptionHandler; import org.apache.kafka.streams.errors.LogAndFailExceptionHandler; import org.apache.kafka.streams.errors.StreamsException; @@ -37,6 +34,7 @@ import org.apache.kafka.streams.processor.TimestampExtractor; import org.apache.kafka.streams.state.StateSerdes; import org.apache.kafka.test.InternalMockProcessorContext; +import org.apache.kafka.test.MockRecordCollector; import org.apache.kafka.test.MockSourceNode; import org.apache.kafka.test.MockTimestampExtractor; import org.junit.After; @@ -56,16 +54,9 @@ public class RecordQueueTest { private final TimestampExtractor timestampExtractor = new MockTimestampExtractor(); private final String[] topics = {"topic"}; - private final Sensor droppedRecordsSensor = new Metrics().sensor("skipped-records"); - final InternalMockProcessorContext context = new InternalMockProcessorContext( StateSerdes.withBuiltinTypes("anyName", Bytes.class, Bytes.class), - new RecordCollectorImpl( - null, - new LogContext("record-queue-test "), - new DefaultProductionExceptionHandler(), - droppedRecordsSensor - ) + new MockRecordCollector() ); private final MockSourceNode mockSourceNodeWithMetrics = new MockSourceNode<>(topics, intDeserializer, intDeserializer); private final RecordQueue queue = new RecordQueue( diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/SinkNodeTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/SinkNodeTest.java index 8abb7fd8f6a8f..bee33dd454edd 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/SinkNodeTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/SinkNodeTest.java @@ -16,16 +16,15 @@ */ package org.apache.kafka.streams.processor.internals; -import org.apache.kafka.clients.producer.MockProducer; -import org.apache.kafka.common.metrics.Metrics; +import org.apache.kafka.common.header.Headers; import org.apache.kafka.common.serialization.Serdes; import org.apache.kafka.common.serialization.Serializer; import org.apache.kafka.common.utils.Bytes; -import org.apache.kafka.common.utils.LogContext; -import org.apache.kafka.streams.errors.DefaultProductionExceptionHandler; import org.apache.kafka.streams.errors.StreamsException; +import org.apache.kafka.streams.processor.StreamPartitioner; import org.apache.kafka.streams.state.StateSerdes; import org.apache.kafka.test.InternalMockProcessorContext; +import org.apache.kafka.test.MockRecordCollector; import org.junit.Before; import org.junit.Test; @@ -35,19 +34,23 @@ import static org.junit.Assert.fail; public class SinkNodeTest { - private final Serializer anySerializer = Serdes.ByteArray().serializer(); private final StateSerdes anyStateSerde = StateSerdes.withBuiltinTypes("anyName", Bytes.class, Bytes.class); - private final RecordCollector recordCollector = new RecordCollectorImpl( - null, - new LogContext("sinknode-test "), - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); + private final Serializer anySerializer = Serdes.ByteArray().serializer(); + private final RecordCollector recordCollector = new MockRecordCollector() { + @Override + public void send(final String topic, + final K key, + final V value, + final Headers headers, + final Long timestamp, + final Serializer keySerializer, + final Serializer valueSerializer, + final StreamPartitioner partitioner) { + throw new ClassCastException("boom"); + } + }; - private final InternalMockProcessorContext context = new InternalMockProcessorContext( - anyStateSerde, - recordCollector - ); + private final InternalMockProcessorContext context = new InternalMockProcessorContext(anyStateSerde, recordCollector); private final SinkNode sink = new SinkNode<>("anyNodeName", new StaticTopicNameExtractor<>("any-output-topic"), anySerializer, anySerializer, null); @@ -57,19 +60,15 @@ public class SinkNodeTest { @Before public void before() { - recordCollector.init(new MockProducer<>(true, anySerializer, anySerializer)); sink.init(context); } @Test public void shouldThrowStreamsExceptionOnInputRecordWithInvalidTimestamp() { - final Bytes anyKey = new Bytes("any key".getBytes()); - final Bytes anyValue = new Bytes("any value".getBytes()); - // When/Then context.setTime(-1); // ensures a negative timestamp is set for the record we send next try { - illTypedSink.process(anyKey, anyValue); + illTypedSink.process("any key".getBytes(), "any value".getBytes()); fail("Should have thrown StreamsException"); } catch (final StreamsException ignored) { // expected @@ -77,14 +76,11 @@ public void shouldThrowStreamsExceptionOnInputRecordWithInvalidTimestamp() { } @Test - public void shouldThrowStreamsExceptionOnKeyValueTypeSerializerMismatch() { - final String keyOfDifferentTypeThanSerializer = "key with different type"; - final String valueOfDifferentTypeThanSerializer = "value with different type"; - + public void shouldThrowStreamsExceptionWithClassCastFromRecordCollector() { // When/Then context.setTime(0); try { - illTypedSink.process(keyOfDifferentTypeThanSerializer, valueOfDifferentTypeThanSerializer); + illTypedSink.process("key", "value"); fail("Should have thrown StreamsException"); } catch (final StreamsException e) { assertThat(e.getCause(), instanceOf(ClassCastException.class)); @@ -92,13 +88,11 @@ public void shouldThrowStreamsExceptionOnKeyValueTypeSerializerMismatch() { } @Test - public void shouldHandleNullKeysWhenThrowingStreamsExceptionOnKeyValueTypeSerializerMismatch() { - final String invalidValueToTriggerSerializerMismatch = ""; - + public void shouldThrowStreamsExceptionNullKeyWithClassCastFromRecordCollector() { // When/Then context.setTime(1); try { - illTypedSink.process(null, invalidValueToTriggerSerializerMismatch); + illTypedSink.process(null, ""); fail("Should have thrown StreamsException"); } catch (final StreamsException e) { assertThat(e.getCause(), instanceOf(ClassCastException.class)); @@ -107,13 +101,11 @@ public void shouldHandleNullKeysWhenThrowingStreamsExceptionOnKeyValueTypeSerial } @Test - public void shouldHandleNullValuesWhenThrowingStreamsExceptionOnKeyValueTypeSerializerMismatch() { - final String invalidKeyToTriggerSerializerMismatch = ""; - + public void shouldThrowStreamsExceptionNullValueWithClassCastFromRecordCollector() { // When/Then context.setTime(1); try { - illTypedSink.process(invalidKeyToTriggerSerializerMismatch, null); + illTypedSink.process("", null); fail("Should have thrown StreamsException"); } catch (final StreamsException e) { assertThat(e.getCause(), instanceOf(ClassCastException.class)); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StandbyTaskTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StandbyTaskTest.java index 017f817d630c4..917dd2a9c00be 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StandbyTaskTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StandbyTaskTest.java @@ -30,47 +30,28 @@ import org.apache.kafka.common.metrics.Sensor; import org.apache.kafka.common.metrics.stats.CumulativeSum; import org.apache.kafka.common.record.TimestampType; -import org.apache.kafka.common.serialization.IntegerDeserializer; import org.apache.kafka.common.serialization.IntegerSerializer; -import org.apache.kafka.common.serialization.LongSerializer; -import org.apache.kafka.common.serialization.Serdes; -import org.apache.kafka.common.serialization.Serializer; -import org.apache.kafka.common.utils.Bytes; -import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.common.utils.Utils; -import org.apache.kafka.streams.KeyValue; import org.apache.kafka.streams.StreamsConfig; import org.apache.kafka.streams.errors.ProcessorStateException; -import org.apache.kafka.streams.kstream.Materialized; -import org.apache.kafka.streams.kstream.TimeWindows; -import org.apache.kafka.streams.kstream.Windowed; -import org.apache.kafka.streams.kstream.internals.ConsumedInternal; -import org.apache.kafka.streams.kstream.internals.InternalStreamsBuilder; -import org.apache.kafka.streams.kstream.internals.InternalStreamsBuilderTest; -import org.apache.kafka.streams.kstream.internals.TimeWindow; import org.apache.kafka.streams.processor.TaskId; import org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl; -import org.apache.kafka.streams.state.KeyValueIterator; -import org.apache.kafka.streams.state.TimestampedWindowStore; -import org.apache.kafka.streams.state.ValueAndTimestamp; -import org.apache.kafka.streams.state.WindowStore; -import org.apache.kafka.streams.state.internals.OffsetCheckpoint; -import org.apache.kafka.streams.state.internals.WindowKeySchema; -import org.apache.kafka.test.MockKeyValueStore; import org.apache.kafka.test.MockKeyValueStoreBuilder; import org.apache.kafka.test.MockRestoreConsumer; -import org.apache.kafka.test.MockStateRestoreListener; import org.apache.kafka.test.MockTimestampExtractor; import org.apache.kafka.test.TestUtils; +import org.easymock.EasyMock; +import org.easymock.EasyMockRunner; +import org.easymock.Mock; +import org.easymock.MockType; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import java.io.File; import java.io.IOException; -import java.time.Duration; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -80,27 +61,24 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import static java.time.Duration.ofMillis; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; -import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; import static org.apache.kafka.common.utils.Utils.mkEntry; import static org.apache.kafka.common.utils.Utils.mkMap; import static org.apache.kafka.common.utils.Utils.mkProperties; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +@RunWith(EasyMockRunner.class) public class StandbyTaskTest { private final String threadId = Thread.currentThread().getName(); private final TaskId taskId = new TaskId(0, 1); private StandbyTask task; - private final Serializer intSerializer = new IntegerSerializer(); private final String applicationId = "test-application"; private final String storeName1 = "store1"; @@ -109,10 +87,7 @@ public class StandbyTaskTest { private final String storeChangelogTopicName2 = ProcessorStateManager.storeChangelogTopic(applicationId, storeName2); private final String globalStoreName = "ktable1"; - private final TopicPartition partition1 = new TopicPartition(storeChangelogTopicName1, 1); - private final TopicPartition partition2 = new TopicPartition(storeChangelogTopicName2, 1); - private final MockStateRestoreListener stateRestoreListener = new MockStateRestoreListener(); - + private final TopicPartition partition = new TopicPartition(storeChangelogTopicName1, 1); private final Set topicPartitions = Collections.emptySet(); private final ProcessorTopology topology = ProcessorTopologyFactories.withLocalStores( asList(new MockKeyValueStoreBuilder(storeName1, false).build(), @@ -150,20 +125,17 @@ private StreamsConfig createConfig(final File baseDir) throws IOException { new IntegerSerializer(), new IntegerSerializer() ); - private final StoreChangelogReader changelogReader = new StoreChangelogReader( - restoreStateConsumer, - Duration.ZERO, - stateRestoreListener, - new LogContext("standby-task-test ") - ); - - private final byte[] recordValue = intSerializer.serialize(null, 10); - private final byte[] recordKey = intSerializer.serialize(null, 1); private final String threadName = "threadName"; private final StreamsMetricsImpl streamsMetrics = new StreamsMetricsImpl(new Metrics(), threadName, StreamsConfig.METRICS_LATEST); + @Mock(type = MockType.NICE) + private ProcessorStateManager stateManager; + + @Mock(type = MockType.NICE) + private RecordCollector recordCollector; + @Before public void setup() throws Exception { restoreStateConsumer.reset(); @@ -185,7 +157,7 @@ public void setup() throws Exception { @After public void cleanup() throws IOException { if (task != null && !task.isClosed()) { - task.close(true, false); + task.close(true); task = null; } Utils.delete(baseDir); @@ -193,294 +165,21 @@ public void cleanup() throws IOException { @Test public void testStorePartitions() throws IOException { - final StreamsConfig config = createConfig(baseDir); - task = new StandbyTask(taskId, - topicPartitions, - topology, - consumer, - changelogReader, - config, - streamsMetrics, - stateDirectory); - task.initializeStateStores(); - assertEquals(Utils.mkSet(partition2, partition1), new HashSet<>(task.checkpointedOffsets().keySet())); - } + EasyMock.expect(stateManager.checkpointed()).andReturn(Collections.singletonMap(partition, 50L)); + EasyMock.replay(stateManager); - @SuppressWarnings("unchecked") - @Test - public void testUpdateNonInitializedStore() throws IOException { final StreamsConfig config = createConfig(baseDir); task = new StandbyTask(taskId, - topicPartitions, - topology, - consumer, - changelogReader, - config, - streamsMetrics, - stateDirectory); - - restoreStateConsumer.assign(new ArrayList<>(task.checkpointedOffsets().keySet())); - - try { - task.update(partition1, - singletonList( - new ConsumerRecord<>( - partition1.topic(), - partition1.partition(), - 10, - 0L, - TimestampType.CREATE_TIME, - 0L, - 0, - 0, - recordKey, - recordValue)) - ); - fail("expected an exception"); - } catch (final NullPointerException npe) { - assertThat(npe.getMessage(), containsString("stateRestoreCallback must not be null")); - } - - } - - @Test - public void testUpdate() throws IOException { - final StreamsConfig config = createConfig(baseDir); - task = new StandbyTask(taskId, - topicPartitions, - topology, - consumer, - changelogReader, - config, - streamsMetrics, - stateDirectory); - task.initializeStateStores(); - assertThat(task.checkpointedOffsets(), - equalTo(mkMap(mkEntry(partition1, -1L), mkEntry(partition2, -1L)))); - final Set partition = Collections.singleton(partition2); - restoreStateConsumer.assign(partition); - - for (final ConsumerRecord record : asList(new ConsumerRecord<>(partition2.topic(), - partition2.partition(), - 10, - 0L, - TimestampType.CREATE_TIME, - 0L, - 0, - 0, - 1, - 100), - new ConsumerRecord<>(partition2.topic(), - partition2.partition(), - 20, - 0L, - TimestampType.CREATE_TIME, - 0L, - 0, - 0, - 2, - 100), - new ConsumerRecord<>(partition2.topic(), - partition2.partition(), - 30, - 0L, - TimestampType.CREATE_TIME, - 0L, - 0, - 0, - 3, - 100))) { - restoreStateConsumer.bufferRecord(record); - } - - restoreStateConsumer.seekToBeginning(partition); - task.update(partition2, restoreStateConsumer.poll(ofMillis(100)).records(partition2)); - assertThat( - task.checkpointedOffsets(), - equalTo( - mkMap( - mkEntry(partition1, -1L), - mkEntry(partition2, 31L /*the checkpoint should be 1+ the highest consumed offset*/) - ) - ) - ); - final StandbyContextImpl context = (StandbyContextImpl) task.context(); - final MockKeyValueStore store1 = (MockKeyValueStore) context.getStateMgr().getStore(storeName1); - final MockKeyValueStore store2 = (MockKeyValueStore) context.getStateMgr().getStore(storeName2); - - assertEquals(Collections.emptyList(), store1.keys); - assertEquals(asList(1, 2, 3), store2.keys); - } - - @Test - public void shouldRestoreToWindowedStores() throws IOException { - final String storeName = "windowed-store"; - final String changelogName = applicationId + "-" + storeName + "-changelog"; - - final TopicPartition topicPartition = new TopicPartition(changelogName, 1); - - final Set partitions = Collections.singleton(topicPartition); - - consumer.assign(partitions); - - final InternalTopologyBuilder internalTopologyBuilder = new InternalTopologyBuilder().setApplicationId(applicationId); - - final InternalStreamsBuilder builder = new InternalStreamsBuilder(internalTopologyBuilder); - - builder - .stream(Collections.singleton("topic"), new ConsumedInternal<>()) - .groupByKey() - .windowedBy(TimeWindows.of(ofMillis(60_000)).grace(ofMillis(0L))) - .count(Materialized.>as(storeName).withRetention(ofMillis(120_000L))); - - builder.buildAndOptimizeTopology(); - - task = new StandbyTask( - taskId, - partitions, - internalTopologyBuilder.build(0), - consumer, - new StoreChangelogReader( - restoreStateConsumer, - Duration.ZERO, - stateRestoreListener, - new LogContext("standby-task-test ") - ), - createConfig(baseDir), - new MockStreamsMetrics(new Metrics()), - stateDirectory - ); - - task.initializeStateStores(); - - consumer.commitSync(mkMap(mkEntry(topicPartition, new OffsetAndMetadata(35L)))); - task.commit(); - - final List> remaining1 = task.update( - topicPartition, - asList( - makeWindowedConsumerRecord(changelogName, 10, 1, 0L, 60_000L), - makeWindowedConsumerRecord(changelogName, 20, 2, 60_000L, 120_000), - makeWindowedConsumerRecord(changelogName, 30, 3, 120_000L, 180_000), - makeWindowedConsumerRecord(changelogName, 40, 4, 180_000L, 240_000) - ) - ); - - assertEquals( - asList( - new KeyValue<>(new Windowed<>(1, new TimeWindow(0, 60_000)), ValueAndTimestamp.make(100L, 60_000L)), - new KeyValue<>(new Windowed<>(2, new TimeWindow(60_000, 120_000)), ValueAndTimestamp.make(100L, 120_000L)), - new KeyValue<>(new Windowed<>(3, new TimeWindow(120_000, 180_000)), ValueAndTimestamp.make(100L, 180_000L)) - ), - getWindowedStoreContents(storeName, task) - ); - - consumer.commitSync(mkMap(mkEntry(topicPartition, new OffsetAndMetadata(45L)))); - task.commit(); - - final List> remaining2 = task.update(topicPartition, remaining1); - assertEquals(emptyList(), remaining2); - - // the first record's window should have expired. - assertEquals( - asList( - new KeyValue<>(new Windowed<>(2, new TimeWindow(60_000, 120_000)), ValueAndTimestamp.make(100L, 120_000L)), - new KeyValue<>(new Windowed<>(3, new TimeWindow(120_000, 180_000)), ValueAndTimestamp.make(100L, 180_000L)), - new KeyValue<>(new Windowed<>(4, new TimeWindow(180_000, 240_000)), ValueAndTimestamp.make(100L, 240_000L)) - ), - getWindowedStoreContents(storeName, task) - ); - } - - private ConsumerRecord makeWindowedConsumerRecord(final String changelogName, - final int offset, - final int key, - final long start, - final long end) { - final Windowed data = new Windowed<>(key, new TimeWindow(start, end)); - final Bytes wrap = Bytes.wrap(new IntegerSerializer().serialize(null, data.key())); - final byte[] keyBytes = WindowKeySchema.toStoreKeyBinary(new Windowed<>(wrap, data.window()), 1).get(); - return new ConsumerRecord<>( - changelogName, - 1, - offset, - end, - TimestampType.CREATE_TIME, - 0L, - 0, - 0, - keyBytes, - new LongSerializer().serialize(null, 100L) - ); - } - - @Test - public void shouldWriteCheckpointFile() throws IOException { - final String storeName = "checkpoint-file-store"; - final String changelogName = applicationId + "-" + storeName + "-changelog"; - - final TopicPartition topicPartition = new TopicPartition(changelogName, 1); - final Set partitions = Collections.singleton(topicPartition); - - final InternalTopologyBuilder internalTopologyBuilder = new InternalTopologyBuilder().setApplicationId(applicationId); - - final InternalStreamsBuilder builder = new InternalStreamsBuilder(internalTopologyBuilder); - builder.stream(Collections.singleton("topic"), new ConsumedInternal<>()) - .groupByKey() - .count(Materialized.as(storeName)); - - builder.buildAndOptimizeTopology(); - - consumer.assign(partitions); - - task = new StandbyTask( - taskId, - partitions, - internalTopologyBuilder.build(0), + topicPartitions, + topology, consumer, - changelogReader, - createConfig(baseDir), - new MockStreamsMetrics(new Metrics()), - stateDirectory - ); + config, + streamsMetrics, + stateManager, + stateDirectory); task.initializeStateStores(); - consumer.commitSync(mkMap(mkEntry(topicPartition, new OffsetAndMetadata(20L)))); - task.commit(); - - task.update( - topicPartition, - singletonList(makeWindowedConsumerRecord(changelogName, 10, 1, 0L, 60_000L)) - ); - - task.close(true, false); - - final File taskDir = stateDirectory.directoryForTask(taskId); - final OffsetCheckpoint checkpoint = new OffsetCheckpoint(new File(taskDir, StateManagerUtil.CHECKPOINT_FILE_NAME)); - final Map offsets = checkpoint.read(); - - assertEquals(1, offsets.size()); - assertEquals(Long.valueOf(11L), offsets.get(topicPartition)); - } - - @SuppressWarnings("unchecked") - private List, ValueAndTimestamp>> getWindowedStoreContents(final String storeName, - final StandbyTask task) { - final StandbyContextImpl context = (StandbyContextImpl) task.context(); - - final List, ValueAndTimestamp>> result = new ArrayList<>(); - - try (final KeyValueIterator, ValueAndTimestamp> iterator = - ((TimestampedWindowStore) context.getStateMgr().getStore(storeName)).all()) { - - while (iterator.hasNext()) { - final KeyValue, ValueAndTimestamp> next = iterator.next(); - final Integer deserializedKey = new IntegerDeserializer().deserialize(null, next.key.key()); - result.add(new KeyValue<>(new Windowed<>(deserializedKey, next.key.window()), next.value)); - } - } - - return result; + assertEquals(Utils.mkSet(partition), new HashSet<>(task.checkpointedOffsets().keySet())); } @Test @@ -493,9 +192,9 @@ public void shouldRestoreToKTable() throws IOException { ktablePartitions, ktableTopology, consumer, - changelogReader, createConfig(baseDir), streamsMetrics, + stateManager, stateDirectory ); task.initializeStateStores(); @@ -587,9 +286,9 @@ public synchronized Map committed(final Set committed(final Set committed(final Set()).groupByKey().count(); - - initializeStandbyStores(builder); - } - - @Test - public void shouldInitializeWindowStoreWithoutException() throws IOException { - final InternalStreamsBuilder builder = new InternalStreamsBuilder(new InternalTopologyBuilder()); - builder.stream(Collections.singleton("topic"), - new ConsumedInternal<>()).groupByKey().windowedBy(TimeWindows.of(ofMillis(100))).count(); - - initializeStandbyStores(builder); - } - - private void initializeStandbyStores(final InternalStreamsBuilder builder) throws IOException { - final StreamsConfig config = createConfig(baseDir); - builder.buildAndOptimizeTopology(); - final InternalTopologyBuilder internalTopologyBuilder = InternalStreamsBuilderTest.internalTopologyBuilder(builder); - final ProcessorTopology topology = internalTopologyBuilder.setApplicationId(applicationId).build(0); - - task = new StandbyTask( - taskId, - emptySet(), - topology, - consumer, - changelogReader, - config, - new MockStreamsMetrics(new Metrics()), - stateDirectory - ); - - task.initializeStateStores(); - - assertTrue(task.hasStateStores()); - } - @Test public void shouldCheckpointStoreOffsetsOnCommit() throws IOException { - consumer.assign(Collections.singletonList(globalTopicPartition)); - final Map committedOffsets = new HashMap<>(); - committedOffsets.put(new TopicPartition(globalTopicPartition.topic(), globalTopicPartition.partition()), - new OffsetAndMetadata(100L)); - consumer.commitSync(committedOffsets); - - restoreStateConsumer.updatePartitions( - globalStoreName, - Collections.singletonList(new PartitionInfo(globalStoreName, 0, Node.noNode(), new Node[0], new Node[0])) - ); + stateManager.checkpoint(EasyMock.eq(Collections.emptyMap())); + EasyMock.replay(stateManager); final TaskId taskId = new TaskId(0, 0); - final MockTime time = new MockTime(); final StreamsConfig config = createConfig(baseDir); task = new StandbyTask( taskId, ktablePartitions, ktableTopology, consumer, - changelogReader, config, streamsMetrics, + stateManager, stateDirectory ); task.initializeStateStores(); - restoreStateConsumer.assign(new ArrayList<>(task.checkpointedOffsets().keySet())); - - final byte[] serializedValue = Serdes.Integer().serializer().serialize("", 1); - task.update( - globalTopicPartition, - singletonList(new ConsumerRecord<>(globalTopicPartition.topic(), - globalTopicPartition.partition(), - 50L, - serializedValue, - serializedValue)) - ); - - time.sleep(config.getLong(StreamsConfig.COMMIT_INTERVAL_MS_CONFIG)); task.commit(); - final Map checkpoint = new OffsetCheckpoint( - new File(stateDirectory.directoryForTask(taskId), StateManagerUtil.CHECKPOINT_FILE_NAME) - ).read(); - assertThat(checkpoint, equalTo(Collections.singletonMap(globalTopicPartition, 51L))); - + EasyMock.verify(stateManager); } @Test @@ -764,9 +398,9 @@ public void shouldCloseStateMangerOnTaskCloseWhenCommitFailed() throws Exception ktablePartitions, ktableTopology, consumer, - changelogReader, config, streamsMetrics, + stateManager, stateDirectory ) { @Override @@ -781,7 +415,7 @@ void closeStateManager(final boolean clean) throws ProcessorStateException { }; task.initializeStateStores(); try { - task.close(true, false); + task.close(true); fail("should have thrown exception"); } catch (final Exception e) { // expected @@ -813,15 +447,13 @@ public void shouldRecordTaskClosedMetricOnClose() throws IOException { ktablePartitions, ktableTopology, consumer, - changelogReader, createConfig(baseDir), streamsMetrics, + stateManager, stateDirectory ); - final boolean clean = true; - final boolean isZombie = false; - task.close(clean, isZombie); + task.close(true); final double expectedCloseTaskMetric = 1.0; verifyCloseTaskMetric(expectedCloseTaskMetric, streamsMetrics, metricName); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamTaskTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamTaskTest.java index e804b113c834e..b53146e7aa95c 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamTaskTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamTaskTest.java @@ -21,16 +21,10 @@ import org.apache.kafka.clients.consumer.MockConsumer; import org.apache.kafka.clients.consumer.OffsetAndMetadata; import org.apache.kafka.clients.consumer.OffsetResetStrategy; -import org.apache.kafka.clients.producer.MockProducer; -import org.apache.kafka.clients.producer.internals.DefaultPartitioner; -import org.apache.kafka.common.Cluster; import org.apache.kafka.common.KafkaException; import org.apache.kafka.common.MetricName; -import org.apache.kafka.common.PartitionInfo; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.errors.AuthorizationException; -import org.apache.kafka.common.errors.ProducerFencedException; -import org.apache.kafka.common.errors.TimeoutException; import org.apache.kafka.common.errors.WakeupException; import org.apache.kafka.common.metrics.JmxReporter; import org.apache.kafka.common.metrics.KafkaMetric; @@ -39,50 +33,40 @@ import org.apache.kafka.common.metrics.Sensor; import org.apache.kafka.common.record.TimestampType; import org.apache.kafka.common.serialization.Deserializer; -import org.apache.kafka.common.serialization.IntegerSerializer; import org.apache.kafka.common.serialization.Serdes; import org.apache.kafka.common.serialization.Serializer; -import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.common.utils.Utils; import org.apache.kafka.streams.StreamsConfig; -import org.apache.kafka.streams.errors.DefaultProductionExceptionHandler; import org.apache.kafka.streams.errors.ProcessorStateException; import org.apache.kafka.streams.errors.StreamsException; -import org.apache.kafka.streams.errors.TaskMigratedException; import org.apache.kafka.streams.processor.PunctuationType; import org.apache.kafka.streams.processor.Punctuator; -import org.apache.kafka.streams.processor.StateRestoreListener; import org.apache.kafka.streams.processor.StateStore; -import org.apache.kafka.streams.processor.StreamPartitioner; import org.apache.kafka.streams.processor.TaskId; -import org.apache.kafka.streams.processor.WallclockTimestampExtractor; import org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl; -import org.apache.kafka.streams.processor.internals.testutil.LogCaptureAppender; -import org.apache.kafka.streams.state.internals.OffsetCheckpoint; import org.apache.kafka.test.MockKeyValueStore; import org.apache.kafka.test.MockProcessorNode; import org.apache.kafka.test.MockSourceNode; -import org.apache.kafka.test.MockStateRestoreListener; import org.apache.kafka.test.MockTimestampExtractor; -import org.apache.kafka.test.MockRecordCollector; import org.apache.kafka.test.TestUtils; +import org.easymock.EasyMock; +import org.easymock.EasyMockRunner; +import org.easymock.Mock; +import org.easymock.MockType; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; -import java.time.Duration; import java.util.Base64; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -93,7 +77,6 @@ import static org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl.THREAD_ID_TAG; import static org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl.THREAD_ID_TAG_0100_TO_24; import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; @@ -103,12 +86,13 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +// TODO K9113: should improve test coverage +@RunWith(EasyMockRunner.class) public class StreamTaskTest { private static final File BASE_DIR = TestUtils.tempDirectory(); private final Serializer intSerializer = Serdes.Integer().serializer(); - private final Serializer bytesSerializer = Serdes.ByteArray().serializer(); private final Deserializer intDeserializer = Serdes.Integer().deserializer(); private final String topic1 = "topic1"; private final String topic2 = "topic2"; @@ -135,23 +119,8 @@ public void close() { private final String storeName = "store"; private final StateStore stateStore = new MockKeyValueStore(storeName, false); private final TopicPartition changelogPartition = new TopicPartition("store-changelog", 0); - private final Long offset = 543L; - - private final ProcessorTopology topology = withSources( - asList(source1, source2, processorStreamTime, processorSystemTime), - mkMap(mkEntry(topic1, source1), mkEntry(topic2, source2)) - ); private final MockConsumer consumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST); - private MockProducer producer; - private final MockConsumer restoreStateConsumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST); - private final StateRestoreListener stateRestoreListener = new MockStateRestoreListener(); - private final StoreChangelogReader changelogReader = new StoreChangelogReader(restoreStateConsumer, Duration.ZERO, stateRestoreListener, new LogContext("stream-task-test ")) { - @Override - public Map restoredOffsets() { - return Collections.singletonMap(changelogPartition, offset); - } - }; private final byte[] recordValue = intSerializer.serialize(null, 10); private final byte[] recordKey = intSerializer.serialize(null, 1); private final Metrics metrics = new Metrics(new MetricConfig().recordLevel(Sensor.RecordingLevel.DEBUG)); @@ -165,6 +134,12 @@ public Map restoredOffsets() { private static final String APPLICATION_ID = "stream-task-test"; private static final long DEFAULT_TIMESTAMP = 1000; + @Mock(type = MockType.NICE) + private ProcessorStateManager stateManager; + + @Mock(type = MockType.NICE) + private RecordCollector recordCollector; + private final Punctuator punctuator = new Punctuator() { @Override public void punctuate(final long timestamp) { @@ -196,7 +171,7 @@ private static ProcessorTopology withSources(final List processor } // Exposed to make it easier to create StreamTask config from other tests. - static StreamsConfig createConfig(final boolean enableEoS) { + private static StreamsConfig createConfig(final boolean enableEoS) { final String canonicalPath; try { canonicalPath = BASE_DIR.getCanonicalPath(); @@ -225,7 +200,7 @@ public void cleanup() throws IOException { try { if (task != null) { try { - task.close(true, false); + task.close(true); } catch (final Exception e) { // swallow } @@ -235,136 +210,6 @@ public void cleanup() throws IOException { } } - @Test - public void shouldHandleInitTransactionsTimeoutExceptionOnCreation() { - final LogCaptureAppender appender = LogCaptureAppender.createAndRegister(); - - final ProcessorTopology topology = withSources( - asList(source1, source2, processorStreamTime, processorSystemTime), - mkMap(mkEntry(topic1, source1), mkEntry(topic2, source2)) - ); - - source1.addChild(processorStreamTime); - source2.addChild(processorStreamTime); - source1.addChild(processorSystemTime); - source2.addChild(processorSystemTime); - - try { - new StreamTask( - taskId00, - partitions, - topology, - consumer, - changelogReader, - createConfig(true), - streamsMetrics, - stateDirectory, - null, - time, - () -> producer = new MockProducer(false, bytesSerializer, bytesSerializer) { - @Override - public void initTransactions() { - throw new TimeoutException("test"); - } - }, - null - ); - fail("Expected an exception"); - } catch (final StreamsException expected) { - // make sure we log the explanation as an ERROR - assertTimeoutErrorLog(appender); - - // make sure we report the correct message - assertThat( - expected.getMessage(), - is("stream-thread [" + Thread.currentThread().getName() + "] task [0_0] Failed to initialize task 0_0 due to timeout.")); - - // make sure we preserve the cause - assertEquals(expected.getCause().getClass(), TimeoutException.class); - assertThat(expected.getCause().getMessage(), is("test")); - } - LogCaptureAppender.unregister(appender); - } - - @Test - public void shouldHandleInitTransactionsTimeoutExceptionOnResume() { - final LogCaptureAppender appender = LogCaptureAppender.createAndRegister(); - - final ProcessorTopology topology = withSources( - asList(source1, source2, processorStreamTime, processorSystemTime), - mkMap(mkEntry(topic1, source1), mkEntry(topic2, source2)) - ); - - source1.addChild(processorStreamTime); - source2.addChild(processorStreamTime); - source1.addChild(processorSystemTime); - source2.addChild(processorSystemTime); - - final AtomicBoolean timeOut = new AtomicBoolean(false); - - final StreamTask testTask = new StreamTask( - taskId00, - partitions, - topology, - consumer, - changelogReader, - createConfig(true), - streamsMetrics, - stateDirectory, - null, - time, - () -> producer = new MockProducer(false, bytesSerializer, bytesSerializer) { - @Override - public void initTransactions() { - if (timeOut.get()) { - throw new TimeoutException("test"); - } else { - super.initTransactions(); - } - } - }, - null - ); - testTask.initializeTopology(); - testTask.suspend(); - timeOut.set(true); - try { - testTask.resume(); - fail("Expected an exception"); - } catch (final StreamsException expected) { - // make sure we log the explanation as an ERROR - assertTimeoutErrorLog(appender); - - // make sure we report the correct message - assertThat( - expected.getMessage(), - is("stream-thread [" + Thread.currentThread().getName() + "] task [0_0] Failed to initialize task 0_0 due to timeout.")); - - // make sure we preserve the cause - assertEquals(expected.getCause().getClass(), TimeoutException.class); - assertThat(expected.getCause().getMessage(), is("test")); - } - LogCaptureAppender.unregister(appender); - } - - private void assertTimeoutErrorLog(final LogCaptureAppender appender) { - - final String expectedErrorLogMessage = - "stream-thread [" + Thread.currentThread().getName() + "] task [0_0] Timeout exception caught when initializing transactions for task 0_0. " + - "This might happen if the broker is slow to respond, if the network " + - "connection to the broker was interrupted, or if similar circumstances arise. " + - "You can increase producer parameter `max.block.ms` to increase this timeout."; - - final List expectedError = - appender - .getEvents() - .stream() - .filter(event -> event.getMessage().equals(expectedErrorLogMessage)) - .map(LogCaptureAppender.Event::getLevel) - .collect(Collectors.toList()); - assertThat(expectedError, is(singletonList("ERROR"))); - } - @Test public void testProcessOrder() { task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); @@ -775,44 +620,6 @@ public void shouldRespectCommitNeeded() { assertFalse(task.commitNeeded()); } - @Test - public void shouldRestorePartitionTimeAfterRestartWithEosDisabled() { - createTaskWithProcessAndCommit(false); - - assertEquals(DEFAULT_TIMESTAMP, task.decodeTimestamp(consumer.committed(Collections.singleton(partition1)).get(partition1).metadata())); - // reset times here by creating a new task - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - - task.initializeMetadata(); - assertEquals(DEFAULT_TIMESTAMP, task.partitionTime(partition1)); - assertEquals(DEFAULT_TIMESTAMP, task.streamTime()); - } - - @Test - public void shouldRestorePartitionTimeAfterRestartWithEosEnabled() { - createTaskWithProcessAndCommit(true); - - moveCommittedOffsetsFromProducerToConsumer(DEFAULT_TIMESTAMP); - - // reset times here by creating a new task - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - - task.initializeMetadata(); - assertEquals(DEFAULT_TIMESTAMP, task.partitionTime(partition1)); - assertEquals(DEFAULT_TIMESTAMP, task.streamTime()); - } - - private void createTaskWithProcessAndCommit(final boolean eosEnabled) { - task = createStatelessTask(createConfig(eosEnabled), StreamsConfig.METRICS_LATEST); - task.initializeStateStores(); - task.initializeTopology(); - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, DEFAULT_TIMESTAMP))); - - task.process(); - task.commit(); - } - @Test public void shouldEncodeAndDecodeMetadata() { task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); @@ -1016,21 +823,6 @@ public void shouldPunctuateOnceSystemTimeAfterGap() { processorSystemTime.mockProcessor.checkAndClearPunctuateResult(PunctuationType.WALL_CLOCK_TIME, now + 100, now + 110, now + 122, now + 130, now + 235, now + 240); } - @Test - public void shouldWrapKafkaExceptionsWithStreamsExceptionAndAddContext() { - task = createTaskThatThrowsException(false); - task.initializeStateStores(); - task.initializeTopology(); - task.addRecords(partition2, singletonList(getConsumerRecord(partition2, 0))); - - try { - task.process(); - fail("Should've thrown StreamsException"); - } catch (final Exception e) { - assertThat(task.processorContext.currentNode(), nullValue()); - } - } - @Test public void shouldWrapKafkaExceptionsWithStreamsExceptionAndAddContextWhenPunctuatingStreamTime() { task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); @@ -1068,42 +860,29 @@ public void shouldWrapKafkaExceptionsWithStreamsExceptionAndAddContextWhenPunctu } @Test - public void shouldFlushRecordCollectorOnFlushState() { - final StreamsMetricsImpl streamsMetrics = new MockStreamsMetrics(new Metrics()); - final MockRecordCollector collector = new MockRecordCollector(); - final StreamTask streamTask = new StreamTask( - taskId00, - partitions, - topology, - consumer, - changelogReader, - createConfig(false), - streamsMetrics, - stateDirectory, - null, - time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer), - collector); - streamTask.flushState(); - assertTrue(collector.flushed()); - } + public void shouldCheckpointOffsetsOnCommit() { + final Long offset = 543L; + + EasyMock.expect(recordCollector.offsets()).andReturn(Collections.singletonMap(changelogPartition, offset)); + stateManager.checkpoint(EasyMock.eq(Collections.singletonMap(changelogPartition, offset))); + EasyMock.expectLastCall(); - @Test - public void shouldCheckpointOffsetsOnCommit() throws IOException { task = createStatefulTask(createConfig(false), true); + EasyMock.replay(stateManager, recordCollector); + + task.initializeStateStores(); task.initializeTopology(); task.commit(); - final OffsetCheckpoint checkpoint = new OffsetCheckpoint( - new File(stateDirectory.directoryForTask(taskId00), StateManagerUtil.CHECKPOINT_FILE_NAME) - ); - assertThat(checkpoint.read(), equalTo(Collections.singletonMap(changelogPartition, offset))); + EasyMock.verify(recordCollector); } @Test public void shouldNotCheckpointOffsetsOnCommitIfEosIsEnabled() { task = createStatefulTask(createConfig(true), true); + EasyMock.replay(stateManager, recordCollector); + task.initializeStateStores(); task.initializeTopology(); task.commit(); @@ -1163,677 +942,45 @@ public void shouldNotThrowExceptionOnScheduleIfCurrentNodeIsNotNull() { } @Test - public void shouldNotCloseProducerOnCleanCloseWithEosDisabled() { - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - task.close(true, false); - task = null; - - assertFalse(producer.closed()); - } - - @Test - public void shouldNotCloseProducerOnUncleanCloseWithEosDisabled() { - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - task.close(false, false); - task = null; - - assertFalse(producer.closed()); - } - - @Test - public void shouldNotCloseProducerOnErrorDuringCleanCloseWithEosDisabled() { - task = createTaskThatThrowsException(false); - - try { - task.close(true, false); - fail("should have thrown runtime exception"); - } catch (final RuntimeException expected) { - task = null; - } - - assertFalse(producer.closed()); - } - - @Test - public void shouldNotCloseProducerOnErrorDuringUncleanCloseWithEosDisabled() { - task = createTaskThatThrowsException(false); - - task.close(false, false); - task = null; - - assertFalse(producer.closed()); - } - - @Test - public void shouldCommitTransactionAndCloseProducerOnCleanCloseWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - task.close(true, false); - task = null; - - assertTrue(producer.transactionCommitted()); - assertFalse(producer.transactionInFlight()); - assertTrue(producer.closed()); - } - - @Test - public void shouldNotAbortTransactionAndNotCloseProducerOnErrorDuringCleanCloseWithEosEnabled() { - task = createTaskThatThrowsException(true); - task.initializeTopology(); - - try { - task.close(true, false); - fail("should have thrown runtime exception"); - } catch (final RuntimeException expected) { - task = null; - } - - assertTrue(producer.transactionInFlight()); - assertFalse(producer.closed()); - } - - @Test - public void shouldOnlyCloseProducerIfFencedOnCommitDuringCleanCloseWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - producer.fenceProducer(); - - try { - task.close(true, false); - fail("should have thrown TaskMigratedException"); - } catch (final TaskMigratedException expected) { - task = null; - assertTrue(expected.getCause() instanceof RecoverableClientException); - } - - assertFalse(producer.transactionCommitted()); - assertTrue(producer.transactionInFlight()); - assertFalse(producer.transactionAborted()); - assertFalse(producer.transactionCommitted()); - assertTrue(producer.closed()); - } - - @Test - public void shouldNotCloseProducerIfFencedOnCloseDuringCleanCloseWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - producer.fenceProducerOnClose(); - - try { - task.close(true, false); - fail("should have thrown TaskMigratedException"); - } catch (final TaskMigratedException expected) { - task = null; - assertTrue(expected.getCause() instanceof RecoverableClientException); - } - - assertTrue(producer.transactionCommitted()); - assertFalse(producer.transactionInFlight()); - assertFalse(producer.closed()); - } - - @Test - public void shouldAbortTransactionAndCloseProducerOnUncleanCloseWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - task.close(false, false); - task = null; - - assertTrue(producer.transactionAborted()); - assertFalse(producer.transactionInFlight()); - assertTrue(producer.closed()); - } - - @Test - public void shouldAbortTransactionAndCloseProducerOnErrorDuringUncleanCloseWithEosEnabled() { - task = createTaskThatThrowsException(true); - task.initializeTopology(); - - task.close(false, false); - - assertTrue(producer.transactionAborted()); - assertTrue(producer.closed()); - } - - @Test - public void shouldOnlyCloseProducerIfFencedOnAbortDuringUncleanCloseWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - producer.fenceProducer(); - - task.close(false, false); - task = null; - - assertTrue(producer.transactionInFlight()); - assertFalse(producer.transactionAborted()); - assertFalse(producer.transactionCommitted()); - assertTrue(producer.closed()); - } - - @Test - public void shouldMigrateTaskIfFencedDuringFlush() { - final StateStore stateStore = new MockKeyValueStore(storeName, true, true); - - final Map storeToChangelogTopic = true ? Collections.singletonMap(storeName, storeName + "-changelog") : Collections.emptyMap(); - final ProcessorTopology topology1 = new ProcessorTopology( - asList(source1, source2), - mkMap(mkEntry(topic1, source1), mkEntry(topic2, source2)), - Collections.emptyMap(), - singletonList(stateStore), - Collections.emptyList(), - storeToChangelogTopic, - Collections.emptySet() - ); - - task = new StreamTask( - taskId00, - partitions, - topology1, - consumer, - changelogReader, - createConfig(true), - streamsMetrics, - stateDirectory, - null, - time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer) - ); - task.initializeStateStores(); - task.initializeTopology(); - producer.fenceProducer(); - - try { - task.flushState(); - fail("Expected a TaskMigratedException"); - } catch (final TaskMigratedException expected) { - assertThat(expected.migratedTask(), is(task)); - } - } - - @Test - public void shouldMigrateTaskIfFencedDuringProcess() { - final StateStore stateStore = new MockKeyValueStore(storeName, true, true); - - final Map storeToChangelogTopic = true ? Collections.singletonMap(storeName, storeName + "-changelog") : Collections.emptyMap(); - final SourceNode sourceNode = new SourceNode<>("test", singletonList(topic1), new WallclockTimestampExtractor(), intDeserializer, intDeserializer); - final SinkNode sinkNode = new SinkNode<>("test-sink", new StaticTopicNameExtractor<>("out-topic"), intSerializer, intSerializer, new StreamPartitioner() { - @Override - public Integer partition(final String topic, - final Integer key, - final Integer value, - final int numPartitions) { - return 1; - } - }); - - sourceNode.addChild(sinkNode); - - final ProcessorTopology topology1 = new ProcessorTopology( - asList(sourceNode, sinkNode), - mkMap(mkEntry(topic1, sourceNode), mkEntry(topic2, sourceNode)), - mkMap(mkEntry("out-topic", sinkNode)), + public void shouldCloseStateManagerEvenFailureOnUncleanTaskClose() { + final ProcessorTopology topology = ProcessorTopologyFactories.with( + asList(source1, source3), + mkMap(mkEntry(topic1, source1), mkEntry(topic2, source3)), singletonList(stateStore), - Collections.emptyList(), - storeToChangelogTopic, - Collections.emptySet() - ); - - task = new StreamTask( - taskId00, - partitions, - topology1, - consumer, - changelogReader, - createConfig(true), - streamsMetrics, - stateDirectory, - null, - time, - () -> producer = new MockProducer( - Cluster.empty(), - false, - new DefaultPartitioner(), - bytesSerializer, - bytesSerializer - ) { - @Override - public List partitionsFor(final String topic) { - return singletonList(null); - } - } - ); - task.initializeStateStores(); - task.initializeTopology(); - producer.fenceProducer(); - - try { - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - fail("Expected a TaskMigratedException"); - } catch (final TaskMigratedException expected) { - assertThat(expected.migratedTask(), is(task)); - } - } - - @Test - public void shouldMigrateTaskIfFencedDuringPunctuate() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - - final RecordCollectorImpl recordCollector = new RecordCollectorImpl("StreamTask", - new LogContext("StreamTaskTest "), new DefaultProductionExceptionHandler(), new Metrics().sensor("skipped-records")); - recordCollector.init(producer); - - task.initializeStateStores(); - task.initializeTopology(); - producer.fenceProducer(); - try { - task.punctuate( - processorSystemTime, - 5, - PunctuationType.WALL_CLOCK_TIME, - timestamp -> recordCollector.send( - "result-topic1", - 3, - 5, - null, - 0, - time.milliseconds(), - new IntegerSerializer(), - new IntegerSerializer() - ) - ); - fail("Expected a TaskMigratedException"); - } catch (final TaskMigratedException expected) { - assertThat(expected.migratedTask(), is(task)); - } - } - - @Test - public void shouldMigrateTaskIfFencedDuringCommit() { - final ProcessorTopology topology1 = withSources( - asList(source1, source2, processorStreamTime, processorSystemTime), - mkMap(mkEntry(topic1, source1), mkEntry(topic2, source2)) - ); + Collections.emptyMap()); - source1.addChild(processorStreamTime); - source2.addChild(processorStreamTime); - source1.addChild(processorSystemTime); - source2.addChild(processorSystemTime); + EasyMock.expect(stateManager.changelogPartitions()).andReturn(Collections.emptySet()); + stateManager.registerGlobalStateStores(EasyMock.eq(Collections.emptyList())); + EasyMock.expectLastCall(); + stateManager.close(EasyMock.eq(false)); + EasyMock.expectLastCall(); + EasyMock.replay(stateManager); task = new StreamTask( taskId00, partitions, - topology1, + topology, consumer, - changelogReader, createConfig(true), streamsMetrics, stateDirectory, null, time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer)) { - @Override - protected void flushState() { - // do nothing so that we actually make it to the commit interaction. - } - }; - - final RecordCollectorImpl recordCollector = new RecordCollectorImpl("StreamTask", - new LogContext("StreamTaskTest "), new DefaultProductionExceptionHandler(), new Metrics().sensor("skipped-records")); - recordCollector.init(producer); + stateManager, + recordCollector); task.initializeStateStores(); task.initializeTopology(); - producer.fenceProducer(); - try { - task.commit(); - fail("Expected a TaskMigratedException"); - } catch (final TaskMigratedException expected) { - assertThat(expected.migratedTask(), is(task)); - } - } - - @Test - public void shouldOnlyCloseFencedProducerOnUncleanClosedWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - producer.fenceProducer(); - - task.close(false, true); - task = null; - - assertFalse(producer.transactionAborted()); - assertTrue(producer.closed()); - } - - @Test - public void shouldAbortTransactionButNotCloseProducerIfFencedOnCloseDuringUncleanCloseWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - producer.fenceProducerOnClose(); - - task.close(false, false); - task = null; - - assertTrue(producer.transactionAborted()); - assertFalse(producer.closed()); - } - - @Test - public void shouldThrowExceptionIfAnyExceptionsRaisedDuringCloseButStillCloseAllProcessorNodesTopology() { - task = createTaskThatThrowsException(false); - task.initializeStateStores(); - task.initializeTopology(); - try { - task.close(true, false); - fail("should have thrown runtime exception"); - } catch (final RuntimeException expected) { - task = null; - } - assertTrue(processorSystemTime.closed); - assertTrue(processorStreamTime.closed); - assertTrue(source1.closed); - } - - @Test - public void shouldInitAndBeginTransactionOnCreateIfEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - assertTrue(producer.transactionInitialized()); - assertTrue(producer.transactionInFlight()); - } - - @Test - public void shouldWrapProducerFencedExceptionWithTaskMigratedExceptionForBeginTransaction() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - producer.fenceProducer(); - - try { - task.initializeTopology(); - fail("Should have throws TaskMigratedException"); - } catch (final TaskMigratedException expected) { - assertTrue(expected.getCause() instanceof ProducerFencedException); - } - } - - @Test - public void shouldNotThrowOnCloseIfTaskWasNotInitializedWithEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - - assertFalse(producer.transactionInFlight()); - task.close(false, false); - } - - @Test - public void shouldNotInitOrBeginTransactionOnCreateIfEosDisabled() { - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - - assertFalse(producer.transactionInitialized()); - assertFalse(producer.transactionInFlight()); - } - - @Test - public void shouldSendOffsetsAndCommitTransactionButNotStartNewTransactionOnSuspendIfEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - task.suspend(); - assertTrue(producer.sentOffsets()); - assertTrue(producer.transactionCommitted()); - assertFalse(producer.transactionInFlight()); - } - - @Test - public void shouldCommitTransactionOnSuspendEvenIfTransactionIsEmptyIfEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - task.suspend(); - - assertTrue(producer.transactionCommitted()); - assertFalse(producer.transactionInFlight()); - } - - @Test - public void shouldNotSendOffsetsAndCommitTransactionNorStartNewTransactionOnSuspendIfEosDisabled() { - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - task.suspend(); + task.close(false); - assertFalse(producer.sentOffsets()); - assertFalse(producer.transactionCommitted()); - assertFalse(producer.transactionInFlight()); - } - - @Test - public void shouldWrapProducerFencedExceptionWithTaskMigratedExceptionInSuspendWhenCommitting() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - producer.fenceProducer(); - - try { - task.suspend(); - fail("Should have throws TaskMigratedException"); - } catch (final TaskMigratedException expected) { - assertTrue(expected.getCause() instanceof RecoverableClientException); - } - task = null; - - assertFalse(producer.transactionCommitted()); - } - - @Test - public void shouldWrapProducerFencedExceptionWithTaskMigragedExceptionInSuspendWhenClosingProducer() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - producer.fenceProducerOnClose(); - try { - task.suspend(); - fail("Should have throws TaskMigratedException"); - } catch (final TaskMigratedException expected) { - assertTrue(expected.getCause() instanceof RecoverableClientException); - } - - assertTrue(producer.transactionCommitted()); - } - - @Test - public void shouldInitTaskTimeOnResumeWithEosDisabled() { - shouldInitTaskTimeOnResume(false); - } - - @Test - public void shouldInitTaskTimeOnResumeWithEosEnabled() { - shouldInitTaskTimeOnResume(true); - } - - private void shouldInitTaskTimeOnResume(final boolean eosEnabled) { - task = createStatelessTask(createConfig(eosEnabled), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - assertThat(task.partitionTime(partition1), is(RecordQueue.UNKNOWN)); - assertThat(task.streamTime(), is(RecordQueue.UNKNOWN)); - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - assertThat(task.partitionTime(partition1), is(0L)); - assertThat(task.streamTime(), is(0L)); - - task.suspend(); - assertThat(task.partitionTime(partition1), is(RecordQueue.UNKNOWN)); - assertThat(task.streamTime(), is(RecordQueue.UNKNOWN)); - - if (eosEnabled) { - moveCommittedOffsetsFromProducerToConsumer(0L); - } - - task.resume(); - assertThat(task.partitionTime(partition1), is(0L)); - assertThat(task.streamTime(), is(0L)); - } - - @Test - public void shouldStartNewTransactionOnResumeIfEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - task.suspend(); - - task.resume(); - task.initializeTopology(); - assertTrue(producer.transactionInFlight()); - } - - @Test - public void shouldNotStartNewTransactionOnResumeIfEosDisabled() { - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - task.suspend(); - - task.resume(); - assertFalse(producer.transactionInFlight()); - } - - @Test - public void shouldStartNewTransactionOnCommitIfEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - - task.commit(); - assertTrue(producer.transactionInFlight()); - } - - @Test - public void shouldNotStartNewTransactionOnCommitIfEosDisabled() { - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - - task.addRecords(partition1, singletonList(getConsumerRecord(partition1, 0))); - task.process(); - - task.commit(); - assertFalse(producer.transactionInFlight()); - } - - @Test - public void shouldNotAbortTransactionOnZombieClosedIfEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.close(false, true); - task = null; - - assertFalse(producer.transactionAborted()); - } - - @Test - public void shouldNotAbortTransactionOnDirtyClosedIfEosDisabled() { - task = createStatelessTask(createConfig(false), StreamsConfig.METRICS_LATEST); - task.close(false, false); - task = null; - - assertFalse(producer.transactionAborted()); - } - - @Test - public void shouldCloseProducerOnCloseWhenEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - task.close(true, false); - task = null; - - assertTrue(producer.closed()); - } - - @Test - public void shouldCloseProducerOnUncleanCloseNotZombieWhenEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - task.close(false, false); - task = null; - - assertTrue(producer.closed()); - } - - @Test - public void shouldCloseProducerOnUncleanCloseIsZombieWhenEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - task.initializeTopology(); - task.close(false, true); - task = null; - - assertTrue(producer.closed()); - } - - @Test - public void shouldNotViolateAtLeastOnceWhenExceptionOccursDuringFlushing() { - task = createTaskThatThrowsException(false); - task.initializeStateStores(); - task.initializeTopology(); - - try { - task.commit(); - fail("should have thrown an exception"); - } catch (final Exception e) { - // all good - } - } - - @Test - public void shouldNotViolateAtLeastOnceWhenExceptionOccursDuringTaskSuspension() { - final StreamTask task = createTaskThatThrowsException(false); - - task.initializeStateStores(); - task.initializeTopology(); - try { - task.suspend(); - fail("should have thrown an exception"); - } catch (final Exception e) { - // all good - } - } - - @Test - public void shouldCloseStateManagerIfFailureOnTaskClose() { - task = createStatefulTaskThatThrowsExceptionOnClose(); - task.initializeStateStores(); - task.initializeTopology(); - - try { - task.close(true, false); - fail("should have thrown an exception"); - } catch (final Exception e) { - // all good - } - - task = null; - assertFalse(stateStore.isOpen()); - } - - @Test - public void shouldNotCloseTopologyProcessorNodesIfNotInitialized() { - final StreamTask task = createTaskThatThrowsException(false); - try { - task.close(false, false); - } catch (final Exception e) { - fail("should have not closed non-initialized topology"); - } + EasyMock.verify(stateManager); } @Test public void shouldBeInitializedIfChangelogPartitionsIsEmpty() { final StreamTask task = createStatefulTask(createConfig(false), false); + EasyMock.replay(stateManager, recordCollector); assertTrue(task.initializeStateStores()); } @@ -1841,6 +988,7 @@ public void shouldBeInitializedIfChangelogPartitionsIsEmpty() { @Test public void shouldNotBeInitializedIfChangelogPartitionsIsNonEmpty() { final StreamTask task = createStatefulTask(createConfig(false), true); + EasyMock.replay(stateManager, recordCollector); assertFalse(task.initializeStateStores()); } @@ -1856,18 +1004,22 @@ public void shouldReturnOffsetsForRepartitionTopicsForPurging() { ); consumer.assign(asList(partition1, repartition)); + EasyMock.expect(stateManager.changelogPartitions()).andReturn(Collections.emptySet()); + EasyMock.expect(recordCollector.offsets()).andReturn(Collections.emptyMap()).anyTimes(); + EasyMock.replay(stateManager, recordCollector); + task = new StreamTask( taskId00, mkSet(partition1, repartition), topology, consumer, - changelogReader, createConfig(false), streamsMetrics, stateDirectory, null, time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer)); + stateManager, + recordCollector); task.initializeStateStores(); task.initializeTopology(); @@ -1884,44 +1036,12 @@ public void shouldReturnOffsetsForRepartitionTopicsForPurging() { assertThat(map, equalTo(Collections.singletonMap(repartition, 11L))); } - @Test - public void shouldThrowOnCleanCloseTaskWhenEosEnabledIfTransactionInFlight() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - try { - task.close(true, false); - fail("should have throw IllegalStateException"); - } catch (final IllegalStateException expected) { - // pass - } - task = null; - - assertTrue(producer.closed()); - } - - @Test - public void shouldAlwaysCommitIfEosEnabled() { - task = createStatelessTask(createConfig(true), StreamsConfig.METRICS_LATEST); - - final RecordCollectorImpl recordCollector = new RecordCollectorImpl( - "StreamTask", - new LogContext("StreamTaskTest "), - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") - ); - recordCollector.init(producer); - - task.initializeStateStores(); - task.initializeTopology(); - task.punctuate(processorSystemTime, 5, PunctuationType.WALL_CLOCK_TIME, timestamp -> recordCollector.send("result-topic1", 3, 5, null, 0, time.milliseconds(), - new IntegerSerializer(), new IntegerSerializer())); - task.commit(); - assertEquals(1, producer.history().size()); - } - @Test(expected = ProcessorStateException.class) public void shouldThrowProcessorStateExceptionOnInitializeOffsetsWhenAuthorizationException() { final Consumer consumer = mockConsumerWithCommittedException(new AuthorizationException("message")); final StreamTask task = createOptimizedStatefulTask(createConfig(false), consumer); + EasyMock.replay(stateManager); + task.initializeMetadata(); task.initializeStateStores(); } @@ -1930,6 +1050,8 @@ public void shouldThrowProcessorStateExceptionOnInitializeOffsetsWhenAuthorizati public void shouldThrowProcessorStateExceptionOnInitializeOffsetsWhenKafkaException() { final Consumer consumer = mockConsumerWithCommittedException(new KafkaException("message")); final AbstractTask task = createOptimizedStatefulTask(createConfig(false), consumer); + EasyMock.replay(stateManager); + task.initializeMetadata(); task.initializeStateStores(); } @@ -1938,6 +1060,8 @@ public void shouldThrowProcessorStateExceptionOnInitializeOffsetsWhenKafkaExcept public void shouldThrowWakeupExceptionOnInitializeOffsetsWhenWakeupException() { final Consumer consumer = mockConsumerWithCommittedException(new WakeupException()); final AbstractTask task = createOptimizedStatefulTask(createConfig(false), consumer); + EasyMock.replay(stateManager); + task.initializeMetadata(); task.initializeStateStores(); } @@ -1951,22 +1075,6 @@ public Map committed(final Set>> metadataList = - producer.consumerGroupOffsetsHistory(); - final String storedMetadata = metadataList.get(0).get(APPLICATION_ID).get(partition1).metadata(); - final long partitionTime = task.decodeTimestamp(storedMetadata); - assertThat(partitionTime, is(expectedPartitionTime)); - - // since producer and consumer is mocked, we need to "connect" producer and consumer - // so we should manually commit offsets here to simulate this "connection" - final Map offsetMap = new HashMap<>(); - final String encryptedMetadata = task.encodeTimestamp(partitionTime); - offsetMap.put(partition1, new OffsetAndMetadata(partitionTime, encryptedMetadata)); - consumer.commitSync(offsetMap); - } - private StreamTask createOptimizedStatefulTask(final StreamsConfig config, final Consumer consumer) { final StateStore stateStore = new MockKeyValueStore(storeName, true); @@ -1976,18 +1084,20 @@ private StreamTask createOptimizedStatefulTask(final StreamsConfig config, final singletonList(stateStore), Collections.singletonMap(storeName, topic1)); + EasyMock.expect(stateManager.changelogPartitions()).andReturn(Collections.singleton(partition1)); + return new StreamTask( taskId00, mkSet(partition1), topology, consumer, - changelogReader, config, streamsMetrics, stateDirectory, null, time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer)); + stateManager, + recordCollector); } private StreamTask createStatefulTask(final StreamsConfig config, final boolean logged) { @@ -1999,39 +1109,21 @@ private StreamTask createStatefulTask(final StreamsConfig config, final boolean singletonList(stateStore), logged ? Collections.singletonMap(storeName, storeName + "-changelog") : Collections.emptyMap()); - return new StreamTask( - taskId00, - partitions, - topology, - consumer, - changelogReader, - config, - streamsMetrics, - stateDirectory, - null, - time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer)); - } - - private StreamTask createStatefulTaskThatThrowsExceptionOnClose() { - final ProcessorTopology topology = ProcessorTopologyFactories.with( - asList(source1, source3), - mkMap(mkEntry(topic1, source1), mkEntry(topic2, source3)), - singletonList(stateStore), - Collections.emptyMap()); + EasyMock.expect(stateManager.changelogPartitions()).andReturn( + logged ? Collections.singleton(new TopicPartition(storeName + "-changelog", 1)) : Collections.emptySet()); return new StreamTask( taskId00, partitions, topology, consumer, - changelogReader, - createConfig(true), + config, streamsMetrics, stateDirectory, null, time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer)); + stateManager, + recordCollector); } private StreamTask createStatelessTask(final StreamsConfig streamsConfig, @@ -2046,49 +1138,22 @@ private StreamTask createStatelessTask(final StreamsConfig streamsConfig, source1.addChild(processorSystemTime); source2.addChild(processorSystemTime); + EasyMock.expect(stateManager.changelogPartitions()).andReturn(Collections.emptySet()); + EasyMock.expect(recordCollector.offsets()).andReturn(Collections.emptyMap()).anyTimes(); + EasyMock.replay(stateManager, recordCollector); + return new StreamTask( taskId00, partitions, topology, consumer, - changelogReader, streamsConfig, new StreamsMetricsImpl(metrics, "test", builtInMetricsVersion), stateDirectory, null, time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer)); - } - - // this task will throw exception when processing (on partition2), flushing, suspending and closing - private StreamTask createTaskThatThrowsException(final boolean enableEos) { - final ProcessorTopology topology = withSources( - asList(source1, source3, processorStreamTime, processorSystemTime), - mkMap(mkEntry(topic1, source1), mkEntry(topic2, source3)) - ); - - source1.addChild(processorStreamTime); - source3.addChild(processorStreamTime); - source1.addChild(processorSystemTime); - source3.addChild(processorSystemTime); - - return new StreamTask( - taskId00, - partitions, - topology, - consumer, - changelogReader, - createConfig(enableEos), - streamsMetrics, - stateDirectory, - null, - time, - () -> producer = new MockProducer<>(false, bytesSerializer, bytesSerializer)) { - @Override - protected void flushState() { - throw new RuntimeException("KABOOM!"); - } - }; + stateManager, + recordCollector); } private ConsumerRecord getConsumerRecord(final TopicPartition topicPartition, final long offset) { diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java index d8f5d5564f4d3..001a6f680ac48 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamThreadTest.java @@ -27,6 +27,7 @@ import org.apache.kafka.clients.producer.MockProducer; import org.apache.kafka.clients.producer.Producer; import org.apache.kafka.common.Cluster; +import org.apache.kafka.common.KafkaException; import org.apache.kafka.common.Metric; import org.apache.kafka.common.MetricName; import org.apache.kafka.common.Node; @@ -954,7 +955,6 @@ public void shouldNotCloseTaskAsZombieAndRemoveFromActiveTasksIfProducerWasFence thread.runOnce(); assertThat(producer.history().size(), equalTo(1)); - assertFalse(producer.transactionCommitted()); mockTime.sleep(config.getLong(StreamsConfig.COMMIT_INTERVAL_MS_CONFIG) + 1L); TestUtils.waitForCondition( () -> producer.commitCount() == 1, @@ -966,7 +966,8 @@ public void shouldNotCloseTaskAsZombieAndRemoveFromActiveTasksIfProducerWasFence try { thread.runOnce(); fail("Should have thrown TaskMigratedException"); - } catch (final TaskMigratedException expected) { + } catch (final KafkaException expected) { + assertTrue(expected.getCause() instanceof TaskMigratedException); assertTrue("StreamsThread removed the fenced zombie task already, should wait for rebalance to close all zombies together.", thread.tasks().containsKey(task1)); } @@ -1007,7 +1008,6 @@ public void shouldCloseTaskAsZombieAndRemoveFromActiveTasksIfProducerGotFencedIn clientSupplier.producers.get(0).fenceProducer(); thread.rebalanceListener.onPartitionsRevoked(assignedPartitions); - assertTrue(clientSupplier.producers.get(0).transactionInFlight()); assertFalse(clientSupplier.producers.get(0).transactionCommitted()); assertTrue(clientSupplier.producers.get(0).closed()); assertTrue(thread.tasks().isEmpty()); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/TaskManagerTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/TaskManagerTest.java index 8e4b2c242166d..9d64593701f39 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/TaskManagerTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/TaskManagerTest.java @@ -85,9 +85,9 @@ public class TaskManagerTest { @Mock(type = MockType.NICE) private Consumer consumer; @Mock(type = MockType.NICE) - private StreamThread.AbstractTaskCreator activeTaskCreator; + private StreamThread.TaskCreator activeTaskCreator; @Mock(type = MockType.NICE) - private StreamThread.AbstractTaskCreator standbyTaskCreator; + private StreamThread.StandbyTaskCreator standbyTaskCreator; @Mock(type = MockType.NICE) private Admin adminClient; @Mock(type = MockType.NICE) diff --git a/streams/src/test/java/org/apache/kafka/streams/state/KeyValueStoreTestDriver.java b/streams/src/test/java/org/apache/kafka/streams/state/KeyValueStoreTestDriver.java index 0a1d98716bdff..587e944285ab1 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/KeyValueStoreTestDriver.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/KeyValueStoreTestDriver.java @@ -16,6 +16,9 @@ */ package org.apache.kafka.streams.state; +import org.apache.kafka.clients.consumer.Consumer; +import org.apache.kafka.clients.consumer.MockConsumer; +import org.apache.kafka.clients.consumer.OffsetResetStrategy; import org.apache.kafka.clients.producer.MockProducer; import org.apache.kafka.clients.producer.Producer; import org.apache.kafka.common.header.Headers; @@ -27,11 +30,12 @@ import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.streams.KeyValue; import org.apache.kafka.streams.StreamsConfig; -import org.apache.kafka.streams.errors.DefaultProductionExceptionHandler; import org.apache.kafka.streams.processor.ProcessorContext; import org.apache.kafka.streams.processor.StateRestoreCallback; import org.apache.kafka.streams.processor.StateStore; import org.apache.kafka.streams.processor.StreamPartitioner; +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.MockStreamsMetrics; import org.apache.kafka.streams.processor.internals.RecordCollector; import org.apache.kafka.streams.processor.internals.RecordCollectorImpl; import org.apache.kafka.streams.state.internals.MeteredKeyValueStore; @@ -39,6 +43,7 @@ import org.apache.kafka.streams.state.internals.ThreadCache; import org.apache.kafka.test.InternalMockProcessorContext; import org.apache.kafka.test.MockTimestampExtractor; +import org.apache.kafka.test.StreamsTestUtils; import org.apache.kafka.test.TestUtils; import java.io.File; @@ -188,12 +193,15 @@ public static KeyValueStoreTestDriver create(final Serializer ke private KeyValueStoreTestDriver(final StateSerdes serdes) { final ByteArraySerializer rawSerializer = new ByteArraySerializer(); final Producer producer = new MockProducer<>(true, rawSerializer, rawSerializer); + final Consumer consumer = new MockConsumer<>(OffsetResetStrategy.EARLIEST); final RecordCollector recordCollector = new RecordCollectorImpl( - "KeyValueStoreTestDriver", + new TaskId(0, 0), + new StreamsConfig(StreamsTestUtils.getStreamsConfig("test")), new LogContext("KeyValueStoreTestDriver "), - new DefaultProductionExceptionHandler(), - new Metrics().sensor("dropped-records") + new MockStreamsMetrics(new Metrics()), + consumer, + id -> producer ) { @Override public void send(final String topic, @@ -224,7 +232,6 @@ public void send(final String topic, throw new UnsupportedOperationException(); } }; - recordCollector.init(producer); final File stateDir = TestUtils.tempDirectory(); //noinspection ResultOfMethodCallIgnored diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/StreamThreadStateStoreProviderTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/StreamThreadStateStoreProviderTest.java index ce816195fb9db..e1d9c23691305 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/StreamThreadStateStoreProviderTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/StreamThreadStateStoreProviderTest.java @@ -29,7 +29,10 @@ import org.apache.kafka.streams.errors.InvalidStateStoreException; import org.apache.kafka.streams.processor.TaskId; import org.apache.kafka.streams.processor.internals.MockStreamsMetrics; +import org.apache.kafka.streams.processor.internals.ProcessorStateManager; import org.apache.kafka.streams.processor.internals.ProcessorTopology; +import org.apache.kafka.streams.processor.internals.RecordCollector; +import org.apache.kafka.streams.processor.internals.RecordCollectorImpl; import org.apache.kafka.streams.processor.internals.StateDirectory; import org.apache.kafka.streams.processor.internals.StoreChangelogReader; import org.apache.kafka.streams.processor.internals.StreamTask; @@ -59,6 +62,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import static org.apache.kafka.streams.state.QueryableStoreTypes.timestampedWindowStore; import static org.apache.kafka.streams.state.QueryableStoreTypes.windowStore; @@ -302,22 +306,39 @@ private StreamTask createStreamsTask(final StreamsConfig streamsConfig, final ProcessorTopology topology, final TaskId taskId) { final Metrics metrics = new Metrics(); - return new StreamTask( + final LogContext logContext = new LogContext("test-stream-task "); + final Set partitions = Collections.singleton(new TopicPartition(topicName, taskId.partition)); + final ProcessorStateManager stateManager = new ProcessorStateManager( taskId, - Collections.singleton(new TopicPartition(topicName, taskId.partition)), - topology, - clientSupplier.consumer, + partitions, + false, + stateDirectory, + topology.storeToChangelogTopic(), new StoreChangelogReader( clientSupplier.restoreConsumer, Duration.ZERO, new MockStateRestoreListener(), new LogContext("test-stream-task ")), + logContext); + final RecordCollector recordCollector = new RecordCollectorImpl( + taskId, + streamsConfig, + logContext, + new MockStreamsMetrics(metrics), + clientSupplier.consumer, + id -> clientSupplier.getProducer(new HashMap<>())); + return new StreamTask( + taskId, + partitions, + topology, + clientSupplier.consumer, streamsConfig, new MockStreamsMetrics(metrics), stateDirectory, null, new MockTime(), - () -> clientSupplier.getProducer(new HashMap<>())); + stateManager, + recordCollector); } private void mockThread(final boolean initialized) { diff --git a/streams/src/test/java/org/apache/kafka/test/MockRecordCollector.java b/streams/src/test/java/org/apache/kafka/test/MockRecordCollector.java index ac48a7fa829ae..597a8b771d9c9 100644 --- a/streams/src/test/java/org/apache/kafka/test/MockRecordCollector.java +++ b/streams/src/test/java/org/apache/kafka/test/MockRecordCollector.java @@ -16,7 +16,7 @@ */ package org.apache.kafka.test; -import org.apache.kafka.clients.producer.Producer; +import org.apache.kafka.clients.consumer.OffsetAndMetadata; import org.apache.kafka.clients.producer.ProducerRecord; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.header.Headers; @@ -36,9 +36,15 @@ public class MockRecordCollector implements RecordCollector { // remember all records that are collected so far private final List> collected = new LinkedList<>(); + // remember all commits that are submitted so far + private final List> committed = new LinkedList<>(); + // remember if flushed is called private boolean flushed = false; + @Override + public void initialize() {} + @Override public void send(final String topic, final K key, @@ -74,7 +80,9 @@ public void send(final String topic, } @Override - public void init(final Producer producer) {} + public void commit(final Map offsets) { + committed.add(offsets); + } @Override public void flush() { @@ -93,6 +101,10 @@ public List> collected() { return unmodifiableList(collected); } + public List> committed() { + return unmodifiableList(committed); + } + public boolean flushed() { return flushed; } diff --git a/streams/test-utils/src/main/java/org/apache/kafka/streams/TopologyTestDriver.java b/streams/test-utils/src/main/java/org/apache/kafka/streams/TopologyTestDriver.java index 98d7aad48ac78..b9588ae393ce9 100644 --- a/streams/test-utils/src/main/java/org/apache/kafka/streams/TopologyTestDriver.java +++ b/streams/test-utils/src/main/java/org/apache/kafka/streams/TopologyTestDriver.java @@ -43,6 +43,9 @@ import org.apache.kafka.streams.internals.KeyValueStoreFacade; import org.apache.kafka.streams.internals.QuietStreamsConfig; import org.apache.kafka.streams.internals.WindowStoreFacade; +import org.apache.kafka.streams.processor.internals.ProcessorStateManager; +import org.apache.kafka.streams.processor.internals.RecordCollector; +import org.apache.kafka.streams.processor.internals.RecordCollectorImpl; import org.apache.kafka.streams.processor.internals.metrics.TaskMetrics; import org.apache.kafka.streams.processor.ProcessorContext; import org.apache.kafka.streams.processor.PunctuationType; @@ -216,6 +219,17 @@ public class TopologyTestDriver implements Closeable { private final Map>> outputRecordsByTopic = new HashMap<>(); private final boolean eosEnabled; + private final StateRestoreListener stateRestoreListener = new StateRestoreListener() { + @Override + public void onRestoreStart(final TopicPartition topicPartition, final String storeName, final long startingOffset, final long endingOffset) {} + + @Override + public void onBatchRestored(final TopicPartition topicPartition, final String storeName, final long batchEndOffset, final long numRestored) {} + + @Override + public void onRestoreEnd(final TopicPartition topicPartition, final String storeName, final long totalRestored) {} + }; + /** * Create a new test diver instance. * Initialized the internally mocked wall-clock time with {@link System#currentTimeMillis() current system time}. @@ -312,16 +326,6 @@ public List partitionsFor(final String topic) { new LogContext("topology-test-driver "), Math.max(0, streamsConfig.getLong(StreamsConfig.CACHE_MAX_BYTES_BUFFERING_CONFIG)), streamsMetrics); - final StateRestoreListener stateRestoreListener = new StateRestoreListener() { - @Override - public void onRestoreStart(final TopicPartition topicPartition, final String storeName, final long startingOffset, final long endingOffset) {} - - @Override - public void onBatchRestored(final TopicPartition topicPartition, final String storeName, final long batchEndOffset, final long numRestored) {} - - @Override - public void onRestoreEnd(final TopicPartition topicPartition, final String storeName, final long totalRestored) {} - }; for (final InternalTopologyBuilder.TopicsInfo topicsInfo : internalTopologyBuilder.topicGroups().values()) { internalTopics.addAll(topicsInfo.repartitionSourceTopics.keySet()); @@ -377,22 +381,38 @@ public void onRestoreEnd(final TopicPartition topicPartition, final String store } if (!partitionsByTopic.isEmpty()) { - task = new StreamTask( + final LogContext logContext = new LogContext("topology-test-driver "); + final ProcessorStateManager stateManager = new ProcessorStateManager( TASK_ID, new HashSet<>(partitionsByTopic.values()), - processorTopology, - consumer, + false, + stateDirectory, + processorTopology.storeToChangelogTopic(), new StoreChangelogReader( createRestoreConsumer(processorTopology.storeToChangelogTopic()), Duration.ZERO, stateRestoreListener, - new LogContext("topology-test-driver ")), + logContext), + logContext); + final RecordCollector recordCollector = new RecordCollectorImpl( + TASK_ID, + streamsConfig, + logContext, + streamsMetrics, + consumer, + taskId -> producer); + task = new StreamTask( + TASK_ID, + new HashSet<>(partitionsByTopic.values()), + processorTopology, + consumer, streamsConfig, streamsMetrics, stateDirectory, cache, mockWallClockTime, - () -> producer); + stateManager, + recordCollector); task.initializeStateStores(); task.initializeTopology(); ((InternalProcessorContext) task.context()).setRecordContext(new ProcessorRecordContext( @@ -978,7 +998,7 @@ public SessionStore getSessionStore(final String name) { */ public void close() { if (task != null) { - task.close(true, false); + task.close(true); } if (globalStateTask != null) { try {