diff --git a/Jenkinsfile b/Jenkinsfile index 0624ae3caf62c..909a3d1d0f10c 100755 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -65,14 +65,18 @@ def job = { } stage("Check compilation compatibility with Scala 2.12") { - sh "./retry_zinc ./gradlew clean assemble spotlessScalaCheck checkstyleMain checkstyleTest spotbugsMain " + - "--no-daemon --stacktrace -PxmlSpotBugsReport=true -PscalaVersion=2.12" + sh """ + ./retry_zinc ./gradlew clean build -x test \ + --no-daemon --stacktrace -PxmlSpotBugsReport=true -PscalaVersion=2.12 + """ } stage("Compile and validate") { - sh "./retry_zinc ./gradlew clean assemble publishToMavenLocal spotlessScalaCheck checkstyleMain checkstyleTest spotbugsMain " + - "--no-daemon --stacktrace -PxmlSpotBugsReport=true" + sh """ + ./retry_zinc ./gradlew clean publishToMavenLocal build -x test \ + --no-daemon --stacktrace -PxmlSpotBugsReport=true + """ } if (config.publish) { diff --git a/build.gradle b/build.gradle index 5fe36b77e24ca..ea23d83e558e0 100644 --- a/build.gradle +++ b/build.gradle @@ -14,6 +14,7 @@ // limitations under the License. import org.ajoberstar.grgit.Grgit +import org.gradle.api.JavaVersion import java.nio.charset.StandardCharsets @@ -30,26 +31,18 @@ buildscript { } plugins { - id 'com.diffplug.spotless' version '6.13.0' id 'com.github.ben-manes.versions' version '0.44.0' id 'idea' id 'java-library' id 'org.owasp.dependencycheck' version '8.0.2' id 'org.nosphere.apache.rat' version "0.8.0" + id "io.swagger.core.v3.swagger-gradle-plugin" version "2.2.0" id "com.github.spotbugs" version '5.0.13' apply false id 'org.gradle.test-retry' version '1.5.1' apply false id 'org.scoverage' version '7.0.1' apply false id 'com.github.johnrengelman.shadow' version '7.1.2' apply false - id "io.swagger.core.v3.swagger-gradle-plugin" version "2.2.0" -} - -spotless { - scala { - target 'streams/**/*.scala' - scalafmt("$versions.scalafmt").configFile('checkstyle/.scalafmt.conf').scalaMajorVersion(versions.baseScala) - licenseHeaderFile 'checkstyle/java.header', 'package' - } + id 'com.diffplug.spotless' version '6.14.0' apply false // 6.14.1 and newer require Java 11 at compile time, so we can't upgrade until AK 4.0 } task copyGitHooks(type: Copy) { @@ -74,7 +67,7 @@ task installGitHooks(type: Exec) { ext { gradleVersion = versions.gradle - minJavaVersion = "8" + minJavaVersion = 8 buildVersionFileName = "kafka-version.properties" defaultMaxHeapSize = "2g" @@ -229,6 +222,7 @@ if (file('.git').exists()) { '**/build/**', 'CONTRIBUTING.md', 'PULL_REQUEST_TEMPLATE.md', + '.github/**', 'gradlew', 'gradlew.bat', 'gradle/wrapper/gradle-wrapper.properties', @@ -286,9 +280,6 @@ subprojects { fineTuneEclipseClasspathFile(eclipse, project) } - sourceCompatibility = minJavaVersion - targetCompatibility = minJavaVersion - java { consistentResolution { // resolve the compileClasspath and then "inject" the result of resolution as strict constraints into the runtimeClasspath @@ -306,12 +297,16 @@ subprojects { options.compilerArgs << "-Xlint:-try" options.compilerArgs << "-Werror" // --release is the recommended way to select the target release, but it's only supported in Java 9 so we also - // set --source and --target via `sourceCompatibility` and `targetCompatibility`. If/when Gradle supports `--release` - // natively (https://github.com/gradle/gradle/issues/2510), we should switch to that. + // set --source and --target via `sourceCompatibility` and `targetCompatibility` a couple of lines below if (JavaVersion.current().isJava9Compatible()) - options.compilerArgs << "--release" << minJavaVersion + options.release = minJavaVersion dependsOn installGitHooks } + + // We should only set this if Java version is < 9 (--release is recommended for >= 9), but the Scala plugin for IntelliJ sets + // `-target` incorrectly if this is unset + sourceCompatibility = minJavaVersion + targetCompatibility = minJavaVersion if (shouldPublish) { @@ -447,7 +442,7 @@ subprojects { "**/KafkaConfigBackingStoreTest.*", "**/KafkaBasedLogTest.*", "**/StandaloneHerderTest.*", "**/WorkerSinkTaskTest.*", "**/WorkerSinkTaskThreadedTest.*", - "**/WorkerSourceTaskTest.*", "**/AbstractWorkerSourceTaskTest.*" + "**/WorkerSourceTaskTest.*" ]) } @@ -716,9 +711,12 @@ subprojects { ] } - // Scalac's `-release` requires Java 9 or higher - if (JavaVersion.current().isJava9Compatible()) - scalaCompileOptions.additionalParameters += ["-release", minJavaVersion] + // Scalac 2.12 `-release` requires Java 9 or higher, but Scala 2.13 doesn't have that restriction + if (versions.baseScala == "2.13") + scalaCompileOptions.additionalParameters += ["-release:" + minJavaVersion] // Use `:` here to workaround Gradle bug (see https://github.com/gradle/gradle/issues/23962#issuecomment-1437348400) + else if (JavaVersion.current().isJava9Compatible()) + scalaCompileOptions.additionalParameters += ["-release", String.valueOf(minJavaVersion)] // Don't use `:` here as it breaks compilation with Scala 2.12 + configure(scalaCompileOptions.forkOptions) { memoryMaximumSize = defaultMaxHeapSize @@ -1040,6 +1038,7 @@ project(':core') { } compileJava.dependsOn 'processMessages' + srcJar.dependsOn 'processMessages' task genProtocolErrorDocs(type: JavaExec) { classpath = sourceSets.main.runtimeClasspath @@ -1271,6 +1270,7 @@ project(':metadata') { } compileJava.dependsOn 'processMessages' + srcJar.dependsOn 'processMessages' sourceSets { main { @@ -1345,6 +1345,7 @@ project(':group-coordinator') { } compileJava.dependsOn 'processMessages' + srcJar.dependsOn 'processMessages' } project(':examples') { @@ -1483,6 +1484,7 @@ project(':clients') { } compileJava.dependsOn 'processMessages' + srcJar.dependsOn 'processMessages' compileTestJava.dependsOn 'processTestMessages' @@ -1585,6 +1587,7 @@ project(':raft') { } compileJava.dependsOn 'processMessages' + srcJar.dependsOn 'processMessages' jar { dependsOn createVersionFile @@ -1807,6 +1810,7 @@ project(':storage') { } compileJava.dependsOn 'processMessages' + srcJar.dependsOn 'processMessages' jar { dependsOn createVersionFile @@ -2045,6 +2049,7 @@ project(':streams') { } compileJava.dependsOn 'processMessages' + srcJar.dependsOn 'processMessages' javadoc { include "**/org/apache/kafka/streams/**" @@ -2167,7 +2172,17 @@ project(':streams:streams-scala') { dependsOn 'copyDependantLibs' } - test.dependsOn(':spotlessScalaCheck') + // spotless 6.14 requires Java 11 at runtime + if (JavaVersion.current().isJava11Compatible()) { + apply plugin: 'com.diffplug.spotless' + spotless { + scala { + target '**/*.scala' + scalafmt("$versions.scalafmt").configFile('../../checkstyle/.scalafmt.conf').scalaMajorVersion(versions.baseScala) + licenseHeaderFile '../../checkstyle/java.header', 'package' + } + } + } } project(':streams:test-utils') { diff --git a/checkstyle/import-control-core.xml b/checkstyle/import-control-core.xml index 1b052b62d7885..0d5935f9f2b09 100644 --- a/checkstyle/import-control-core.xml +++ b/checkstyle/import-control-core.xml @@ -75,10 +75,8 @@ - - - - + + diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java new file mode 100644 index 0000000000000..00a722a9f9e7a --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java @@ -0,0 +1,238 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.clients.consumer.internals; + +import org.apache.kafka.clients.ClientResponse; +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.clients.consumer.OffsetAndMetadata; +import org.apache.kafka.clients.consumer.RetriableCommitFailedException; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.message.OffsetCommitRequestData; +import org.apache.kafka.common.record.RecordBatch; +import org.apache.kafka.common.requests.OffsetCommitRequest; +import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.Time; +import org.apache.kafka.common.utils.Timer; +import org.slf4j.Logger; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Queue; +import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; + +public class CommitRequestManager implements RequestManager { + private final Queue stagedCommits; + // TODO: We will need to refactor the subscriptionState + private final SubscriptionState subscriptionState; + private final Logger log; + private final Optional autoCommitState; + private final CoordinatorRequestManager coordinatorRequestManager; + private final GroupState groupState; + + public CommitRequestManager( + final Time time, + final LogContext logContext, + final SubscriptionState subscriptionState, + final ConsumerConfig config, + final CoordinatorRequestManager coordinatorRequestManager, + final GroupState groupState) { + Objects.requireNonNull(coordinatorRequestManager, "Coordinator is needed upon committing offsets"); + this.log = logContext.logger(getClass()); + this.stagedCommits = new LinkedList<>(); + if (config.getBoolean(ConsumerConfig.ENABLE_AUTO_COMMIT_CONFIG)) { + final long autoCommitInterval = + Integer.toUnsignedLong(config.getInt(ConsumerConfig.AUTO_COMMIT_INTERVAL_MS_CONFIG)); + this.autoCommitState = Optional.of(new AutoCommitState(time, autoCommitInterval)); + } else { + this.autoCommitState = Optional.empty(); + } + this.coordinatorRequestManager = coordinatorRequestManager; + this.groupState = groupState; + this.subscriptionState = subscriptionState; + } + + /** + * Poll for the commit request if there's any. The function will also try to autocommit, if enabled. + * + * @param currentTimeMs + * @return + */ + @Override + public NetworkClientDelegate.PollResult poll(final long currentTimeMs) { + maybeAutoCommit(currentTimeMs); + + if (stagedCommits.isEmpty()) { + return new NetworkClientDelegate.PollResult(Long.MAX_VALUE, new ArrayList<>()); + } + + List unsentCommitRequests = + stagedCommits.stream().map(StagedCommit::toUnsentRequest).collect(Collectors.toList()); + stagedCommits.clear(); + return new NetworkClientDelegate.PollResult(Long.MAX_VALUE, Collections.unmodifiableList(unsentCommitRequests)); + } + + public CompletableFuture add(final Map offsets) { + StagedCommit commit = new StagedCommit( + offsets, + groupState.groupId, + groupState.groupInstanceId.orElse(null), + groupState.generation); + this.stagedCommits.add(commit); + return commit.future(); + } + + private void maybeAutoCommit(final long currentTimeMs) { + if (!autoCommitState.isPresent()) { + return; + } + + AutoCommitState autocommit = autoCommitState.get(); + if (!autocommit.canSendAutocommit()) { + return; + } + + Map allConsumedOffsets = subscriptionState.allConsumed(); + log.debug("Auto-committing offsets {}", allConsumedOffsets); + sendAutoCommit(allConsumedOffsets); + autocommit.resetTimer(); + + } + + // Visible for testing + CompletableFuture sendAutoCommit(final Map allConsumedOffsets) { + CompletableFuture future = this.add(allConsumedOffsets) + .whenComplete((response, throwable) -> { + if (throwable == null) { + log.debug("Completed asynchronous auto-commit of offsets {}", allConsumedOffsets); + } + // setting inflight commit to false upon completion + autoCommitState.get().setInflightCommitStatus(false); + }) + .exceptionally(t -> { + if (t instanceof RetriableCommitFailedException) { + log.debug("Asynchronous auto-commit of offsets {} failed due to retriable error: {}", allConsumedOffsets, t); + } else { + log.warn("Asynchronous auto-commit of offsets {} failed: {}", allConsumedOffsets, t.getMessage()); + } + return null; + }); + return future; + } + + public void clientPoll(final long currentTimeMs) { + this.autoCommitState.ifPresent(t -> t.ack(currentTimeMs)); + } + + // Visible for testing + Queue stagedCommits() { + return this.stagedCommits; + } + + private class StagedCommit { + private final Map offsets; + private final String groupId; + private final GroupState.Generation generation; + private final String groupInstanceId; + private final NetworkClientDelegate.FutureCompletionHandler future; + + public StagedCommit(final Map offsets, + final String groupId, + final String groupInstanceId, + final GroupState.Generation generation) { + this.offsets = offsets; + // if no callback is provided, DefaultOffsetCommitCallback will be used. + this.future = new NetworkClientDelegate.FutureCompletionHandler(); + this.groupId = groupId; + this.generation = generation; + this.groupInstanceId = groupInstanceId; + } + + public CompletableFuture future() { + return future.future(); + } + + public NetworkClientDelegate.UnsentRequest toUnsentRequest() { + Map requestTopicDataMap = new HashMap<>(); + for (Map.Entry entry : offsets.entrySet()) { + TopicPartition topicPartition = entry.getKey(); + OffsetAndMetadata offsetAndMetadata = entry.getValue(); + + OffsetCommitRequestData.OffsetCommitRequestTopic topic = requestTopicDataMap + .getOrDefault(topicPartition.topic(), + new OffsetCommitRequestData.OffsetCommitRequestTopic() + .setName(topicPartition.topic()) + ); + + topic.partitions().add(new OffsetCommitRequestData.OffsetCommitRequestPartition() + .setPartitionIndex(topicPartition.partition()) + .setCommittedOffset(offsetAndMetadata.offset()) + .setCommittedLeaderEpoch(offsetAndMetadata.leaderEpoch().orElse(RecordBatch.NO_PARTITION_LEADER_EPOCH)) + .setCommittedMetadata(offsetAndMetadata.metadata()) + ); + requestTopicDataMap.put(topicPartition.topic(), topic); + } + + OffsetCommitRequest.Builder builder = new OffsetCommitRequest.Builder( + new OffsetCommitRequestData() + .setGroupId(this.groupId) + .setGenerationId(generation.generationId) + .setMemberId(generation.memberId) + .setGroupInstanceId(groupInstanceId) + .setTopics(new ArrayList<>(requestTopicDataMap.values()))); + return new NetworkClientDelegate.UnsentRequest( + builder, + coordinatorRequestManager.coordinator(), + future); + } + } + + static class AutoCommitState { + private final Timer timer; + private final long autoCommitInterval; + private boolean hasInflightCommit; + + public AutoCommitState( + final Time time, + final long autoCommitInterval) { + this.autoCommitInterval = autoCommitInterval; + this.timer = time.timer(autoCommitInterval); + } + + public boolean canSendAutocommit() { + return !hasInflightCommit && this.timer.isExpired(); + } + + public void resetTimer() { + this.timer.reset(autoCommitInterval); + } + + public void setInflightCommitStatus(final boolean hasInflightCommit) { + this.hasInflightCommit = hasInflightCommit; + } + + public void ack(final long currentTimeMs) { + this.timer.update(currentTimeMs); + } + } +} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java index 481137e074786..62f555b356d37 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThread.java @@ -30,8 +30,10 @@ import org.apache.kafka.common.utils.Utils; import org.slf4j.Logger; -import java.util.Iterator; +import java.util.Collections; +import java.util.HashMap; import java.util.LinkedList; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Queue; @@ -46,7 +48,7 @@ * initialized by the polling thread. */ public class DefaultBackgroundThread extends KafkaThread { - private static final int MAX_POLL_TIMEOUT_MS = 5000; + private static final long MAX_POLL_TIMEOUT_MS = 5000; private static final String BACKGROUND_THREAD_NAME = "consumer_background_thread"; private final Time time; @@ -56,13 +58,14 @@ public class DefaultBackgroundThread extends KafkaThread { private final ConsumerMetadata metadata; private final ConsumerConfig config; // empty if groupId is null - private final Optional coordinatorManager; private final ApplicationEventProcessor applicationEventProcessor; private final NetworkClientDelegate networkClientDelegate; private final ErrorEventHandler errorEventHandler; - + private final GroupState groupState; private boolean running; + private final Map> requestManagerRegistry; + // Visible for testing DefaultBackgroundThread(final Time time, final ConsumerConfig config, @@ -73,7 +76,9 @@ public class DefaultBackgroundThread extends KafkaThread { final ApplicationEventProcessor processor, final ConsumerMetadata metadata, final NetworkClientDelegate networkClient, - final CoordinatorRequestManager coordinatorManager) { + final GroupState groupState, + final CoordinatorRequestManager coordinatorManager, + final CommitRequestManager commitRequestManager) { super(BACKGROUND_THREAD_NAME, true); this.time = time; this.running = true; @@ -84,8 +89,12 @@ public class DefaultBackgroundThread extends KafkaThread { this.config = config; this.metadata = metadata; this.networkClientDelegate = networkClient; - this.coordinatorManager = Optional.ofNullable(coordinatorManager); this.errorEventHandler = errorEventHandler; + this.groupState = groupState; + + this.requestManagerRegistry = new HashMap<>(); + this.requestManagerRegistry.put(RequestManager.Type.COORDINATOR, Optional.ofNullable(coordinatorManager)); + this.requestManagerRegistry.put(RequestManager.Type.COMMIT, Optional.ofNullable(commitRequestManager)); } public DefaultBackgroundThread(final Time time, final ConsumerConfig config, @@ -111,23 +120,37 @@ public DefaultBackgroundThread(final Time time, networkClient); this.running = true; this.errorEventHandler = new ErrorEventHandler(this.backgroundEventQueue); - String groupId = rebalanceConfig.groupId; - this.coordinatorManager = groupId == null ? - Optional.empty() : - Optional.of(new CoordinatorRequestManager( - time, - logContext, - config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), - errorEventHandler, - groupId - )); - this.applicationEventProcessor = new ApplicationEventProcessor(backgroundEventQueue); + this.groupState = new GroupState(rebalanceConfig); + this.requestManagerRegistry = Collections.unmodifiableMap(buildRequestManagerRegistry(logContext)); + this.applicationEventProcessor = new ApplicationEventProcessor(backgroundEventQueue, requestManagerRegistry); } catch (final Exception e) { close(); throw new KafkaException("Failed to construct background processor", e.getCause()); } } + private Map> buildRequestManagerRegistry(final LogContext logContext) { + Map> registry = new HashMap<>(); + CoordinatorRequestManager coordinatorManager = groupState.groupId == null ? + null : + new CoordinatorRequestManager( + time, + logContext, + config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG), + errorEventHandler, + groupState.groupId); + // Add subscriptionState + CommitRequestManager commitRequestManager = coordinatorManager == null ? + null : + new CommitRequestManager(time, + logContext, null, config, + coordinatorManager, + groupState); + registry.put(RequestManager.Type.COORDINATOR, Optional.ofNullable(coordinatorManager)); + registry.put(RequestManager.Type.COMMIT, Optional.ofNullable(commitRequestManager)); + return registry; + } + @Override public void run() { try { @@ -151,31 +174,25 @@ public void run() { /** * Poll and process an {@link ApplicationEvent}. It performs the following tasks: - * 1. Drains and try to process all of the requests in the queue. - * 2. Poll request managers to queue up the necessary requests. + * 1. Drains and try to process all the requests in the queue. + * 2. Iterate through the registry, poll, and get the next poll time for the network poll * 3. Poll the networkClient to send and retrieve the response. */ void runOnce() { - drainAndProcess(); - + drain(); final long currentTimeMs = time.milliseconds(); - long pollWaitTimeMs = MAX_POLL_TIMEOUT_MS; - - if (coordinatorManager.isPresent()) { - pollWaitTimeMs = Math.min( - pollWaitTimeMs, - handlePollResult(coordinatorManager.get().poll(currentTimeMs))); - } - + final long pollWaitTimeMs = requestManagerRegistry.values().stream() + .filter(Optional::isPresent) + .map(m -> m.get().poll(currentTimeMs)) + .map(this::handlePollResult) + .reduce(MAX_POLL_TIMEOUT_MS, Math::min); networkClientDelegate.poll(pollWaitTimeMs, currentTimeMs); } - private void drainAndProcess() { + private void drain() { Queue events = pollApplicationEvent(); - Iterator iter = events.iterator(); - while (iter.hasNext()) { - ApplicationEvent event = iter.next(); - log.debug("processing application event: {}", event); + for (ApplicationEvent event : events) { + log.debug("Consuming application event: {}", event); consumeApplicationEvent(event); } } @@ -198,7 +215,6 @@ private Queue pollApplicationEvent() { } private void consumeApplicationEvent(final ApplicationEvent event) { - log.debug("try consuming event: {}", Optional.ofNullable(event)); Objects.requireNonNull(event); applicationEventProcessor.process(event); } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/GroupState.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/GroupState.java new file mode 100644 index 0000000000000..4e16055f6fdb9 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/GroupState.java @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.clients.consumer.internals; + +import org.apache.kafka.clients.GroupRebalanceConfig; +import org.apache.kafka.common.requests.JoinGroupRequest; +import org.apache.kafka.common.requests.OffsetCommitRequest; + +import java.util.Objects; +import java.util.Optional; + +public class GroupState { + public final String groupId; + public final Optional groupInstanceId; + public Generation generation = Generation.NO_GENERATION; + + public GroupState(String groupId, Optional groupInstanceId) { + this.groupId = groupId; + this.groupInstanceId = groupInstanceId; + } + + public GroupState(final GroupRebalanceConfig config) { + this.groupId = config.groupId; + this.groupInstanceId = config.groupInstanceId; + } + + protected static class Generation { + public static final Generation NO_GENERATION = new Generation( + OffsetCommitRequest.DEFAULT_GENERATION_ID, + JoinGroupRequest.UNKNOWN_MEMBER_ID, + null); + + public final int generationId; + public final String memberId; + public final String protocolName; + + public Generation(int generationId, String memberId, String protocolName) { + this.generationId = generationId; + this.memberId = memberId; + this.protocolName = protocolName; + } + + /** + * @return true if this generation has a valid member id, false otherwise. A member might have an id before + * it becomes part of a group generation. + */ + public boolean hasMemberId() { + return !memberId.isEmpty(); + } + + @Override + public boolean equals(final Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final Generation that = (Generation) o; + return generationId == that.generationId && + Objects.equals(memberId, that.memberId) && + Objects.equals(protocolName, that.protocolName); + } + + @Override + public int hashCode() { + return Objects.hash(generationId, memberId, protocolName); + } + + @Override + public String toString() { + return "Generation{" + + "generationId=" + generationId + + ", memberId='" + memberId + '\'' + + ", protocol='" + protocolName + '\'' + + '}'; + } + } +} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegate.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegate.java index fd36861a2edc3..c817b942e2e86 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegate.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegate.java @@ -201,9 +201,15 @@ public static class UnsentRequest { private Timer timer; public UnsentRequest( - final AbstractRequest.Builder requestBuilder, - final Optional node - ) { + final AbstractRequest.Builder requestBuilder, + final Optional node) { + this(requestBuilder, node, new FutureCompletionHandler()); + } + + public UnsentRequest( + final AbstractRequest.Builder requestBuilder, + final Optional node, + final FutureCompletionHandler handler) { Objects.requireNonNull(requestBuilder); this.requestBuilder = requestBuilder; this.node = node; @@ -243,6 +249,10 @@ public void onFailure(final RuntimeException e) { future.completeExceptionally(e); } + public CompletableFuture future() { + return future; + } + @Override public void onComplete(final ClientResponse response) { if (response.authenticationException() != null) { diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java index 9c66c8a76eae6..18fd79621a946 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java @@ -29,8 +29,8 @@ import org.apache.kafka.clients.consumer.OffsetAndTimestamp; import org.apache.kafka.clients.consumer.OffsetCommitCallback; import org.apache.kafka.clients.consumer.OffsetResetStrategy; -import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.BackgroundEvent; +import org.apache.kafka.clients.consumer.internals.events.CommitApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.EventHandler; import org.apache.kafka.common.KafkaException; import org.apache.kafka.common.Metric; @@ -216,18 +216,21 @@ private Fetch collectFetches() { */ @Override public void commitAsync() { - final ApplicationEvent commitEvent = new CommitApplicationEvent(); - eventHandler.add(commitEvent); + commitAsync(null); } @Override public void commitAsync(OffsetCommitCallback callback) { - throw new KafkaException("method not implemented"); + commitAsync(subscriptions.allConsumed(), callback); } @Override public void commitAsync(Map offsets, OffsetCommitCallback callback) { - throw new KafkaException("method not implemented"); + final CommitApplicationEvent commitEvent = new CommitApplicationEvent(offsets); + commitEvent.future().whenComplete((r, t) -> { + callback.onComplete(offsets, new RuntimeException(t)); + }); + eventHandler.add(commitEvent); } @Override @@ -395,10 +398,10 @@ public void wakeup() { */ @Override public void commitSync(final Duration timeout) { - final CommitApplicationEvent commitEvent = new CommitApplicationEvent(); + final CommitApplicationEvent commitEvent = new CommitApplicationEvent(subscriptions.allConsumed()); eventHandler.add(commitEvent); - final CompletableFuture commitFuture = commitEvent.commitFuture; + final CompletableFuture commitFuture = commitEvent.future(); try { commitFuture.get(timeout.toMillis(), TimeUnit.MILLISECONDS); } catch (final TimeoutException e) { @@ -471,18 +474,6 @@ public ConsumerRecords poll(long timeout) { throw new KafkaException("method not implemented"); } - /** - * A stubbed ApplicationEvent for demonstration purpose - */ - private class CommitApplicationEvent extends ApplicationEvent { - // this is the stubbed commitAsyncEvents - CompletableFuture commitFuture = new CompletableFuture<>(); - - public CommitApplicationEvent() { - super(Type.COMMIT); - } - } - private static ClusterResourceListeners configureClusterResourceListeners( final Deserializer keyDeserializer, final Deserializer valueDeserializer, @@ -512,4 +503,12 @@ private static Metrics buildMetrics( config.originalsWithPrefix(CommonClientConfigs.METRICS_CONTEXT_PREFIX)); return new Metrics(metricConfig, reporters, time, metricsContext); } + + private class DefaultOffsetCommitCallback implements OffsetCommitCallback { + @Override + public void onComplete(Map offsets, Exception exception) { + if (exception != null) + log.error("Offset commit with offsets {} failed", offsets, exception); + } + } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestManager.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestManager.java index 13c7c4d9d16b4..a9c381fadc7c9 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestManager.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestManager.java @@ -24,4 +24,8 @@ */ public interface RequestManager { PollResult poll(long currentTimeMs); + + enum Type { + COORDINATOR, COMMIT + } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java index 2218441485c38..d62278b9b3fbb 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEvent.java @@ -36,6 +36,6 @@ public String toString() { return type + " ApplicationEvent"; } public enum Type { - NOOP, COMMIT, + NOOP, COMMIT, POLL, } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventProcessor.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventProcessor.java index e031fb9f7bbce..62945bd46f10d 100644 --- a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventProcessor.java +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventProcessor.java @@ -16,22 +16,35 @@ */ package org.apache.kafka.clients.consumer.internals.events; +import org.apache.kafka.clients.consumer.internals.CommitRequestManager; import org.apache.kafka.clients.consumer.internals.NoopBackgroundEvent; +import org.apache.kafka.clients.consumer.internals.RequestManager; +import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.concurrent.BlockingQueue; public class ApplicationEventProcessor { private final BlockingQueue backgroundEventQueue; + private final Map> registry; - public ApplicationEventProcessor(final BlockingQueue backgroundEventQueue) { + public ApplicationEventProcessor( + final BlockingQueue backgroundEventQueue, + final Map> requestManagerRegistry) { this.backgroundEventQueue = backgroundEventQueue; + this.registry = requestManagerRegistry; } + public boolean process(final ApplicationEvent event) { Objects.requireNonNull(event); switch (event.type) { case NOOP: return process((NoopApplicationEvent) event); + case COMMIT: + return process((CommitApplicationEvent) event); + case POLL: + return process((PollApplicationEvent) event); } return false; } @@ -46,4 +59,36 @@ public boolean process(final ApplicationEvent event) { private boolean process(final NoopApplicationEvent event) { return backgroundEventQueue.add(new NoopBackgroundEvent(event.message)); } + + private boolean process(final PollApplicationEvent event) { + Optional commitRequestManger = registry.get(RequestManager.Type.COMMIT); + if (!commitRequestManger.isPresent()) { + return true; + } + + CommitRequestManager manager = (CommitRequestManager) commitRequestManger.get(); + manager.clientPoll(event.pollTimeMs); + return true; + } + + private boolean process(final CommitApplicationEvent event) { + Optional commitRequestManger = registry.get(RequestManager.Type.COMMIT); + if (!commitRequestManger.isPresent()) { + // Leaving this error handling here, but it is a bit strange as the commit API should enforce the group.id + // upfront so we should never get to this block. + backgroundEventQueue.add(new ErrorBackgroundEvent( + new RuntimeException("Unable to commit offset. Most likely because the group.id wasn't set"))); + return false; + } + + CommitRequestManager manager = (CommitRequestManager) commitRequestManger.get(); + manager.add(event.offsets()).whenComplete((r, e) -> { + if (e != null) { + event.future().completeExceptionally(e); + return; + } + event.future().complete(null); + }); + return true; + } } diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java new file mode 100644 index 0000000000000..67c416a8a2fd8 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/CommitApplicationEvent.java @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.clients.consumer.internals.events; + +import org.apache.kafka.clients.consumer.OffsetAndMetadata; +import org.apache.kafka.common.TopicPartition; + +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; + +public class CommitApplicationEvent extends ApplicationEvent { + final private CompletableFuture future; + final private Map offsets; + + public CommitApplicationEvent(final Map offsets) { + super(Type.COMMIT); + this.offsets = offsets; + Optional exception = isValid(offsets); + if (exception.isPresent()) { + throw new RuntimeException(exception.get()); + } + this.future = new CompletableFuture<>(); + } + + public CompletableFuture future() { + return future; + } + + public Map offsets() { + return offsets; + } + + private Optional isValid(final Map offsets) { + for (Map.Entry entry : offsets.entrySet()) { + TopicPartition topicPartition = entry.getKey(); + OffsetAndMetadata offsetAndMetadata = entry.getValue(); + if (offsetAndMetadata.offset() < 0) { + return Optional.of(new IllegalArgumentException("Invalid offset: " + offsetAndMetadata.offset())); + } + } + return Optional.empty(); + } + + @Override + public String toString() { + return "CommitApplicationEvent(" + + "offsets=" + offsets + ")"; + } +} diff --git a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/PollApplicationEvent.java b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/PollApplicationEvent.java new file mode 100644 index 0000000000000..c14998dc65243 --- /dev/null +++ b/clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/PollApplicationEvent.java @@ -0,0 +1,26 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.clients.consumer.internals.events; + +public class PollApplicationEvent extends ApplicationEvent { + public final long pollTimeMs; + + protected PollApplicationEvent(final long currentTimeMs) { + super(Type.POLL); + this.pollTimeMs = currentTimeMs; + } +} diff --git a/clients/src/main/java/org/apache/kafka/common/utils/CopyOnWriteMap.java b/clients/src/main/java/org/apache/kafka/common/utils/CopyOnWriteMap.java index 1a3351f7ab225..35e2d7e365545 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/CopyOnWriteMap.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/CopyOnWriteMap.java @@ -85,7 +85,7 @@ public synchronized void clear() { @Override public synchronized V put(K k, V v) { - Map copy = new HashMap(this.map); + Map copy = new HashMap<>(this.map); V prev = copy.put(k, v); this.map = Collections.unmodifiableMap(copy); return prev; @@ -93,14 +93,14 @@ public synchronized V put(K k, V v) { @Override public synchronized void putAll(Map entries) { - Map copy = new HashMap(this.map); + Map copy = new HashMap<>(this.map); copy.putAll(entries); this.map = Collections.unmodifiableMap(copy); } @Override public synchronized V remove(Object key) { - Map copy = new HashMap(this.map); + Map copy = new HashMap<>(this.map); V prev = copy.remove(key); this.map = Collections.unmodifiableMap(copy); return prev; diff --git a/clients/src/main/java/org/apache/kafka/common/utils/Exit.java b/clients/src/main/java/org/apache/kafka/common/utils/Exit.java index 976dfe4df98c9..6d503ca2ad5c3 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/Exit.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/Exit.java @@ -32,28 +32,15 @@ public interface ShutdownHookAdder { void addShutdownHook(String name, Runnable runnable); } - private static final Procedure DEFAULT_HALT_PROCEDURE = new Procedure() { - @Override - public void execute(int statusCode, String message) { - Runtime.getRuntime().halt(statusCode); - } - }; + private static final Procedure DEFAULT_HALT_PROCEDURE = (statusCode, message) -> Runtime.getRuntime().halt(statusCode); - private static final Procedure DEFAULT_EXIT_PROCEDURE = new Procedure() { - @Override - public void execute(int statusCode, String message) { - System.exit(statusCode); - } - }; + private static final Procedure DEFAULT_EXIT_PROCEDURE = (statusCode, message) -> System.exit(statusCode); - private static final ShutdownHookAdder DEFAULT_SHUTDOWN_HOOK_ADDER = new ShutdownHookAdder() { - @Override - public void addShutdownHook(String name, Runnable runnable) { - if (name != null) - Runtime.getRuntime().addShutdownHook(KafkaThread.nonDaemon(name, runnable)); - else - Runtime.getRuntime().addShutdownHook(new Thread(runnable)); - } + private static final ShutdownHookAdder DEFAULT_SHUTDOWN_HOOK_ADDER = (name, runnable) -> { + if (name != null) + Runtime.getRuntime().addShutdownHook(KafkaThread.nonDaemon(name, runnable)); + else + Runtime.getRuntime().addShutdownHook(new Thread(runnable)); }; private volatile static Procedure exitProcedure = DEFAULT_EXIT_PROCEDURE; diff --git a/clients/src/main/java/org/apache/kafka/common/utils/ExponentialBackoff.java b/clients/src/main/java/org/apache/kafka/common/utils/ExponentialBackoff.java index 7550184ba39d0..30a6bf8e7fed8 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/ExponentialBackoff.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/ExponentialBackoff.java @@ -20,12 +20,15 @@ import java.util.concurrent.ThreadLocalRandom; /** - * An utility class for keeping the parameters and providing the value of exponential + * A utility class for keeping the parameters and providing the value of exponential * retry backoff, exponential reconnect backoff, exponential timeout, etc. + *

* The formula is: - * Backoff(attempts) = random(1 - jitter, 1 + jitter) * initialInterval * multiplier ^ attempts - * If initialInterval is greater or equal than maxInterval, a constant backoff of will be provided - * This class is thread-safe + *

Backoff(attempts) = random(1 - jitter, 1 + jitter) * initialInterval * multiplier ^ attempts
+ * If {@code initialInterval} is greater than or equal to {@code maxInterval}, a constant backoff of + * {@code initialInterval} will be provided. + *

+ * This class is thread-safe. */ public class ExponentialBackoff { private final int multiplier; diff --git a/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashCollection.java b/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashCollection.java index ef33f5fee7dc3..193ef6c2a1888 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashCollection.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashCollection.java @@ -320,7 +320,7 @@ final int slot(Element[] curElements, Object e) { * @param key The element to match. * @return The match index, or INVALID_INDEX if no match was found. */ - final private int findIndexOfEqualElement(Object key) { + private int findIndexOfEqualElement(Object key) { if (key == null || size == 0) { return INVALID_INDEX; } diff --git a/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashMultiCollection.java b/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashMultiCollection.java index b95b7de21673b..534bb588c1ff7 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashMultiCollection.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/ImplicitLinkedHashMultiCollection.java @@ -121,7 +121,7 @@ int findElementToRemove(Object key) { */ final public List findAll(E key) { if (key == null || size() == 0) { - return Collections.emptyList(); + return Collections.emptyList(); } ArrayList results = new ArrayList<>(); int slot = slot(elements, key); diff --git a/clients/src/main/java/org/apache/kafka/common/utils/Sanitizer.java b/clients/src/main/java/org/apache/kafka/common/utils/Sanitizer.java index f921590012b58..ce6812f6522cf 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/Sanitizer.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/Sanitizer.java @@ -50,9 +50,8 @@ public class Sanitizer { * using URL-encoding. */ public static String sanitize(String name) { - String encoded = ""; try { - encoded = URLEncoder.encode(name, StandardCharsets.UTF_8.name()); + String encoded = URLEncoder.encode(name, StandardCharsets.UTF_8.name()); StringBuilder builder = new StringBuilder(); for (int i = 0; i < encoded.length(); i++) { char c = encoded.charAt(i); diff --git a/clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java b/clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java index 88a4cfc592536..10c6b3bbf78fe 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java @@ -19,8 +19,6 @@ import org.apache.kafka.common.acl.AclOperation; import org.apache.kafka.common.acl.AclPermissionType; import org.apache.kafka.common.config.SecurityConfig; -import org.apache.kafka.common.resource.PatternType; -import org.apache.kafka.common.resource.ResourcePattern; import org.apache.kafka.common.resource.ResourceType; import org.apache.kafka.common.security.auth.SecurityProviderCreator; import org.apache.kafka.common.security.auth.KafkaPrincipal; @@ -171,9 +169,4 @@ public static void authorizeByResourceTypeCheckArgs(AclOperation op, "Unknown operation type"); } } - - public static boolean denyAll(ResourcePattern pattern) { - return pattern.patternType() == PatternType.LITERAL - && pattern.name().equals(ResourcePattern.WILDCARD_RESOURCE); - } } diff --git a/clients/src/main/java/org/apache/kafka/common/utils/Utils.java b/clients/src/main/java/org/apache/kafka/common/utils/Utils.java index 0c3d6f1563600..2b35a4aa7a6b0 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/Utils.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/Utils.java @@ -1281,15 +1281,6 @@ public static List toList(Iterator iterator, Predicate predicate) { return res; } - public static List concatListsUnmodifiable(List left, List right) { - return concatLists(left, right, Collections::unmodifiableList); - } - - public static List concatLists(List left, List right, Function, List> finisher) { - return Stream.concat(left.stream(), right.stream()) - .collect(Collectors.collectingAndThen(Collectors.toList(), finisher)); - } - public static int to32BitField(final Set bytes) { int value = 0; for (final byte b : bytes) @@ -1315,18 +1306,6 @@ public static Set from32BitField(final int intValue) { return result; } - public static Map transformMap( - Map map, - Function keyMapper, - Function valueMapper) { - return map.entrySet().stream().collect( - Collectors.toMap( - entry -> keyMapper.apply(entry.getKey()), - entry -> valueMapper.apply(entry.getValue()) - ) - ); - } - /** * A Collector that offers two kinds of convenience: * 1. You can specify the concrete type of the returned Map @@ -1478,12 +1457,6 @@ public static boolean isBlank(String str) { return str == null || str.trim().isEmpty(); } - public static Map initializeMap(Collection keys, Supplier valueSupplier) { - Map res = new HashMap<>(keys.size()); - keys.forEach(key -> res.put(key, valueSupplier.get())); - return res; - } - /** * Get an array containing all of the {@link Object#toString string representations} of a given enumerable type. * @param enumClass the enum class; may not be null diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/CommitRequestManagerTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/CommitRequestManagerTest.java new file mode 100644 index 0000000000000..95749bde194bd --- /dev/null +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/CommitRequestManagerTest.java @@ -0,0 +1,139 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.clients.consumer.internals; + +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.clients.consumer.OffsetAndMetadata; +import org.apache.kafka.common.KafkaException; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.serialization.StringDeserializer; +import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.MockTime; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.Properties; + +import static org.apache.kafka.clients.consumer.ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG; +import static org.apache.kafka.clients.consumer.ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class CommitRequestManagerTest { + private SubscriptionState subscriptionState; + private GroupState groupState; + private LogContext logContext; + private MockTime time; + private CoordinatorRequestManager coordinatorRequestManager; + private Properties props; + + @BeforeEach + public void setup() { + this.logContext = new LogContext(); + this.time = new MockTime(0); + this.subscriptionState = mock(SubscriptionState.class); + this.coordinatorRequestManager = mock(CoordinatorRequestManager.class); + this.groupState = new GroupState("group-1", Optional.empty()); + + this.props = new Properties(); + this.props.put(ConsumerConfig.AUTO_COMMIT_INTERVAL_MS_CONFIG, 100); + this.props.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + this.props.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); + } + + @Test + public void testPoll() { + CommitRequestManager commitRequestManger = create(false, 0); + NetworkClientDelegate.PollResult res = commitRequestManger.poll(time.milliseconds()); + assertEquals(0, res.unsentRequests.size()); + assertEquals(Long.MAX_VALUE, res.timeUntilNextPollMs); + + Map offsets = new HashMap<>(); + offsets.put(new TopicPartition("t1", 0), new OffsetAndMetadata(0)); + commitRequestManger.add(offsets); + res = commitRequestManger.poll(time.milliseconds()); + assertEquals(1, res.unsentRequests.size()); + } + + @Test + public void testPollAndAutoCommit() { + CommitRequestManager commitRequestManger = create(true, 100); + NetworkClientDelegate.PollResult res = commitRequestManger.poll(time.milliseconds()); + assertEquals(0, res.unsentRequests.size()); + assertEquals(Long.MAX_VALUE, res.timeUntilNextPollMs); + + Map offsets = new HashMap<>(); + offsets.put(new TopicPartition("t1", 0), new OffsetAndMetadata(0)); + commitRequestManger.clientPoll(time.milliseconds()); + when(subscriptionState.allConsumed()).thenReturn(offsets); + time.sleep(100); + commitRequestManger.clientPoll(time.milliseconds()); + res = commitRequestManger.poll(time.milliseconds()); + assertEquals(1, res.unsentRequests.size()); + } + + @Test + public void testAutocommitStateUponFailure() { + CommitRequestManager commitRequestManger = create(true, 100); + time.sleep(100); + commitRequestManger.clientPoll(time.milliseconds()); + NetworkClientDelegate.PollResult res = commitRequestManger.poll(time.milliseconds()); + time.sleep(100); + // We want to make sure we don't resend autocommit if the previous request has not been completed + assertEquals(Long.MAX_VALUE, commitRequestManger.poll(time.milliseconds()).timeUntilNextPollMs); + + // complete the autocommit request (exceptionally) + res.unsentRequests.get(0).future().completeExceptionally(new KafkaException("test exception")); + + // we can then autocommit again + commitRequestManger.clientPoll(time.milliseconds()); + res = commitRequestManger.poll(time.milliseconds()); + assertEquals(1, res.unsentRequests.size()); + } + + @Test + public void testEnsureStagedCommitsPurgedAfterPoll() { + CommitRequestManager commitRequestManger = create(true, 100); + commitRequestManger.add(new HashMap<>()); + assertEquals(1, commitRequestManger.stagedCommits().size()); + NetworkClientDelegate.PollResult res = commitRequestManger.poll(time.milliseconds()); + assertTrue(commitRequestManger.stagedCommits().isEmpty()); + } + + @Test + public void testAutoCommitFuture() { + CommitRequestManager commitRequestManger = create(true, 100); + commitRequestManger.sendAutoCommit(new HashMap<>()).complete(null); + commitRequestManger.sendAutoCommit(new HashMap<>()).completeExceptionally(new RuntimeException("mock " + + "exception")); + } + + private CommitRequestManager create(final boolean autoCommitEnabled, final long autoCommitInterval) { + return new CommitRequestManager( + this.time, + this.logContext, + this.subscriptionState, + new ConsumerConfig(props), + this.coordinatorRequestManager, + this.groupState); + } +} diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java index 7ee4ee6a1263d..dbc1753959083 100644 --- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/DefaultBackgroundThreadTest.java @@ -16,6 +16,7 @@ */ package org.apache.kafka.clients.consumer.internals; +import org.apache.kafka.clients.GroupRebalanceConfig; import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.clients.consumer.internals.events.ApplicationEvent; import org.apache.kafka.clients.consumer.internals.events.ApplicationEventProcessor; @@ -50,7 +51,7 @@ import static org.mockito.Mockito.when; public class DefaultBackgroundThreadTest { - private static final long REFRESH_BACK_OFF_MS = 100; + private static final long RETRY_BACKOFF_MS = 100; private final Properties properties = new Properties(); private MockTime time; private ConsumerMetadata metadata; @@ -61,6 +62,8 @@ public class DefaultBackgroundThreadTest { private CoordinatorRequestManager coordinatorManager; private ErrorEventHandler errorEventHandler; private int requestTimeoutMs = 500; + private GroupState groupState; + private CommitRequestManager commitManager; @BeforeEach @SuppressWarnings("unchecked") @@ -73,6 +76,16 @@ public void setup() { this.processor = mock(ApplicationEventProcessor.class); this.coordinatorManager = mock(CoordinatorRequestManager.class); this.errorEventHandler = mock(ErrorEventHandler.class); + GroupRebalanceConfig rebalanceConfig = new GroupRebalanceConfig( + 100, + 100, + 100, + "group_id", + Optional.empty(), + 100, + true); + this.groupState = new GroupState(rebalanceConfig); + this.commitManager = mock(CommitRequestManager.class); } @Test @@ -87,7 +100,8 @@ public void testStartupAndTearDown() { public void testApplicationEvent() { this.applicationEventsQueue = new LinkedBlockingQueue<>(); this.backgroundEventsQueue = new LinkedBlockingQueue<>(); - when(coordinatorManager.poll(anyLong())).thenReturn(mockPollResult()); + when(coordinatorManager.poll(anyLong())).thenReturn(mockPollCoordinatorResult()); + when(commitManager.poll(anyLong())).thenReturn(mockPollCommitResult()); DefaultBackgroundThread backgroundThread = mockBackgroundThread(); ApplicationEvent e = new NoopApplicationEvent("noop event"); this.applicationEventsQueue.add(e); @@ -99,7 +113,8 @@ public void testApplicationEvent() { @Test void testFindCoordinator() { DefaultBackgroundThread backgroundThread = mockBackgroundThread(); - when(this.coordinatorManager.poll(time.milliseconds())).thenReturn(mockPollResult()); + when(this.coordinatorManager.poll(anyLong())).thenReturn(mockPollCoordinatorResult()); + when(this.commitManager.poll(anyLong())).thenReturn(mockPollCommitResult()); backgroundThread.runOnce(); Mockito.verify(coordinatorManager, times(1)).poll(anyLong()); Mockito.verify(networkClient, times(1)).poll(anyLong(), anyLong()); @@ -122,8 +137,8 @@ void testPollResultTimer() { } private static NetworkClientDelegate.UnsentRequest findCoordinatorUnsentRequest( - final Time time, - final long timeout + final Time time, + final long timeout ) { NetworkClientDelegate.UnsentRequest req = new NetworkClientDelegate.UnsentRequest( new FindCoordinatorRequest.Builder( @@ -138,7 +153,7 @@ private static NetworkClientDelegate.UnsentRequest findCoordinatorUnsentRequest( private DefaultBackgroundThread mockBackgroundThread() { properties.put(KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); properties.put(VALUE_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class); - properties.put(RETRY_BACKOFF_MS_CONFIG, REFRESH_BACK_OFF_MS); + properties.put(RETRY_BACKOFF_MS_CONFIG, RETRY_BACKOFF_MS); return new DefaultBackgroundThread( this.time, @@ -150,12 +165,20 @@ private DefaultBackgroundThread mockBackgroundThread() { processor, this.metadata, this.networkClient, - this.coordinatorManager); + this.groupState, + this.coordinatorManager, + this.commitManager); } - private NetworkClientDelegate.PollResult mockPollResult() { + private NetworkClientDelegate.PollResult mockPollCoordinatorResult() { return new NetworkClientDelegate.PollResult( - 0, + RETRY_BACKOFF_MS, + Collections.singletonList(findCoordinatorUnsentRequest(time, requestTimeoutMs))); + } + + private NetworkClientDelegate.PollResult mockPollCommitResult() { + return new NetworkClientDelegate.PollResult( + RETRY_BACKOFF_MS, Collections.singletonList(findCoordinatorUnsentRequest(time, requestTimeoutMs))); } } diff --git a/clients/src/test/java/org/apache/kafka/clients/producer/internals/DefaultPartitionerTest.java b/clients/src/test/java/org/apache/kafka/clients/producer/internals/DefaultPartitionerTest.java index c851408f3633c..2af61806a34e7 100644 --- a/clients/src/test/java/org/apache/kafka/clients/producer/internals/DefaultPartitionerTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/producer/internals/DefaultPartitionerTest.java @@ -46,7 +46,7 @@ public void testKeyPartitionIsStable() { final Partitioner partitioner = new DefaultPartitioner(); final Cluster cluster = new Cluster("clusterId", asList(NODES), PARTITIONS, Collections.emptySet(), Collections.emptySet()); - int partition = partitioner.partition("test", null, KEY_BYTES, null, null, cluster); - assertEquals(partition, partitioner.partition("test", null, KEY_BYTES, null, null, cluster), "Same key should yield same partition"); + int partition = partitioner.partition(TOPIC, null, KEY_BYTES, null, null, cluster); + assertEquals(partition, partitioner.partition(TOPIC, null, KEY_BYTES, null, null, cluster), "Same key should yield same partition"); } } diff --git a/connect/api/src/main/java/org/apache/kafka/connect/storage/SimpleHeaderConverter.java b/connect/api/src/main/java/org/apache/kafka/connect/storage/SimpleHeaderConverter.java index 69c4b86189878..9aaed75f2bf7c 100644 --- a/connect/api/src/main/java/org/apache/kafka/connect/storage/SimpleHeaderConverter.java +++ b/connect/api/src/main/java/org/apache/kafka/connect/storage/SimpleHeaderConverter.java @@ -58,9 +58,6 @@ public SchemaAndValue toConnectHeader(String topic, String headerKey, byte[] val } try { String str = new String(value, UTF_8); - if (str.isEmpty()) { - return new SchemaAndValue(Schema.STRING_SCHEMA, str); - } return Values.parseString(str); } catch (NoSuchElementException e) { throw new DataException("Failed to deserialize value for header '" + headerKey + "' on topic '" + topic + "'", e); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTask.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTask.java index fb3c04be6cf97..2021808286c05 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTask.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTask.java @@ -578,6 +578,8 @@ static class SourceRecordWriteCounter { private final int batchSize; private boolean completed = false; private int counter; + private int skipped; // Keeps track of filtered records + public SourceRecordWriteCounter(int batchSize, SourceTaskMetricsGroup metricsGroup) { assert batchSize > 0; assert metricsGroup != null; @@ -586,6 +588,7 @@ public SourceRecordWriteCounter(int batchSize, SourceTaskMetricsGroup metricsGro this.metricsGroup = metricsGroup; } public void skipRecord() { + skipped += 1; if (counter > 0 && --counter == 0) { finishedAllWrites(); } @@ -600,7 +603,7 @@ public void retryRemaining() { } private void finishedAllWrites() { if (!completed) { - metricsGroup.recordWrite(batchSize - counter); + metricsGroup.recordWrite(batchSize - counter, skipped); completed = true; } } @@ -652,8 +655,8 @@ void recordPoll(int batchSize, long duration) { sourceRecordActiveCount.record(activeRecordCount); } - void recordWrite(int recordCount) { - sourceRecordWrite.record(recordCount); + void recordWrite(int recordCount, int skippedCount) { + sourceRecordWrite.record(recordCount - skippedCount); activeRecordCount -= recordCount; activeRecordCount = Math.max(0, activeRecordCount); sourceRecordActiveCount.record(activeRecordCount); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectMetricsRegistry.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectMetricsRegistry.java index d8579d44fc655..507ebc405c80e 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectMetricsRegistry.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/ConnectMetricsRegistry.java @@ -179,15 +179,14 @@ public ConnectMetricsRegistry(Set tags) { "belonging to the named source connector in this worker.", sourceTaskTags); sourceRecordWriteRate = createTemplate("source-record-write-rate", SOURCE_TASK_GROUP_NAME, - "The average per-second number of records output from the transformations and written" + - " to Kafka for this task belonging to the named source connector in this worker. This" + - " is after transformations are applied and excludes any records filtered out by the " + - "transformations.", + "The average per-second number of records written to Kafka for this task belonging to the " + + "named source connector in this worker, since the task was last restarted. This is after " + + "transformations are applied, and excludes any records filtered out by the transformations.", sourceTaskTags); sourceRecordWriteTotal = createTemplate("source-record-write-total", SOURCE_TASK_GROUP_NAME, - "The number of records output from the transformations and written to Kafka for this" + - " task belonging to the named source connector in this worker, since the task was " + - "last restarted.", + "The number of records output written to Kafka for this task belonging to the " + + "named source connector in this worker, since the task was last restarted. This is after " + + "transformations are applied, and excludes any records filtered out by the transformations.", sourceTaskTags); sourceRecordPollBatchTimeMax = createTemplate("poll-batch-max-time-ms", SOURCE_TASK_GROUP_NAME, "The maximum time in milliseconds taken by this task to poll for a batch of " + diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/integration/ExactlyOnceSourceIntegrationTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/integration/ExactlyOnceSourceIntegrationTest.java index a2dd730b344f8..bacfd311f5dbe 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/integration/ExactlyOnceSourceIntegrationTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/integration/ExactlyOnceSourceIntegrationTest.java @@ -81,6 +81,7 @@ import static org.apache.kafka.connect.integration.MonitorableSourceConnector.CUSTOM_EXACTLY_ONCE_SUPPORT_CONFIG; import static org.apache.kafka.connect.integration.MonitorableSourceConnector.CUSTOM_TRANSACTION_BOUNDARIES_CONFIG; import static org.apache.kafka.connect.integration.MonitorableSourceConnector.MESSAGES_PER_POLL_CONFIG; +import static org.apache.kafka.connect.integration.MonitorableSourceConnector.MAX_MESSAGES_PER_SECOND_CONFIG; import static org.apache.kafka.connect.integration.MonitorableSourceConnector.TOPIC_CONFIG; import static org.apache.kafka.connect.runtime.ConnectorConfig.CONNECTOR_CLASS_CONFIG; import static org.apache.kafka.connect.runtime.ConnectorConfig.CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX; @@ -115,6 +116,12 @@ public class ExactlyOnceSourceIntegrationTest { private static final int SOURCE_TASK_PRODUCE_TIMEOUT_MS = 30_000; private static final int DEFAULT_NUM_WORKERS = 3; + // Tests require that a minimum but not unreasonably large number of records are sourced. + // Throttle the poll such that a reasonable amount of records are produced while the test runs. + private static final int MINIMUM_MESSAGES = 100; + private static final String MESSAGES_PER_POLL = Integer.toString(MINIMUM_MESSAGES); + private static final String MESSAGES_PER_SECOND = Long.toString(MINIMUM_MESSAGES / 2); + private Properties brokerProps; private Map workerProps; private EmbeddedConnectCluster.Builder connectBuilder; @@ -255,7 +262,6 @@ public void testPollBoundary() throws Exception { connect.kafka().createTopic(topic, 3); int numTasks = 1; - int recordsProduced = 100; Map props = new HashMap<>(); props.put(CONNECTOR_CLASS_CONFIG, MonitorableSourceConnector.class.getName()); @@ -265,11 +271,12 @@ public void testPollBoundary() throws Exception { props.put(VALUE_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); props.put(NAME_CONFIG, CONNECTOR_NAME); props.put(TRANSACTION_BOUNDARY_CONFIG, POLL.toString()); - props.put(MESSAGES_PER_POLL_CONFIG, Integer.toString(recordsProduced)); + props.put(MESSAGES_PER_POLL_CONFIG, MESSAGES_PER_POLL); + props.put(MAX_MESSAGES_PER_SECOND_CONFIG, MESSAGES_PER_SECOND); // expect all records to be consumed and committed by the connector - connectorHandle.expectedRecords(recordsProduced); - connectorHandle.expectedCommits(recordsProduced); + connectorHandle.expectedRecords(MINIMUM_MESSAGES); + connectorHandle.expectedCommits(MINIMUM_MESSAGES); // start a source connector connect.configureConnector(CONNECTOR_NAME, props); @@ -293,8 +300,8 @@ public void testPollBoundary() throws Exception { null, topic ); - assertTrue("Not enough records produced by source connector. Expected at least: " + recordsProduced + " + but got " + records.count(), - records.count() >= recordsProduced); + assertTrue("Not enough records produced by source connector. Expected at least: " + MINIMUM_MESSAGES + " + but got " + records.count(), + records.count() >= MINIMUM_MESSAGES); assertExactlyOnceSeqnos(records, numTasks); } @@ -314,7 +321,6 @@ public void testIntervalBoundary() throws Exception { connect.kafka().createTopic(topic, 3); int numTasks = 1; - int recordsProduced = 100; Map props = new HashMap<>(); props.put(CONNECTOR_CLASS_CONFIG, MonitorableSourceConnector.class.getName()); @@ -325,11 +331,12 @@ public void testIntervalBoundary() throws Exception { props.put(NAME_CONFIG, CONNECTOR_NAME); props.put(TRANSACTION_BOUNDARY_CONFIG, INTERVAL.toString()); props.put(TRANSACTION_BOUNDARY_INTERVAL_CONFIG, "10000"); - props.put(MESSAGES_PER_POLL_CONFIG, Integer.toString(recordsProduced)); + props.put(MESSAGES_PER_POLL_CONFIG, MESSAGES_PER_POLL); + props.put(MAX_MESSAGES_PER_SECOND_CONFIG, MESSAGES_PER_SECOND); // expect all records to be consumed and committed by the connector - connectorHandle.expectedRecords(recordsProduced); - connectorHandle.expectedCommits(recordsProduced); + connectorHandle.expectedRecords(MINIMUM_MESSAGES); + connectorHandle.expectedCommits(MINIMUM_MESSAGES); // start a source connector connect.configureConnector(CONNECTOR_NAME, props); @@ -353,8 +360,8 @@ public void testIntervalBoundary() throws Exception { null, topic ); - assertTrue("Not enough records produced by source connector. Expected at least: " + recordsProduced + " + but got " + records.count(), - records.count() >= recordsProduced); + assertTrue("Not enough records produced by source connector. Expected at least: " + MINIMUM_MESSAGES + " + but got " + records.count(), + records.count() >= MINIMUM_MESSAGES); assertExactlyOnceSeqnos(records, numTasks); } @@ -375,8 +382,6 @@ public void testConnectorBoundary() throws Exception { String topic = "test-topic"; connect.kafka().createTopic(topic, 3); - int recordsProduced = 100; - Map props = new HashMap<>(); props.put(CONNECTOR_CLASS_CONFIG, MonitorableSourceConnector.class.getName()); props.put(TASKS_MAX_CONFIG, "1"); @@ -386,11 +391,12 @@ public void testConnectorBoundary() throws Exception { props.put(NAME_CONFIG, CONNECTOR_NAME); props.put(TRANSACTION_BOUNDARY_CONFIG, CONNECTOR.toString()); props.put(CUSTOM_TRANSACTION_BOUNDARIES_CONFIG, MonitorableSourceConnector.TRANSACTION_BOUNDARIES_SUPPORTED); - props.put(MESSAGES_PER_POLL_CONFIG, Integer.toString(recordsProduced)); + props.put(MESSAGES_PER_POLL_CONFIG, MESSAGES_PER_POLL); + props.put(MAX_MESSAGES_PER_SECOND_CONFIG, MESSAGES_PER_SECOND); // expect all records to be consumed and committed by the connector - connectorHandle.expectedRecords(recordsProduced); - connectorHandle.expectedCommits(recordsProduced); + connectorHandle.expectedRecords(MINIMUM_MESSAGES); + connectorHandle.expectedCommits(MINIMUM_MESSAGES); // start a source connector connect.configureConnector(CONNECTOR_NAME, props); @@ -412,15 +418,15 @@ public void testConnectorBoundary() throws Exception { null, topic ); - assertTrue("Not enough records produced by source connector. Expected at least: " + recordsProduced + " + but got " + sourceRecords.count(), - sourceRecords.count() >= recordsProduced); + assertTrue("Not enough records produced by source connector. Expected at least: " + MINIMUM_MESSAGES + " + but got " + sourceRecords.count(), + sourceRecords.count() >= MINIMUM_MESSAGES); // also consume from the cluster's offsets topic to verify that the expected offsets (which should correspond to the connector's // custom transaction boundaries) were committed List expectedOffsetSeqnos = new ArrayList<>(); long lastExpectedOffsetSeqno = 1; long nextExpectedOffsetSeqno = 1; - while (nextExpectedOffsetSeqno <= recordsProduced) { + while (nextExpectedOffsetSeqno <= MINIMUM_MESSAGES) { expectedOffsetSeqnos.add(nextExpectedOffsetSeqno); nextExpectedOffsetSeqno += lastExpectedOffsetSeqno; lastExpectedOffsetSeqno = nextExpectedOffsetSeqno - lastExpectedOffsetSeqno; @@ -438,7 +444,7 @@ public void testConnectorBoundary() throws Exception { assertEquals("Committed offsets should match connector-defined transaction boundaries", expectedOffsetSeqnos, actualOffsetSeqnos.subList(0, expectedOffsetSeqnos.size())); - List expectedRecordSeqnos = LongStream.range(1, recordsProduced + 1).boxed().collect(Collectors.toList()); + List expectedRecordSeqnos = LongStream.range(1, MINIMUM_MESSAGES + 1).boxed().collect(Collectors.toList()); long priorBoundary = 1; long nextBoundary = 2; while (priorBoundary < expectedRecordSeqnos.get(expectedRecordSeqnos.size() - 1)) { @@ -474,7 +480,6 @@ public void testFencedLeaderRecovery() throws Exception { connect.kafka().createTopic(topic, 3); int numTasks = 1; - int recordsProduced = 100; Map props = new HashMap<>(); props.put(CONNECTOR_CLASS_CONFIG, MonitorableSourceConnector.class.getName()); @@ -484,11 +489,12 @@ public void testFencedLeaderRecovery() throws Exception { props.put(VALUE_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); props.put(NAME_CONFIG, CONNECTOR_NAME); props.put(TRANSACTION_BOUNDARY_CONFIG, POLL.toString()); - props.put(MESSAGES_PER_POLL_CONFIG, Integer.toString(recordsProduced)); + props.put(MESSAGES_PER_POLL_CONFIG, MESSAGES_PER_POLL); + props.put(MAX_MESSAGES_PER_SECOND_CONFIG, MESSAGES_PER_SECOND); // expect all records to be consumed and committed by the connector - connectorHandle.expectedRecords(recordsProduced); - connectorHandle.expectedCommits(recordsProduced); + connectorHandle.expectedRecords(MINIMUM_MESSAGES); + connectorHandle.expectedCommits(MINIMUM_MESSAGES); // make sure the worker is actually up (otherwise, it may fence out our simulated zombie leader, instead of the other way around) assertEquals(404, connect.requestGet(connect.endpointForResource("connectors/nonexistent")).getStatus()); @@ -526,8 +532,8 @@ public void testFencedLeaderRecovery() throws Exception { null, topic ); - assertTrue("Not enough records produced by source connector. Expected at least: " + recordsProduced + " + but got " + records.count(), - records.count() >= recordsProduced); + assertTrue("Not enough records produced by source connector. Expected at least: " + MINIMUM_MESSAGES + " + but got " + records.count(), + records.count() >= MINIMUM_MESSAGES); assertExactlyOnceSeqnos(records, numTasks); } @@ -545,19 +551,18 @@ public void testConnectorReconfiguration() throws Exception { String topic = "test-topic"; connect.kafka().createTopic(topic, 3); - int recordsProduced = 100; - Map props = new HashMap<>(); props.put(CONNECTOR_CLASS_CONFIG, MonitorableSourceConnector.class.getName()); props.put(TOPIC_CONFIG, topic); props.put(KEY_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); props.put(VALUE_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); props.put(NAME_CONFIG, CONNECTOR_NAME); - props.put(MESSAGES_PER_POLL_CONFIG, Integer.toString(recordsProduced)); + props.put(MESSAGES_PER_POLL_CONFIG, MESSAGES_PER_POLL); + props.put(MAX_MESSAGES_PER_SECOND_CONFIG, MESSAGES_PER_SECOND); // expect all records to be consumed and committed by the connector - connectorHandle.expectedRecords(recordsProduced); - connectorHandle.expectedCommits(recordsProduced); + connectorHandle.expectedRecords(MINIMUM_MESSAGES); + connectorHandle.expectedCommits(MINIMUM_MESSAGES); StartAndStopLatch connectorStart = connectorAndTaskStart(3); props.put(TASKS_MAX_CONFIG, "3"); @@ -590,8 +595,8 @@ public void testConnectorReconfiguration() throws Exception { null, topic ); - assertTrue("Not enough records produced by source connector. Expected at least: " + recordsProduced + " + but got " + records.count(), - records.count() >= recordsProduced); + assertTrue("Not enough records produced by source connector. Expected at least: " + MINIMUM_MESSAGES + " + but got " + records.count(), + records.count() >= MINIMUM_MESSAGES); // We used at most five tasks during the tests; each of them should have been able to produce records assertExactlyOnceSeqnos(records, 5); } @@ -743,7 +748,6 @@ public void testSeparateOffsetsTopic() throws Exception { connectorTargetedCluster.createTopic(topic, 3); int numTasks = 1; - int recordsProduced = 100; Map props = new HashMap<>(); props.put(CONNECTOR_CLASS_CONFIG, MonitorableSourceConnector.class.getName()); @@ -753,7 +757,8 @@ public void testSeparateOffsetsTopic() throws Exception { props.put(VALUE_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); props.put(NAME_CONFIG, CONNECTOR_NAME); props.put(TRANSACTION_BOUNDARY_CONFIG, POLL.toString()); - props.put(MESSAGES_PER_POLL_CONFIG, Integer.toString(recordsProduced)); + props.put(MESSAGES_PER_POLL_CONFIG, MESSAGES_PER_POLL); + props.put(MAX_MESSAGES_PER_SECOND_CONFIG, MESSAGES_PER_SECOND); props.put(CONNECTOR_CLIENT_PRODUCER_OVERRIDES_PREFIX + BOOTSTRAP_SERVERS_CONFIG, connectorTargetedCluster.bootstrapServers()); props.put(CONNECTOR_CLIENT_CONSUMER_OVERRIDES_PREFIX + BOOTSTRAP_SERVERS_CONFIG, connectorTargetedCluster.bootstrapServers()); props.put(CONNECTOR_CLIENT_ADMIN_OVERRIDES_PREFIX + BOOTSTRAP_SERVERS_CONFIG, connectorTargetedCluster.bootstrapServers()); @@ -761,8 +766,8 @@ public void testSeparateOffsetsTopic() throws Exception { props.put(OFFSETS_TOPIC_CONFIG, offsetsTopic); // expect all records to be consumed and committed by the connector - connectorHandle.expectedRecords(recordsProduced); - connectorHandle.expectedCommits(recordsProduced); + connectorHandle.expectedRecords(MINIMUM_MESSAGES); + connectorHandle.expectedCommits(MINIMUM_MESSAGES); // start a source connector connect.configureConnector(CONNECTOR_NAME, props); @@ -778,13 +783,13 @@ public void testSeparateOffsetsTopic() throws Exception { // consume at least the expected number of records from the source topic or fail, to ensure that they were correctly produced int recordNum = connectorTargetedCluster .consume( - recordsProduced, + MINIMUM_MESSAGES, TimeUnit.MINUTES.toMillis(1), Collections.singletonMap(ConsumerConfig.ISOLATION_LEVEL_CONFIG, "read_committed"), "test-topic") .count(); - assertTrue("Not enough records produced by source connector. Expected at least: " + recordsProduced + " + but got " + recordNum, - recordNum >= recordsProduced); + assertTrue("Not enough records produced by source connector. Expected at least: " + MINIMUM_MESSAGES + " + but got " + recordNum, + recordNum >= MINIMUM_MESSAGES); // also consume from the connector's dedicated offsets topic ConsumerRecords offsetRecords = connectorTargetedCluster @@ -796,8 +801,8 @@ public void testSeparateOffsetsTopic() throws Exception { ); List seqnos = parseAndAssertOffsetsForSingleTask(offsetRecords); seqnos.forEach(seqno -> - assertEquals("Offset commits should occur on connector-defined poll boundaries, which happen every " + recordsProduced + " records", - 0, seqno % recordsProduced) + assertEquals("Offset commits should occur on connector-defined poll boundaries, which happen every " + MINIMUM_MESSAGES + " records", + 0, seqno % MINIMUM_MESSAGES) ); // also consume from the cluster's global offsets topic @@ -810,8 +815,8 @@ public void testSeparateOffsetsTopic() throws Exception { ); seqnos = parseAndAssertOffsetsForSingleTask(offsetRecords); seqnos.forEach(seqno -> - assertEquals("Offset commits should occur on connector-defined poll boundaries, which happen every " + recordsProduced + " records", - 0, seqno % recordsProduced) + assertEquals("Offset commits should occur on connector-defined poll boundaries, which happen every " + MINIMUM_MESSAGES + " records", + 0, seqno % MINIMUM_MESSAGES) ); // Shut down the whole cluster @@ -820,8 +825,8 @@ public void testSeparateOffsetsTopic() throws Exception { workerProps.put(EXACTLY_ONCE_SOURCE_SUPPORT_CONFIG, "disabled"); // Establish new expectations for records+offsets - connectorHandle.expectedRecords(recordsProduced); - connectorHandle.expectedCommits(recordsProduced); + connectorHandle.expectedRecords(MINIMUM_MESSAGES); + connectorHandle.expectedCommits(MINIMUM_MESSAGES); // Restart the whole cluster for (int i = 0; i < DEFAULT_NUM_WORKERS; i++) { @@ -856,8 +861,8 @@ public void testSeparateOffsetsTopic() throws Exception { null, topic ); - assertTrue("Not enough records produced by source connector. Expected at least: " + recordsProduced + " + but got " + sourceRecords.count(), - sourceRecords.count() >= recordsProduced); + assertTrue("Not enough records produced by source connector. Expected at least: " + MINIMUM_MESSAGES + " + but got " + sourceRecords.count(), + sourceRecords.count() >= MINIMUM_MESSAGES); // also have to check which offsets have actually been committed, since we no longer have exactly-once semantics offsetRecords = connectorTargetedCluster.consumeAll( CONSUME_RECORDS_TIMEOUT_MS, @@ -896,8 +901,6 @@ public void testPotentialDeadlockWhenProducingToOffsetsTopic() throws Exception String topic = "test-topic"; connect.kafka().createTopic(topic, 3); - int recordsProduced = 100; - Map props = new HashMap<>(); // See below; this connector does nothing except request offsets from the worker in SourceTask::poll // and then return a single record targeted at its offsets topic @@ -905,7 +908,6 @@ public void testPotentialDeadlockWhenProducingToOffsetsTopic() throws Exception props.put(TASKS_MAX_CONFIG, "1"); props.put(NAME_CONFIG, CONNECTOR_NAME); props.put(TRANSACTION_BOUNDARY_CONFIG, INTERVAL.toString()); - props.put(MESSAGES_PER_POLL_CONFIG, Integer.toString(recordsProduced)); props.put(OFFSETS_TOPIC_CONFIG, "whoops"); // start a source connector diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/integration/MonitorableSourceConnector.java b/connect/runtime/src/test/java/org/apache/kafka/connect/integration/MonitorableSourceConnector.java index 33ba1588a7d04..102b10d7fb75a 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/integration/MonitorableSourceConnector.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/integration/MonitorableSourceConnector.java @@ -52,6 +52,7 @@ public class MonitorableSourceConnector extends SampleSourceConnector { public static final String TOPIC_CONFIG = "topic"; public static final String MESSAGES_PER_POLL_CONFIG = "messages.per.poll"; + public static final String MAX_MESSAGES_PER_SECOND_CONFIG = "throughput"; public static final String CUSTOM_EXACTLY_ONCE_SUPPORT_CONFIG = "custom.exactly.once.support"; public static final String EXACTLY_ONCE_SUPPORTED = "supported"; @@ -177,7 +178,7 @@ public void start(Map props) { startingSeqno = Optional.ofNullable((Long) offset.get("saved")).orElse(0L); seqno = startingSeqno; log.info("Started {} task {} with properties {}", this.getClass().getSimpleName(), taskId, props); - throttler = new ThroughputThrottler(Long.parseLong(props.getOrDefault("throughput", "-1")), System.currentTimeMillis()); + throttler = new ThroughputThrottler(Long.parseLong(props.getOrDefault(MAX_MESSAGES_PER_SECOND_CONFIG, "-1")), System.currentTimeMillis()); taskHandle.recordTaskStart(); priorTransactionBoundary = 0; nextTransactionBoundary = 1; diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTaskTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTaskTest.java index e95a794739fd2..d77796716f1f9 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTaskTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTaskTest.java @@ -16,20 +16,21 @@ */ package org.apache.kafka.connect.runtime; -import org.apache.kafka.clients.admin.NewTopic; +import java.util.stream.Collectors; import org.apache.kafka.clients.admin.TopicDescription; +import java.util.Arrays; import org.apache.kafka.clients.producer.Callback; +import org.apache.kafka.clients.admin.NewTopic; import org.apache.kafka.clients.producer.KafkaProducer; import org.apache.kafka.clients.producer.ProducerRecord; import org.apache.kafka.clients.producer.RecordMetadata; import org.apache.kafka.common.InvalidRecordException; import org.apache.kafka.common.MetricName; -import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.TopicPartitionInfo; import org.apache.kafka.common.errors.TopicAuthorizationException; -import org.apache.kafka.common.header.Header; -import org.apache.kafka.common.header.Headers; import org.apache.kafka.common.header.internals.RecordHeaders; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.header.Headers; import org.apache.kafka.common.utils.Time; import org.apache.kafka.connect.data.Schema; import org.apache.kafka.connect.data.SchemaAndValue; @@ -53,30 +54,24 @@ import org.apache.kafka.connect.util.ConnectorTaskId; import org.apache.kafka.connect.util.TopicAdmin; import org.apache.kafka.connect.util.TopicCreationGroup; -import org.easymock.Capture; -import org.easymock.EasyMock; -import org.easymock.IAnswer; -import org.easymock.IExpectationSetters; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.powermock.api.easymock.PowerMock; -import org.powermock.api.easymock.annotation.Mock; -import org.powermock.api.easymock.annotation.MockStrict; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.modules.junit4.PowerMockRunner; +import org.mockito.AdditionalAnswers; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.Arrays; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.Future; import java.util.concurrent.TimeoutException; import static org.apache.kafka.connect.integration.MonitorableSourceConnector.TOPIC_CONFIG; @@ -95,12 +90,19 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; -@PowerMockIgnore({"javax.management.*", - "org.apache.log4j.*"}) -@RunWith(PowerMockRunner.class) +@SuppressWarnings("unchecked") +@RunWith(MockitoJUnitRunner.StrictStubs.class) public class AbstractWorkerSourceTaskTest { private static final String TOPIC = "topic"; @@ -118,7 +120,8 @@ public class AbstractWorkerSourceTaskTest { private static final byte[] SERIALIZED_KEY = "converted-key".getBytes(); private static final byte[] SERIALIZED_RECORD = "converted-record".getBytes(); - @Mock private SourceTask sourceTask; + @Mock + private SourceTask sourceTask; @Mock private TopicAdmin admin; @Mock private KafkaProducer producer; @Mock private Converter keyConverter; @@ -130,7 +133,7 @@ public class AbstractWorkerSourceTaskTest { @Mock private ConnectorOffsetBackingStore offsetStore; @Mock private StatusBackingStore statusBackingStore; @Mock private WorkerSourceTaskContext sourceTaskContext; - @MockStrict private TaskStatus.Listener statusListener; + @Mock private TaskStatus.Listener statusListener; private final ConnectorTaskId taskId = new ConnectorTaskId("job", 0); private final ConnectorTaskId taskId1 = new ConnectorTaskId("job", 1); @@ -140,7 +143,6 @@ public class AbstractWorkerSourceTaskTest { private SourceConnectorConfig sourceConfig; private MockConnectMetrics metrics = new MockConnectMetrics(); @Mock private ErrorHandlingMetrics errorHandlingMetrics; - private Capture producerCallbacks; private AbstractWorkerSourceTask workerTask; @@ -149,8 +151,7 @@ public void setup() { Map workerProps = workerProps(); plugins = new Plugins(workerProps); config = new StandaloneConfig(workerProps); - sourceConfig = new SourceConnectorConfig(plugins, sourceConnectorPropsWithGroups(TOPIC), true); - producerCallbacks = EasyMock.newCapture(); + sourceConfig = new SourceConnectorConfig(plugins, sourceConnectorPropsWithGroups(), true); metrics = new MockConnectMetrics(); } @@ -163,27 +164,28 @@ private Map workerProps() { return props; } - private Map sourceConnectorPropsWithGroups(String topic) { + private Map sourceConnectorPropsWithGroups() { // setup up props for the source connector Map props = new HashMap<>(); props.put("name", "foo-connector"); props.put(CONNECTOR_CLASS_CONFIG, MonitorableSourceConnector.class.getSimpleName()); props.put(TASKS_MAX_CONFIG, String.valueOf(1)); - props.put(TOPIC_CONFIG, topic); + props.put(TOPIC_CONFIG, TOPIC); props.put(KEY_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); props.put(VALUE_CONVERTER_CLASS_CONFIG, StringConverter.class.getName()); props.put(TOPIC_CREATION_GROUPS_CONFIG, String.join(",", "foo", "bar")); props.put(DEFAULT_TOPIC_CREATION_PREFIX + REPLICATION_FACTOR_CONFIG, String.valueOf(1)); props.put(DEFAULT_TOPIC_CREATION_PREFIX + PARTITIONS_CONFIG, String.valueOf(1)); - props.put(SourceConnectorConfig.TOPIC_CREATION_PREFIX + "foo" + "." + INCLUDE_REGEX_CONFIG, topic); + props.put(SourceConnectorConfig.TOPIC_CREATION_PREFIX + "foo" + "." + INCLUDE_REGEX_CONFIG, TOPIC); props.put(SourceConnectorConfig.TOPIC_CREATION_PREFIX + "bar" + "." + INCLUDE_REGEX_CONFIG, ".*"); - props.put(SourceConnectorConfig.TOPIC_CREATION_PREFIX + "bar" + "." + EXCLUDE_REGEX_CONFIG, topic); + props.put(SourceConnectorConfig.TOPIC_CREATION_PREFIX + "bar" + "." + EXCLUDE_REGEX_CONFIG, TOPIC); return props; } @After public void tearDown() { if (metrics != null) metrics.stop(); + verifyNoMoreInteractions(statusListener); } @Test @@ -192,18 +194,18 @@ public void testMetricsGroup() { AbstractWorkerSourceTask.SourceTaskMetricsGroup group1 = new AbstractWorkerSourceTask.SourceTaskMetricsGroup(taskId1, metrics); for (int i = 0; i != 10; ++i) { group.recordPoll(100, 1000 + i * 100); - group.recordWrite(10); + group.recordWrite(10, 2); } for (int i = 0; i != 20; ++i) { group1.recordPoll(100, 1000 + i * 100); - group1.recordWrite(10); + group1.recordWrite(10, 4); } assertEquals(1900.0, metrics.currentMetricValueAsDouble(group.metricGroup(), "poll-batch-max-time-ms"), 0.001d); assertEquals(1450.0, metrics.currentMetricValueAsDouble(group.metricGroup(), "poll-batch-avg-time-ms"), 0.001d); assertEquals(33.333, metrics.currentMetricValueAsDouble(group.metricGroup(), "source-record-poll-rate"), 0.001d); assertEquals(1000, metrics.currentMetricValueAsDouble(group.metricGroup(), "source-record-poll-total"), 0.001d); - assertEquals(3.3333, metrics.currentMetricValueAsDouble(group.metricGroup(), "source-record-write-rate"), 0.001d); - assertEquals(100, metrics.currentMetricValueAsDouble(group.metricGroup(), "source-record-write-total"), 0.001d); + assertEquals(2.666, metrics.currentMetricValueAsDouble(group.metricGroup(), "source-record-write-rate"), 0.001d); + assertEquals(80, metrics.currentMetricValueAsDouble(group.metricGroup(), "source-record-write-total"), 0.001d); assertEquals(900.0, metrics.currentMetricValueAsDouble(group.metricGroup(), "source-record-active-count"), 0.001d); // Close the group @@ -227,8 +229,8 @@ public void testMetricsGroup() { assertEquals(1950.0, metrics.currentMetricValueAsDouble(group1.metricGroup(), "poll-batch-avg-time-ms"), 0.001d); assertEquals(66.667, metrics.currentMetricValueAsDouble(group1.metricGroup(), "source-record-poll-rate"), 0.001d); assertEquals(2000, metrics.currentMetricValueAsDouble(group1.metricGroup(), "source-record-poll-total"), 0.001d); - assertEquals(6.667, metrics.currentMetricValueAsDouble(group1.metricGroup(), "source-record-write-rate"), 0.001d); - assertEquals(200, metrics.currentMetricValueAsDouble(group1.metricGroup(), "source-record-write-total"), 0.001d); + assertEquals(4.0, metrics.currentMetricValueAsDouble(group1.metricGroup(), "source-record-write-rate"), 0.001d); + assertEquals(120, metrics.currentMetricValueAsDouble(group1.metricGroup(), "source-record-write-total"), 0.001d); assertEquals(1800.0, metrics.currentMetricValueAsDouble(group1.metricGroup(), "source-record-active-count"), 0.001d); } @@ -236,45 +238,44 @@ public void testMetricsGroup() { public void testSendRecordsConvertsData() { createWorkerTask(); - List records = new ArrayList<>(); // Can just use the same record for key and value - records.add(new SourceRecord(PARTITION, OFFSET, "topic", null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD)); - - Capture> sent = expectSendRecordAnyTimes(); + List records = Collections.singletonList( + new SourceRecord(PARTITION, OFFSET, "topic", null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD) + ); + expectSendRecord(emptyHeaders()); expectTopicCreation(TOPIC); - PowerMock.replayAll(); - workerTask.toSend = records; workerTask.sendRecords(); + + ArgumentCaptor> sent = verifySendRecord(); + assertArrayEquals(SERIALIZED_KEY, sent.getValue().key()); assertArrayEquals(SERIALIZED_RECORD, sent.getValue().value()); - - PowerMock.verifyAll(); + + verifyTaskGetTopic(); + verifyTopicCreation(); } @Test public void testSendRecordsPropagatesTimestamp() { final Long timestamp = System.currentTimeMillis(); - createWorkerTask(); - List records = Collections.singletonList( - new SourceRecord(PARTITION, OFFSET, "topic", null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD, timestamp) - ); - - Capture> sent = expectSendRecordAnyTimes(); - + expectSendRecord(emptyHeaders()); expectTopicCreation(TOPIC); - PowerMock.replayAll(); - - workerTask.toSend = records; + workerTask.toSend = Collections.singletonList( + new SourceRecord(PARTITION, OFFSET, "topic", null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD, timestamp) + ); workerTask.sendRecords(); + + ArgumentCaptor> sent = verifySendRecord(); assertEquals(timestamp, sent.getValue().timestamp()); - PowerMock.verifyAll(); + verifyTaskGetTopic(); + verifyTopicCreation(); } @Test @@ -282,19 +283,14 @@ public void testSendRecordsCorruptTimestamp() { final Long timestamp = -3L; createWorkerTask(); - List records = Collections.singletonList( + expectSendRecord(emptyHeaders()); + + workerTask.toSend = Collections.singletonList( new SourceRecord(PARTITION, OFFSET, "topic", null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD, timestamp) ); - - Capture> sent = expectSendRecordAnyTimes(); - - PowerMock.replayAll(); - - workerTask.toSend = records; assertThrows(InvalidRecordException.class, workerTask::sendRecords); - assertFalse(sent.hasCaptured()); - - PowerMock.verifyAll(); + verifyNoInteractions(producer); + verifyNoInteractions(admin); } @Test @@ -302,49 +298,48 @@ public void testSendRecordsNoTimestamp() { final Long timestamp = -1L; createWorkerTask(); - List records = Collections.singletonList( - new SourceRecord(PARTITION, OFFSET, "topic", null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD, timestamp) - ); - - Capture> sent = expectSendRecordAnyTimes(); - + expectSendRecord(emptyHeaders()); expectTopicCreation(TOPIC); - PowerMock.replayAll(); - - workerTask.toSend = records; + workerTask.toSend = Collections.singletonList( + new SourceRecord(PARTITION, OFFSET, "topic", null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD, timestamp) + ); workerTask.sendRecords(); + + ArgumentCaptor> sent = verifySendRecord(); assertNull(sent.getValue().timestamp()); - PowerMock.verifyAll(); + verifyTaskGetTopic(); + verifyTopicCreation(); } @Test public void testHeaders() { - Headers headers = new RecordHeaders(); - headers.add("header_key", "header_value".getBytes()); + Headers headers = new RecordHeaders() + .add("header_key", "header_value".getBytes()); - org.apache.kafka.connect.header.Headers connectHeaders = new ConnectHeaders(); - connectHeaders.add("header_key", new SchemaAndValue(Schema.STRING_SCHEMA, "header_value")); + org.apache.kafka.connect.header.Headers connectHeaders = new ConnectHeaders() + .add("header_key", new SchemaAndValue(Schema.STRING_SCHEMA, "header_value")); createWorkerTask(); - List records = new ArrayList<>(); - records.add(new SourceRecord(PARTITION, OFFSET, TOPIC, null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD, null, connectHeaders)); - + expectSendRecord(headers); expectTopicCreation(TOPIC); - Capture> sent = expectSendRecord(TOPIC, true, headers); + workerTask.toSend = Collections.singletonList( + new SourceRecord(PARTITION, OFFSET, TOPIC, null, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD, + null, connectHeaders) + ); + workerTask.sendRecords(); - PowerMock.replayAll(); + ArgumentCaptor> sent = verifySendRecord(); - workerTask.toSend = records; - workerTask.sendRecords(); assertArrayEquals(SERIALIZED_KEY, sent.getValue().key()); assertArrayEquals(SERIALIZED_RECORD, sent.getValue().value()); assertEquals(headers, sent.getValue().headers()); - PowerMock.verifyAll(); + verifyTaskGetTopic(); + verifyTopicCreation(); } @Test @@ -354,47 +349,51 @@ public void testHeadersWithCustomConverter() throws Exception { createWorkerTask(stringConverter, testConverter, stringConverter); - List records = new ArrayList<>(); + expectSendRecord(null); + expectTopicCreation(TOPIC); String stringA = "Árvíztűrő tükörfúrógép"; - org.apache.kafka.connect.header.Headers headersA = new ConnectHeaders(); String encodingA = "latin2"; - headersA.addString("encoding", encodingA); - - records.add(new SourceRecord(PARTITION, OFFSET, "topic", null, Schema.STRING_SCHEMA, "a", Schema.STRING_SCHEMA, stringA, null, headersA)); - String stringB = "Тестовое сообщение"; - org.apache.kafka.connect.header.Headers headersB = new ConnectHeaders(); String encodingB = "koi8_r"; - headersB.addString("encoding", encodingB); - records.add(new SourceRecord(PARTITION, OFFSET, "topic", null, Schema.STRING_SCHEMA, "b", Schema.STRING_SCHEMA, stringB, null, headersB)); + org.apache.kafka.connect.header.Headers headersA = new ConnectHeaders() + .addString("encoding", encodingA); + org.apache.kafka.connect.header.Headers headersB = new ConnectHeaders() + .addString("encoding", encodingB); - expectTopicCreation(TOPIC); + workerTask.toSend = Arrays.asList( + new SourceRecord(PARTITION, OFFSET, "topic", null, Schema.STRING_SCHEMA, "a", + Schema.STRING_SCHEMA, stringA, null, headersA), + new SourceRecord(PARTITION, OFFSET, "topic", null, Schema.STRING_SCHEMA, "b", + Schema.STRING_SCHEMA, stringB, null, headersB) + ); + workerTask.sendRecords(); - Capture> sentRecordA = expectSendRecord(TOPIC, false, null); - Capture> sentRecordB = expectSendRecord(TOPIC, false, null); + ArgumentCaptor> sent = verifySendRecord(2); - PowerMock.replayAll(); + List> capturedValues = sent.getAllValues(); + assertEquals(2, capturedValues.size()); - workerTask.toSend = records; - workerTask.sendRecords(); + ProducerRecord sentRecordA = capturedValues.get(0); + ProducerRecord sentRecordB = capturedValues.get(1); - assertEquals(ByteBuffer.wrap("a".getBytes()), ByteBuffer.wrap(sentRecordA.getValue().key())); + assertEquals(ByteBuffer.wrap("a".getBytes()), ByteBuffer.wrap(sentRecordA.key())); assertEquals( - ByteBuffer.wrap(stringA.getBytes(encodingA)), - ByteBuffer.wrap(sentRecordA.getValue().value()) + ByteBuffer.wrap(stringA.getBytes(encodingA)), + ByteBuffer.wrap(sentRecordA.value()) ); - assertEquals(encodingA, new String(sentRecordA.getValue().headers().lastHeader("encoding").value())); + assertEquals(encodingA, new String(sentRecordA.headers().lastHeader("encoding").value())); - assertEquals(ByteBuffer.wrap("b".getBytes()), ByteBuffer.wrap(sentRecordB.getValue().key())); + assertEquals(ByteBuffer.wrap("b".getBytes()), ByteBuffer.wrap(sentRecordB.key())); assertEquals( - ByteBuffer.wrap(stringB.getBytes(encodingB)), - ByteBuffer.wrap(sentRecordB.getValue().value()) + ByteBuffer.wrap(stringB.getBytes(encodingB)), + ByteBuffer.wrap(sentRecordB.value()) ); - assertEquals(encodingB, new String(sentRecordB.getValue().headers().lastHeader("encoding").value())); + assertEquals(encodingB, new String(sentRecordB.headers().lastHeader("encoding").value())); - PowerMock.verifyAll(); + verifyTaskGetTopic(2); + verifyTopicCreation(); } @Test @@ -404,18 +403,21 @@ public void testTopicCreateWhenTopicExists() { SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - expectPreliminaryCalls(); + expectPreliminaryCalls(TOPIC); + TopicPartitionInfo topicPartitionInfo = new TopicPartitionInfo(0, null, Collections.emptyList(), Collections.emptyList()); TopicDescription topicDesc = new TopicDescription(TOPIC, false, Collections.singletonList(topicPartitionInfo)); - EasyMock.expect(admin.describeTopics(TOPIC)).andReturn(Collections.singletonMap(TOPIC, topicDesc)); + when(admin.describeTopics(TOPIC)).thenReturn(Collections.singletonMap(TOPIC, topicDesc)); - expectSendRecord(); - expectSendRecord(); - - PowerMock.replayAll(); + expectSendRecord(emptyHeaders()); workerTask.toSend = Arrays.asList(record1, record2); workerTask.sendRecords(); + + verifySendRecord(2); + verify(admin, never()).createOrFindTopics(any(NewTopic.class)); + // Make sure we didn't try to create the topic after finding out it already existed + verifyNoMoreInteractions(admin); } @Test @@ -425,26 +427,24 @@ public void testSendRecordsTopicDescribeRetries() { SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - expectPreliminaryCalls(); - // First round - call to describe the topic times out - EasyMock.expect(admin.describeTopics(TOPIC)) - .andThrow(new RetriableException(new TimeoutException("timeout"))); - - // Second round - calls to describe and create succeed - expectTopicCreation(TOPIC); - // Exactly two records are sent - expectSendRecord(); - expectSendRecord(); + expectPreliminaryCalls(TOPIC); - PowerMock.replayAll(); + when(admin.describeTopics(TOPIC)) + .thenThrow(new RetriableException(new TimeoutException("timeout"))) + .thenReturn(Collections.emptyMap()); workerTask.toSend = Arrays.asList(record1, record2); workerTask.sendRecords(); assertEquals(Arrays.asList(record1, record2), workerTask.toSend); + verify(admin, never()).createOrFindTopics(any(NewTopic.class)); + verifyNoMoreInteractions(admin); - // Next they all succeed + // Second round - calls to describe and create succeed + expectTopicCreation(TOPIC); workerTask.sendRecords(); assertNull(workerTask.toSend); + + verifyTopicCreation(); } @Test @@ -454,19 +454,14 @@ public void testSendRecordsTopicCreateRetries() { SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - // First call to describe the topic times out - expectPreliminaryCalls(); - EasyMock.expect(admin.describeTopics(TOPIC)).andReturn(Collections.emptyMap()); - Capture newTopicCapture = EasyMock.newCapture(); - EasyMock.expect(admin.createOrFindTopics(EasyMock.capture(newTopicCapture))) - .andThrow(new RetriableException(new TimeoutException("timeout"))); - - // Second round - expectTopicCreation(TOPIC); - expectSendRecord(); - expectSendRecord(); + expectPreliminaryCalls(TOPIC); - PowerMock.replayAll(); + when(admin.describeTopics(TOPIC)).thenReturn(Collections.emptyMap()); + when(admin.createOrFindTopics(any(NewTopic.class))) + // First call to create the topic times out + .thenThrow(new RetriableException(new TimeoutException("timeout"))) + // Next attempt succeeds + .thenReturn(createdTopic(TOPIC)); workerTask.toSend = Arrays.asList(record1, record2); workerTask.sendRecords(); @@ -475,6 +470,9 @@ public void testSendRecordsTopicCreateRetries() { // Next they all succeed workerTask.sendRecords(); assertNull(workerTask.toSend); + + // First attempt failed, second succeeded + verifyTopicCreation(2, TOPIC, TOPIC); } @Test @@ -486,32 +484,37 @@ public void testSendRecordsTopicDescribeRetriesMidway() { SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record3 = new SourceRecord(PARTITION, OFFSET, OTHER_TOPIC, 3, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - // First round + expectPreliminaryCalls(TOPIC); expectPreliminaryCalls(OTHER_TOPIC); - expectTopicCreation(TOPIC); - expectSendRecord(); - expectSendRecord(); - - // First call to describe the topic times out - EasyMock.expect(admin.describeTopics(OTHER_TOPIC)) - .andThrow(new RetriableException(new TimeoutException("timeout"))); - // Second round - expectTopicCreation(OTHER_TOPIC); - expectSendRecord(OTHER_TOPIC, false, emptyHeaders()); - - PowerMock.replayAll(); - - // Try to send 3, make first pass, second fail. Should save last two + when(admin.describeTopics(anyString())) + .thenReturn(Collections.emptyMap()) + .thenThrow(new RetriableException(new TimeoutException("timeout"))) + .thenReturn(Collections.emptyMap()); + when(admin.createOrFindTopics(any(NewTopic.class))).thenAnswer( + (Answer) invocation -> { + NewTopic newTopic = invocation.getArgument(0); + return createdTopic(newTopic.name()); + }); + + // Try to send 3, make first pass, second fail. Should save last record workerTask.toSend = Arrays.asList(record1, record2, record3); workerTask.sendRecords(); - assertEquals(Arrays.asList(record3), workerTask.toSend); + assertEquals(Collections.singletonList(record3), workerTask.toSend); // Next they all succeed workerTask.sendRecords(); assertNull(workerTask.toSend); - PowerMock.verifyAll(); + verify(admin, times(3)).describeTopics(anyString()); + + ArgumentCaptor newTopicCaptor = ArgumentCaptor.forClass(NewTopic.class); + verify(admin, times(2)).createOrFindTopics(newTopicCaptor.capture()); + + assertEquals(Arrays.asList(TOPIC, OTHER_TOPIC), newTopicCaptor.getAllValues() + .stream() + .map(NewTopic::name) + .collect(Collectors.toList())); } @Test @@ -523,34 +526,26 @@ public void testSendRecordsTopicCreateRetriesMidway() { SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record3 = new SourceRecord(PARTITION, OFFSET, OTHER_TOPIC, 3, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - // First round + expectPreliminaryCalls(TOPIC); expectPreliminaryCalls(OTHER_TOPIC); - expectTopicCreation(TOPIC); - expectSendRecord(); - expectSendRecord(); - - EasyMock.expect(admin.describeTopics(OTHER_TOPIC)).andReturn(Collections.emptyMap()); - // First call to create the topic times out - Capture newTopicCapture = EasyMock.newCapture(); - EasyMock.expect(admin.createOrFindTopics(EasyMock.capture(newTopicCapture))) - .andThrow(new RetriableException(new TimeoutException("timeout"))); - - // Second round - expectTopicCreation(OTHER_TOPIC); - expectSendRecord(OTHER_TOPIC, false, emptyHeaders()); - PowerMock.replayAll(); + when(admin.describeTopics(anyString())).thenReturn(Collections.emptyMap()); + when(admin.createOrFindTopics(any(NewTopic.class))) + .thenReturn(createdTopic(TOPIC)) + .thenThrow(new RetriableException(new TimeoutException("timeout"))) + .thenReturn(createdTopic(OTHER_TOPIC)); - // Try to send 3, make first pass, second fail. Should save last two + // Try to send 3, make first pass, second fail. Should save last record workerTask.toSend = Arrays.asList(record1, record2, record3); workerTask.sendRecords(); - assertEquals(Arrays.asList(record3), workerTask.toSend); + assertEquals(Collections.singletonList(record3), workerTask.toSend); + verifyTopicCreation(2, TOPIC, OTHER_TOPIC); // Second call to createOrFindTopics will throw // Next they all succeed workerTask.sendRecords(); assertNull(workerTask.toSend); - PowerMock.verifyAll(); + verifyTopicCreation(3, TOPIC, OTHER_TOPIC, OTHER_TOPIC); } @Test @@ -560,11 +555,10 @@ public void testTopicDescribeFails() { SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - expectPreliminaryCalls(); - EasyMock.expect(admin.describeTopics(TOPIC)) - .andThrow(new ConnectException(new TopicAuthorizationException("unauthorized"))); - - PowerMock.replayAll(); + expectPreliminaryCalls(TOPIC); + when(admin.describeTopics(TOPIC)).thenThrow( + new ConnectException(new TopicAuthorizationException("unauthorized")) + ); workerTask.toSend = Arrays.asList(record1, record2); assertThrows(ConnectException.class, workerTask::sendRecords); @@ -577,18 +571,17 @@ public void testTopicCreateFails() { SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - expectPreliminaryCalls(); - EasyMock.expect(admin.describeTopics(TOPIC)).andReturn(Collections.emptyMap()); - - Capture newTopicCapture = EasyMock.newCapture(); - EasyMock.expect(admin.createOrFindTopics(EasyMock.capture(newTopicCapture))) - .andThrow(new ConnectException(new TopicAuthorizationException("unauthorized"))); - - PowerMock.replayAll(); + expectPreliminaryCalls(TOPIC); + when(admin.describeTopics(TOPIC)).thenReturn(Collections.emptyMap()); + when(admin.createOrFindTopics(any(NewTopic.class))).thenThrow( + new ConnectException(new TopicAuthorizationException("unauthorized")) + ); workerTask.toSend = Arrays.asList(record1, record2); assertThrows(ConnectException.class, workerTask::sendRecords); - assertTrue(newTopicCapture.hasCaptured()); + verify(admin).createOrFindTopics(any()); + + verifyTopicCreation(); } @Test @@ -598,17 +591,16 @@ public void testTopicCreateFailsWithExceptionWhenCreateReturnsTopicNotCreatedOrF SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - expectPreliminaryCalls(); - EasyMock.expect(admin.describeTopics(TOPIC)).andReturn(Collections.emptyMap()); - - Capture newTopicCapture = EasyMock.newCapture(); - EasyMock.expect(admin.createOrFindTopics(EasyMock.capture(newTopicCapture))).andReturn(TopicAdmin.EMPTY_CREATION); + expectPreliminaryCalls(TOPIC); - PowerMock.replayAll(); + when(admin.describeTopics(TOPIC)).thenReturn(Collections.emptyMap()); + when(admin.createOrFindTopics(any(NewTopic.class))).thenReturn(TopicAdmin.EMPTY_CREATION); workerTask.toSend = Arrays.asList(record1, record2); assertThrows(ConnectException.class, workerTask::sendRecords); - assertTrue(newTopicCapture.hasCaptured()); + verify(admin).createOrFindTopics(any()); + + verifyTopicCreation(); } @Test @@ -618,19 +610,21 @@ public void testTopicCreateSucceedsWhenCreateReturnsExistingTopicFound() { SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - expectPreliminaryCalls(); - EasyMock.expect(admin.describeTopics(TOPIC)).andReturn(Collections.emptyMap()); + expectSendRecord(emptyHeaders()); - Capture newTopicCapture = EasyMock.newCapture(); - EasyMock.expect(admin.createOrFindTopics(EasyMock.capture(newTopicCapture))).andReturn(foundTopic(TOPIC)); - - expectSendRecord(); - expectSendRecord(); - - PowerMock.replayAll(); + when(admin.describeTopics(TOPIC)).thenReturn(Collections.emptyMap()); + when(admin.createOrFindTopics(any(NewTopic.class))).thenReturn(foundTopic(TOPIC)); workerTask.toSend = Arrays.asList(record1, record2); workerTask.sendRecords(); + + ArgumentCaptor> sent = verifySendRecord(2); + + List> capturedValues = sent.getAllValues(); + assertEquals(2, capturedValues.size()); + + verifyTaskGetTopic(2); + verifyTopicCreation(); } @Test @@ -640,144 +634,126 @@ public void testTopicCreateSucceedsWhenCreateReturnsNewTopicFound() { SourceRecord record1 = new SourceRecord(PARTITION, OFFSET, TOPIC, 1, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); SourceRecord record2 = new SourceRecord(PARTITION, OFFSET, TOPIC, 2, KEY_SCHEMA, KEY, RECORD_SCHEMA, RECORD); - expectPreliminaryCalls(); - EasyMock.expect(admin.describeTopics(TOPIC)).andReturn(Collections.emptyMap()); - - Capture newTopicCapture = EasyMock.newCapture(); - EasyMock.expect(admin.createOrFindTopics(EasyMock.capture(newTopicCapture))).andReturn(createdTopic(TOPIC)); + expectSendRecord(emptyHeaders()); - expectSendRecord(); - expectSendRecord(); - - PowerMock.replayAll(); + when(admin.describeTopics(TOPIC)).thenReturn(Collections.emptyMap()); + when(admin.createOrFindTopics(any(NewTopic.class))).thenReturn(createdTopic(TOPIC)); workerTask.toSend = Arrays.asList(record1, record2); workerTask.sendRecords(); + + ArgumentCaptor> sent = verifySendRecord(2); + + List> capturedValues = sent.getAllValues(); + assertEquals(2, capturedValues.size()); + + verifyTaskGetTopic(2); + verifyTopicCreation(); } - private Capture> expectSendRecord( - String topic, - boolean anyTimes, - Headers headers - ) { + private void expectSendRecord(Headers headers) { if (headers != null) - expectConvertHeadersAndKeyValue(topic, anyTimes, headers); + expectConvertHeadersAndKeyValue(headers, TOPIC); - expectApplyTransformationChain(anyTimes); + expectApplyTransformationChain(); - Capture> sent = EasyMock.newCapture(); - - IExpectationSetters> expect = EasyMock.expect( - producer.send(EasyMock.capture(sent), EasyMock.capture(producerCallbacks))); + expectTaskGetTopic(); + } - IAnswer> expectResponse = () -> { - synchronized (producerCallbacks) { - for (Callback cb : producerCallbacks.getValues()) { - cb.onCompletion(new RecordMetadata(new TopicPartition("foo", 0), 0, 0, 0L, 0, 0), null); - } - producerCallbacks.reset(); - } - return null; - }; + private ArgumentCaptor> verifySendRecord() { + return verifySendRecord(1); + } - if (anyTimes) - expect.andStubAnswer(expectResponse); - else - expect.andAnswer(expectResponse); + private ArgumentCaptor> verifySendRecord(int times) { + ArgumentCaptor> sent = ArgumentCaptor.forClass(ProducerRecord.class); + ArgumentCaptor producerCallbacks = ArgumentCaptor.forClass(Callback.class); + verify(producer, times(times)).send(sent.capture(), producerCallbacks.capture()); - expectTaskGetTopic(anyTimes); + for (Callback cb : producerCallbacks.getAllValues()) { + cb.onCompletion(new RecordMetadata(new TopicPartition("foo", 0), 0, 0, 0L, 0, 0), + null); + } return sent; } - private Capture> expectSendRecordAnyTimes() { - return expectSendRecord(TOPIC, true, emptyHeaders()); + private void expectTaskGetTopic() { + when(statusBackingStore.getTopic(anyString(), anyString())).thenAnswer((Answer) invocation -> { + String connector = invocation.getArgument(0, String.class); + String topic = invocation.getArgument(1, String.class); + return new TopicStatus(topic, new ConnectorTaskId(connector, 0), Time.SYSTEM.milliseconds()); + }); } - private Capture> expectSendRecord() { - return expectSendRecord(TOPIC, false, emptyHeaders()); + private void verifyTaskGetTopic() { + verifyTaskGetTopic(1); } + private void verifyTaskGetTopic(int times) { + ArgumentCaptor connectorCapture = ArgumentCaptor.forClass(String.class); + ArgumentCaptor topicCapture = ArgumentCaptor.forClass(String.class); + verify(statusBackingStore, times(times)).getTopic(connectorCapture.capture(), topicCapture.capture()); - private void expectTaskGetTopic(boolean anyTimes) { - final Capture connectorCapture = EasyMock.newCapture(); - final Capture topicCapture = EasyMock.newCapture(); - IExpectationSetters expect = EasyMock.expect(statusBackingStore.getTopic( - EasyMock.capture(connectorCapture), - EasyMock.capture(topicCapture))); - if (anyTimes) { - expect.andStubAnswer(() -> new TopicStatus( - topicCapture.getValue(), - new ConnectorTaskId(connectorCapture.getValue(), 0), - Time.SYSTEM.milliseconds())); - } else { - expect.andAnswer(() -> new TopicStatus( - topicCapture.getValue(), - new ConnectorTaskId(connectorCapture.getValue(), 0), - Time.SYSTEM.milliseconds())); - } - if (connectorCapture.hasCaptured() && topicCapture.hasCaptured()) { - assertEquals("job", connectorCapture.getValue()); - assertEquals(TOPIC, topicCapture.getValue()); - } + assertEquals("job", connectorCapture.getValue()); + assertEquals(TOPIC, topicCapture.getValue()); } + @SuppressWarnings("SameParameterValue") private void expectTopicCreation(String topic) { - if (config.topicCreationEnable()) { - EasyMock.expect(admin.describeTopics(topic)).andReturn(Collections.emptyMap()); - Capture newTopicCapture = EasyMock.newCapture(); - EasyMock.expect(admin.createOrFindTopics(EasyMock.capture(newTopicCapture))).andReturn(createdTopic(topic)); - } + when(admin.createOrFindTopics(any(NewTopic.class))).thenReturn(createdTopic(topic)); + } + + private void verifyTopicCreation() { + verifyTopicCreation(1, TOPIC); + } + private void verifyTopicCreation(int times, String... topics) { + ArgumentCaptor newTopicCapture = ArgumentCaptor.forClass(NewTopic.class); + + verify(admin, times(times)).createOrFindTopics(newTopicCapture.capture()); + assertArrayEquals(topics, newTopicCapture.getAllValues() + .stream() + .map(NewTopic::name) + .toArray(String[]::new)); } + @SuppressWarnings("SameParameterValue") private TopicAdmin.TopicCreationResponse createdTopic(String topic) { Set created = Collections.singleton(topic); Set existing = Collections.emptySet(); return new TopicAdmin.TopicCreationResponse(created, existing); } + @SuppressWarnings("SameParameterValue") private TopicAdmin.TopicCreationResponse foundTopic(String topic) { Set created = Collections.emptySet(); Set existing = Collections.singleton(topic); return new TopicAdmin.TopicCreationResponse(created, existing); } - private void expectPreliminaryCalls() { - expectPreliminaryCalls(TOPIC); - } - private void expectPreliminaryCalls(String topic) { - expectConvertHeadersAndKeyValue(topic, true, emptyHeaders()); - expectApplyTransformationChain(false); - PowerMock.expectLastCall(); - } - - private void expectConvertHeadersAndKeyValue(String topic, boolean anyTimes, Headers headers) { - for (Header header : headers) { - IExpectationSetters convertHeaderExpect = EasyMock.expect(headerConverter.fromConnectHeader(topic, header.key(), Schema.STRING_SCHEMA, new String(header.value()))); - if (anyTimes) - convertHeaderExpect.andStubReturn(header.value()); - else - convertHeaderExpect.andReturn(header.value()); + expectConvertHeadersAndKeyValue(emptyHeaders(), topic); + expectApplyTransformationChain(); + } + + private void expectConvertHeadersAndKeyValue(Headers headers, String topic) { + if (headers.iterator().hasNext()) { + when(headerConverter.fromConnectHeader(anyString(), anyString(), eq(Schema.STRING_SCHEMA), + anyString())) + .thenAnswer((Answer) invocation -> { + String headerValue = invocation.getArgument(3, String.class); + return headerValue.getBytes(StandardCharsets.UTF_8); + }); } - IExpectationSetters convertKeyExpect = EasyMock.expect(keyConverter.fromConnectData(topic, headers, KEY_SCHEMA, KEY)); - if (anyTimes) - convertKeyExpect.andStubReturn(SERIALIZED_KEY); - else - convertKeyExpect.andReturn(SERIALIZED_KEY); - IExpectationSetters convertValueExpect = EasyMock.expect(valueConverter.fromConnectData(topic, headers, RECORD_SCHEMA, RECORD)); - if (anyTimes) - convertValueExpect.andStubReturn(SERIALIZED_RECORD); - else - convertValueExpect.andReturn(SERIALIZED_RECORD); - } - - private void expectApplyTransformationChain(boolean anyTimes) { - final Capture recordCapture = EasyMock.newCapture(); - IExpectationSetters convertKeyExpect = EasyMock.expect(transformationChain.apply(EasyMock.capture(recordCapture))); - if (anyTimes) - convertKeyExpect.andStubAnswer(recordCapture::getValue); - else - convertKeyExpect.andAnswer(recordCapture::getValue); + + when(keyConverter.fromConnectData(eq(topic), any(Headers.class), eq(KEY_SCHEMA), eq(KEY))) + .thenReturn(SERIALIZED_KEY); + when(valueConverter.fromConnectData(eq(topic), any(Headers.class), eq(RECORD_SCHEMA), + eq(RECORD))) + .thenReturn(SERIALIZED_RECORD); + } + + private void expectApplyTransformationChain() { + when(transformationChain.apply(any(SourceRecord.class))) + .thenAnswer(AdditionalAnswers.returnsFirstArg()); } private RecordHeaders emptyHeaders() { @@ -839,7 +815,5 @@ protected void producerSendFailed(boolean synchronous, ProducerRecord maybeAdvanceState(TopicPartition topicPartition, + PartitionFetchState currentFetchState) { + return Optional.empty(); + } +} diff --git a/core/src/main/java/kafka/server/ReplicaFetcherTierStateMachine.java b/core/src/main/java/kafka/server/ReplicaFetcherTierStateMachine.java new file mode 100644 index 0000000000000..7cebaae8fe649 --- /dev/null +++ b/core/src/main/java/kafka/server/ReplicaFetcherTierStateMachine.java @@ -0,0 +1,266 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package kafka.server; + +import java.io.BufferedReader; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import kafka.cluster.Partition; +import kafka.log.LeaderOffsetIncremented$; +import kafka.log.UnifiedLog; +import kafka.log.remote.RemoteLogManager; +import org.apache.kafka.common.KafkaException; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.message.FetchResponseData.PartitionData; +import org.apache.kafka.common.message.OffsetForLeaderEpochResponseData.EpochEndOffset; +import org.apache.kafka.common.message.OffsetForLeaderEpochRequestData.OffsetForLeaderPartition; +import org.apache.kafka.common.protocol.Errors; +import org.apache.kafka.common.utils.Utils; +import org.apache.kafka.server.common.CheckpointFile; +import org.apache.kafka.server.common.OffsetAndEpoch; +import org.apache.kafka.server.log.remote.storage.RemoteLogSegmentMetadata; +import org.apache.kafka.server.log.remote.storage.RemoteStorageException; +import org.apache.kafka.server.log.remote.storage.RemoteStorageManager; +import org.apache.kafka.storage.internals.checkpoint.LeaderEpochCheckpointFile; +import org.apache.kafka.storage.internals.log.EpochEntry; +import org.apache.kafka.storage.internals.log.LogFileUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import scala.Option; +import scala.collection.JavaConverters; + +/** + The replica fetcher tier state machine follows a state machine progression. + + Currently, the tier state machine follows a synchronous execution, and we only need to start the machine. + There is no need to advance the state. + + When started, the tier state machine will fetch the local log start offset of the + leader and then build the follower's remote log aux state until the leader's + local log start offset. + */ +public class ReplicaFetcherTierStateMachine implements TierStateMachine { + private static final Logger log = LoggerFactory.getLogger(ReplicaFetcherTierStateMachine.class); + + private LeaderEndPoint leader; + private ReplicaManager replicaMgr; + + public ReplicaFetcherTierStateMachine(LeaderEndPoint leader, + ReplicaManager replicaMgr) { + this.leader = leader; + this.replicaMgr = replicaMgr; + } + + + /** + * Start the tier state machine for the provided topic partition. Currently, this start method will build the + * entire remote aux log state synchronously. + * + * @param topicPartition the topic partition + * @param currentFetchState the current PartitionFetchState which will + * be used to derive the return value + * @param fetchPartitionData the data from the fetch response that returned the offset moved to tiered storage error + * + * @return the new PartitionFetchState after the successful start of the + * tier state machine + */ + public PartitionFetchState start(TopicPartition topicPartition, + PartitionFetchState currentFetchState, + PartitionData fetchPartitionData) throws Exception { + + OffsetAndEpoch epochAndLeaderLocalStartOffset = leader.fetchEarliestLocalOffset(topicPartition, currentFetchState.currentLeaderEpoch()); + int epoch = epochAndLeaderLocalStartOffset.leaderEpoch(); + long leaderLocalStartOffset = epochAndLeaderLocalStartOffset.offset(); + + long offsetToFetch = buildRemoteLogAuxState(topicPartition, currentFetchState.currentLeaderEpoch(), leaderLocalStartOffset, epoch, fetchPartitionData.logStartOffset()); + + OffsetAndEpoch fetchLatestOffsetResult = leader.fetchLatestOffset(topicPartition, currentFetchState.currentLeaderEpoch()); + long leaderEndOffset = fetchLatestOffsetResult.offset(); + + long initialLag = leaderEndOffset - offsetToFetch; + + return PartitionFetchState.apply(currentFetchState.topicId(), offsetToFetch, Option.apply(initialLag), currentFetchState.currentLeaderEpoch(), + Fetching$.MODULE$, replicaMgr.localLogOrException(topicPartition).latestEpoch()); + } + + /** + * This is currently a no-op but will be used for implementing async tiering logic in KAFKA-13560. + * + * @param topicPartition the topic partition + * @param currentFetchState the current PartitionFetchState which will + * be used to derive the return value + * + * @return the original PartitionFetchState + */ + public Optional maybeAdvanceState(TopicPartition topicPartition, + PartitionFetchState currentFetchState) { + // No-op for now + return Optional.of(currentFetchState); + } + + private EpochEndOffset fetchEarlierEpochEndOffset(Integer epoch, + TopicPartition partition, + Integer currentLeaderEpoch) { + int previousEpoch = epoch - 1; + + // Find the end-offset for the epoch earlier to the given epoch from the leader + Map partitionsWithEpochs = new HashMap<>(); + partitionsWithEpochs.put(partition, new OffsetForLeaderPartition().setPartition(partition.partition()).setCurrentLeaderEpoch(currentLeaderEpoch).setLeaderEpoch(previousEpoch)); + Option maybeEpochEndOffset = leader.fetchEpochEndOffsets(JavaConverters.mapAsScalaMap(partitionsWithEpochs)).get(partition); + if (maybeEpochEndOffset.isEmpty()) { + throw new KafkaException("No response received for partition: " + partition); + } + + EpochEndOffset epochEndOffset = maybeEpochEndOffset.get(); + if (epochEndOffset.errorCode() != Errors.NONE.code()) { + throw Errors.forCode(epochEndOffset.errorCode()).exception(); + } + + return epochEndOffset; + } + + private List readLeaderEpochCheckpoint(RemoteLogManager rlm, + RemoteLogSegmentMetadata remoteLogSegmentMetadata) throws IOException, RemoteStorageException { + InputStream inputStream = rlm.storageManager().fetchIndex(remoteLogSegmentMetadata, RemoteStorageManager.IndexType.LEADER_EPOCH); + try (BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) { + CheckpointFile.CheckpointReadBuffer readBuffer = new CheckpointFile.CheckpointReadBuffer<>("", bufferedReader, 0, LeaderEpochCheckpointFile.FORMATTER); + return readBuffer.read(); + } + } + + private void buildProducerSnapshotFile(File snapshotFile, + RemoteLogSegmentMetadata remoteLogSegmentMetadata, + RemoteLogManager rlm) throws IOException, RemoteStorageException { + File tmpSnapshotFile = new File(snapshotFile.getAbsolutePath() + ".tmp"); + // Copy it to snapshot file in atomic manner. + Files.copy(rlm.storageManager().fetchIndex(remoteLogSegmentMetadata, RemoteStorageManager.IndexType.PRODUCER_SNAPSHOT), + tmpSnapshotFile.toPath(), StandardCopyOption.REPLACE_EXISTING); + Utils.atomicMoveWithFallback(tmpSnapshotFile.toPath(), snapshotFile.toPath(), false); + } + + /** + * It tries to build the required state for this partition from leader and remote storage so that it can start + * fetching records from the leader. The return value is the next offset to fetch from the leader, which is the + * next offset following the end offset of the remote log portion. + */ + private Long buildRemoteLogAuxState(TopicPartition topicPartition, + Integer currentLeaderEpoch, + Long leaderLocalLogStartOffset, + Integer epochForLeaderLocalLogStartOffset, + Long leaderLogStartOffset) throws IOException, RemoteStorageException { + + UnifiedLog unifiedLog = replicaMgr.localLogOrException(topicPartition); + + long nextOffset; + + if (unifiedLog.remoteStorageSystemEnable() && unifiedLog.config().remoteLogConfig.remoteStorageEnable) { + if (replicaMgr.remoteLogManager().isEmpty()) throw new IllegalStateException("RemoteLogManager is not yet instantiated"); + + RemoteLogManager rlm = replicaMgr.remoteLogManager().get(); + + // Find the respective leader epoch for (leaderLocalLogStartOffset - 1). We need to build the leader epoch cache + // until that offset + long previousOffsetToLeaderLocalLogStartOffset = leaderLocalLogStartOffset - 1; + int targetEpoch; + // If the existing epoch is 0, no need to fetch from earlier epoch as the desired offset(leaderLogStartOffset - 1) + // will have the same epoch. + if (epochForLeaderLocalLogStartOffset == 0) { + targetEpoch = epochForLeaderLocalLogStartOffset; + } else { + // Fetch the earlier epoch/end-offset(exclusive) from the leader. + EpochEndOffset earlierEpochEndOffset = fetchEarlierEpochEndOffset(epochForLeaderLocalLogStartOffset, topicPartition, currentLeaderEpoch); + // Check if the target offset lies within the range of earlier epoch. Here, epoch's end-offset is exclusive. + if (earlierEpochEndOffset.endOffset() > previousOffsetToLeaderLocalLogStartOffset) { + // Always use the leader epoch from returned earlierEpochEndOffset. + // This gives the respective leader epoch, that will handle any gaps in epochs. + // For ex, leader epoch cache contains: + // leader-epoch start-offset + // 0 20 + // 1 85 + // <2> - gap no messages were appended in this leader epoch. + // 3 90 + // 4 98 + // There is a gap in leader epoch. For leaderLocalLogStartOffset as 90, leader-epoch is 3. + // fetchEarlierEpochEndOffset(2) will return leader-epoch as 1, end-offset as 90. + // So, for offset 89, we should return leader epoch as 1 like below. + targetEpoch = earlierEpochEndOffset.leaderEpoch(); + } else { + targetEpoch = epochForLeaderLocalLogStartOffset; + } + } + + Optional maybeRlsm = rlm.fetchRemoteLogSegmentMetadata(topicPartition, targetEpoch, previousOffsetToLeaderLocalLogStartOffset); + + if (maybeRlsm.isPresent()) { + RemoteLogSegmentMetadata remoteLogSegmentMetadata = maybeRlsm.get(); + // Build leader epoch cache, producer snapshots until remoteLogSegmentMetadata.endOffset() and start + // segments from (remoteLogSegmentMetadata.endOffset() + 1) + // Assign nextOffset with the offset from which next fetch should happen. + nextOffset = remoteLogSegmentMetadata.endOffset() + 1; + + // Truncate the existing local log before restoring the leader epoch cache and producer snapshots. + Partition partition = replicaMgr.getPartitionOrException(topicPartition); + partition.truncateFullyAndStartAt(nextOffset, false); + + // Build leader epoch cache. + unifiedLog.maybeIncrementLogStartOffset(leaderLogStartOffset, LeaderOffsetIncremented$.MODULE$); + List epochs = readLeaderEpochCheckpoint(rlm, remoteLogSegmentMetadata); + if (unifiedLog.leaderEpochCache().isDefined()) { + unifiedLog.leaderEpochCache().get().assign(epochs); + } + + log.debug("Updated the epoch cache from remote tier till offset: {} with size: {} for {}", leaderLocalLogStartOffset, epochs.size(), partition); + + // Restore producer snapshot + File snapshotFile = LogFileUtils.producerSnapshotFile(unifiedLog.dir(), nextOffset); + buildProducerSnapshotFile(snapshotFile, remoteLogSegmentMetadata, rlm); + + // Reload producer snapshots. + unifiedLog.producerStateManager().truncateFullyAndReloadSnapshots(); + unifiedLog.loadProducerState(nextOffset); + log.debug("Built the leader epoch cache and producer snapshots from remote tier for {}, " + + "with active producers size: {}, leaderLogStartOffset: {}, and logEndOffset: {}", + partition, unifiedLog.producerStateManager().activeProducers().size(), leaderLogStartOffset, nextOffset); + } else { + throw new RemoteStorageException("Couldn't build the state from remote store for partition: " + topicPartition + + ", currentLeaderEpoch: " + currentLeaderEpoch + + ", leaderLocalLogStartOffset: " + leaderLocalLogStartOffset + + ", leaderLogStartOffset: " + leaderLogStartOffset + + ", epoch: " + targetEpoch + + "as the previous remote log segment metadata was not found"); + } + } else { + // If the tiered storage is not enabled throw an exception back so that it will retry until the tiered storage + // is set as expected. + throw new RemoteStorageException("Couldn't build the state from remote store for partition " + topicPartition + ", as remote log storage is not yet enabled"); + } + + return nextOffset; + } +} diff --git a/core/src/main/java/kafka/server/TierStateMachine.java b/core/src/main/java/kafka/server/TierStateMachine.java new file mode 100644 index 0000000000000..58a44cc647232 --- /dev/null +++ b/core/src/main/java/kafka/server/TierStateMachine.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package kafka.server; + +import java.util.Optional; + +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.message.FetchResponseData.PartitionData; + +/** + * This interface defines the APIs needed to handle any state transitions related to tiering + */ +public interface TierStateMachine { + + /** + * Start the tier state machine for the provided topic partition. + * + * @param topicPartition the topic partition + * @param currentFetchState the current PartitionFetchState which will + * be used to derive the return value + * @param fetchPartitionData the data from the fetch response that returned the offset moved to tiered storage error + * + * @return the new PartitionFetchState after the successful start of the + * tier state machine + */ + PartitionFetchState start(TopicPartition topicPartition, + PartitionFetchState currentFetchState, + PartitionData fetchPartitionData) throws Exception; + + /** + * Optionally advance the state of the tier state machine, based on the + * current PartitionFetchState. The decision to advance the tier + * state machine is implementation specific. + * + * @param topicPartition the topic partition + * @param currentFetchState the current PartitionFetchState which will + * be used to derive the return value + * + * @return the new PartitionFetchState if the tier state machine was advanced, otherwise, return the currentFetchState + */ + Optional maybeAdvanceState(TopicPartition topicPartition, + PartitionFetchState currentFetchState); +} diff --git a/core/src/main/scala/kafka/network/SocketServer.scala b/core/src/main/scala/kafka/network/SocketServer.scala index dca6c10837476..6807a2a82fcbb 100644 --- a/core/src/main/scala/kafka/network/SocketServer.scala +++ b/core/src/main/scala/kafka/network/SocketServer.scala @@ -1175,10 +1175,11 @@ private[kafka] class Processor( val response = inflightResponses.remove(send.destinationId).getOrElse { throw new IllegalStateException(s"Send for ${send.destinationId} completed, but not in `inflightResponses`") } - updateRequestMetrics(response) - - // Invoke send completion callback + + // Invoke send completion callback, and then update request metrics since there might be some + // request metrics got updated during callback response.onComplete.foreach(onComplete => onComplete(send)) + updateRequestMetrics(response) // Try unmuting the channel. If there was no quota violation and the channel has not been throttled, // it will be unmuted immediately. If the channel has been throttled, it will unmuted only if the throttling diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index 25f2e292cc537..2176ee3518bcc 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -26,6 +26,7 @@ import kafka.utils.{DelayedItem, Logging, Pool} import org.apache.kafka.common.errors._ import org.apache.kafka.common.internals.PartitionStates import org.apache.kafka.common.message.OffsetForLeaderEpochResponseData.EpochEndOffset +import org.apache.kafka.common.message.FetchResponseData.PartitionData import org.apache.kafka.common.message.{FetchResponseData, OffsetForLeaderEpochRequestData} import org.apache.kafka.common.protocol.Errors import org.apache.kafka.common.record.{FileRecords, MemoryRecords, Records} @@ -54,6 +55,7 @@ abstract class AbstractFetcherThread(name: String, clientId: String, val leader: LeaderEndPoint, failedPartitions: FailedPartitions, + val fetchTierStateMachine: TierStateMachine, fetchBackOffMs: Int = 0, isInterruptible: Boolean = true, val brokerTopicStats: BrokerTopicStats) //BrokerTopicStats's lifecycle managed by ReplicaManager @@ -93,22 +95,6 @@ abstract class AbstractFetcherThread(name: String, protected val isOffsetForLeaderEpochSupported: Boolean - /** - * Builds the required remote log auxiliary state for the given topic partition on this follower replica and returns - * the offset to be fetched from the leader replica. - * - * @param partition topic partition - * @param currentLeaderEpoch current leader epoch maintained by this follower replica. - * @param fetchOffset offset to be fetched from the leader. - * @param epochForFetchOffset respective leader epoch for the given fetch pffset. - * @param leaderLogStartOffset log-start-offset on the leader. - */ - protected def buildRemoteLogAuxState(partition: TopicPartition, - currentLeaderEpoch: Int, - fetchOffset: Long, - epochForFetchOffset: Int, - leaderLogStartOffset: Long): Long - override def shutdown(): Unit = { initiateShutdown() inLock(partitionMapLock) { @@ -410,12 +396,7 @@ abstract class AbstractFetcherThread(name: String, case Errors.OFFSET_OUT_OF_RANGE => if (!handleOutOfRangeError(topicPartition, currentFetchState, fetchPartitionData.currentLeaderEpoch)) partitionsWithError += topicPartition - case Errors.OFFSET_MOVED_TO_TIERED_STORAGE => - debug(s"Received error ${Errors.OFFSET_MOVED_TO_TIERED_STORAGE}, " + - s"at fetch offset: ${currentFetchState.fetchOffset}, " + s"topic-partition: $topicPartition") - if (!handleOffsetsMovedToTieredStorage(topicPartition, currentFetchState, - fetchPartitionData.currentLeaderEpoch, partitionData.logStartOffset())) - partitionsWithError += topicPartition + case Errors.UNKNOWN_LEADER_EPOCH => debug(s"Remote broker has a smaller leader epoch for partition $topicPartition than " + s"this replica's current leader epoch of ${currentFetchState.currentLeaderEpoch}.") @@ -425,6 +406,12 @@ abstract class AbstractFetcherThread(name: String, if (onPartitionFenced(topicPartition, fetchPartitionData.currentLeaderEpoch)) partitionsWithError += topicPartition + case Errors.OFFSET_MOVED_TO_TIERED_STORAGE => + debug(s"Received error ${Errors.OFFSET_MOVED_TO_TIERED_STORAGE}, " + + s"at fetch offset: ${currentFetchState.fetchOffset}, " + s"topic-partition: $topicPartition") + if (!handleOffsetsMovedToTieredStorage(topicPartition, currentFetchState, fetchPartitionData.currentLeaderEpoch, partitionData)) + partitionsWithError += topicPartition + case Errors.NOT_LEADER_OR_FOLLOWER => debug(s"Remote broker is not the leader for partition $topicPartition, which could indicate " + "that the partition is being moved") @@ -644,25 +631,9 @@ abstract class AbstractFetcherThread(name: String, } /** - * It returns the next fetch state. It fetches the log-start-offset or local-log-start-offset based on - * `fetchFromLocalLogStartOffset` flag. This is used in truncation by passing it to the given `truncateAndBuild` - * function. - * - * @param topicPartition topic partition - * @param topicId topic id - * @param currentLeaderEpoch current leader epoch maintained by this follower replica. - * @param truncateAndBuild Function to truncate for the given epoch and offset. It returns the next fetch offset value. - * @param fetchFromLocalLogStartOffset Whether to fetch from local-log-start-offset or log-start-offset. If true, it - * requests the local-log-start-offset from the leader, else it requests - * log-start-offset from the leader. This is used in sending the value to the - * given `truncateAndBuild` function. - * @return next PartitionFetchState + * Handle a partition whose offset is out of range and return a new fetch offset. */ - private def fetchOffsetAndApplyTruncateAndBuild(topicPartition: TopicPartition, - topicId: Option[Uuid], - currentLeaderEpoch: Int, - truncateAndBuild: => OffsetAndEpoch => Long, - fetchFromLocalLogStartOffset: Boolean = true): PartitionFetchState = { + private def fetchOffsetAndTruncate(topicPartition: TopicPartition, topicId: Option[Uuid], currentLeaderEpoch: Int): PartitionFetchState = { val replicaEndOffset = logEndOffset(topicPartition) /** @@ -696,33 +667,25 @@ abstract class AbstractFetcherThread(name: String, * produced to the new leader. While the old leader is trying to handle the OffsetOutOfRangeException and query * the log end offset of the new leader, the new leader's log end offset becomes higher than the follower's log end offset. * - * In the first case, the follower's current log end offset is smaller than the leader's log start offset - * (or leader's local log start offset). - * So the follower should truncate all its logs, roll out a new segment and start to fetch from the current - * leader's log start offset(or leader's local log start offset). + * In the first case, if the follower's current log end offset is smaller than the leader's log start offset, the + * follower should truncate all its logs, roll out a new segment and start to fetch from the current leader's log + * start offset since the data are all stale. * In the second case, the follower should just keep the current log segments and retry the fetch. In the second * case, there will be some inconsistency of data between old and new leader. We are not solving it here. * If users want to have strong consistency guarantees, appropriate configurations needs to be set for both * brokers and producers. * * Putting the two cases together, the follower should fetch from the higher one of its replica log end offset - * and the current leader's (local-log-start-offset or) log start offset. + * and the current leader's log start offset. */ - val offsetAndEpoch = if (fetchFromLocalLogStartOffset) - leader.fetchEarliestLocalOffset(topicPartition, currentLeaderEpoch) else - leader.fetchEarliestOffset(topicPartition, currentLeaderEpoch) + val offsetAndEpoch = leader.fetchEarliestOffset(topicPartition, currentLeaderEpoch) val leaderStartOffset = offsetAndEpoch.offset warn(s"Reset fetch offset for partition $topicPartition from $replicaEndOffset to current " + s"leader's start offset $leaderStartOffset") - val offsetToFetch = - if (leaderStartOffset > replicaEndOffset) { - // Only truncate log when current leader's log start offset (local log start offset if >= 3.4 version incaseof - // OffsetMovedToTieredStorage error) is greater than follower's log end offset. - // truncateAndBuild returns offset value from which it needs to start fetching. - truncateAndBuild(offsetAndEpoch) - } else { - replicaEndOffset - } + val offsetToFetch = Math.max(leaderStartOffset, replicaEndOffset) + // Only truncate log when current leader's log start offset is greater than follower's log end offset. + if (leaderStartOffset > replicaEndOffset) + truncateFullyAndStartAt(topicPartition, leaderStartOffset) val initialLag = leaderEndOffset - offsetToFetch fetcherLagStats.getAndMaybePut(topicPartition).lag = initialLag @@ -731,23 +694,6 @@ abstract class AbstractFetcherThread(name: String, } } - /** - * Handle a partition whose offset is out of range and return a new fetch offset. - */ - private def fetchOffsetAndTruncate(topicPartition: TopicPartition, topicId: Option[Uuid], currentLeaderEpoch: Int): PartitionFetchState = { - fetchOffsetAndApplyTruncateAndBuild(topicPartition, topicId, currentLeaderEpoch, - offsetAndEpoch => { - val leaderLogStartOffset = offsetAndEpoch.offset - truncateFullyAndStartAt(topicPartition, leaderLogStartOffset) - leaderLogStartOffset - }, - // In this case, it will fetch from leader's log-start-offset like earlier instead of fetching from - // local-log-start-offset. This handles both the scenarios of whether tiered storage is enabled or not. - // If tiered storage is enabled, the next fetch result of fetching from log-start-offset may result in - // OffsetMovedToTieredStorage error and it will handle building the remote log state. - fetchFromLocalLogStartOffset = false) - } - /** * Handles the out of range error for the given topic partition. * @@ -798,21 +744,20 @@ abstract class AbstractFetcherThread(name: String, * Returns false if there was a retriable error. * * @param topicPartition topic partition - * @param fetchState current partition fetch state. - * @param leaderEpochInRequest current leader epoch sent in the fetch request. - * @param leaderLogStartOffset log-start-offset in the leader replica. + * @param fetchState current partition fetch state + * @param leaderEpochInRequest current leader epoch sent in the fetch request + * @param fetchPartitionData the fetch response data for this topic partition */ private def handleOffsetsMovedToTieredStorage(topicPartition: TopicPartition, fetchState: PartitionFetchState, leaderEpochInRequest: Optional[Integer], - leaderLogStartOffset: Long): Boolean = { + fetchPartitionData: PartitionData): Boolean = { try { - val newFetchState = fetchOffsetAndApplyTruncateAndBuild(topicPartition, fetchState.topicId, fetchState.currentLeaderEpoch, - offsetAndEpoch => { - val leaderLocalLogStartOffset = offsetAndEpoch.offset - buildRemoteLogAuxState(topicPartition, fetchState.currentLeaderEpoch, leaderLocalLogStartOffset, offsetAndEpoch.leaderEpoch(), leaderLogStartOffset) - }) + val newFetchState = fetchTierStateMachine.start(topicPartition, fetchState, fetchPartitionData); + + // TODO: use fetchTierStateMachine.maybeAdvanceState when implementing async tiering logic in KAFKA-13560 + fetcherLagStats.getAndMaybePut(topicPartition).lag = newFetchState.lag.getOrElse(0) partitionStates.updateAndMoveToEnd(topicPartition, newFetchState) debug(s"Current offset ${fetchState.fetchOffset} for partition $topicPartition is " + s"out of range or moved to remote tier. Reset fetch offset to ${newFetchState.fetchOffset}") diff --git a/core/src/main/scala/kafka/server/KafkaConfig.scala b/core/src/main/scala/kafka/server/KafkaConfig.scala index 1130cb5a12ac4..6f1aa52e8fa92 100755 --- a/core/src/main/scala/kafka/server/KafkaConfig.scala +++ b/core/src/main/scala/kafka/server/KafkaConfig.scala @@ -888,7 +888,7 @@ object KafkaConfig { val ReplicaLagTimeMaxMsDoc = "If a follower hasn't sent any fetch requests or hasn't consumed up to the leaders log end offset for at least this time," + " the leader will remove the follower from isr" val ReplicaSocketTimeoutMsDoc = "The socket timeout for network requests. Its value should be at least replica.fetch.wait.max.ms" - val ReplicaSocketReceiveBufferBytesDoc = "The socket receive buffer for network requests" + val ReplicaSocketReceiveBufferBytesDoc = "The socket receive buffer for network requests to the leader for replicating data" val ReplicaFetchMaxBytesDoc = "The number of bytes of messages to attempt to fetch for each partition. This is not an absolute maximum, " + "if the first record batch in the first non-empty partition of the fetch is larger than this value, the record batch will still be returned " + "to ensure that progress can be made. The maximum record batch size accepted by the broker is defined via " + diff --git a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala index e003ae1c76f53..cae9193fba1bc 100644 --- a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala @@ -36,6 +36,7 @@ class ReplicaAlterLogDirsThread(name: String, clientId = name, leader = leader, failedPartitions, + fetchTierStateMachine = new ReplicaAlterLogDirsTierStateMachine(), fetchBackOffMs = fetchBackOffMs, isInterruptible = false, brokerTopicStats) { @@ -122,14 +123,4 @@ class ReplicaAlterLogDirsThread(name: String, val partition = replicaMgr.getPartitionOrException(topicPartition) partition.truncateFullyAndStartAt(offset, isFuture = true) } - - override protected def buildRemoteLogAuxState(partition: TopicPartition, - currentLeaderEpoch: Int, - fetchOffset: Long, - epochForFetchOffset: Int, - leaderLogStartOffset: Long): Long = { - // JBOD is not supported with tiered storage. - throw new UnsupportedOperationException(); - } - } diff --git a/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala index 4a653c435524d..ae75fb571b406 100644 --- a/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala @@ -17,24 +17,14 @@ package kafka.server -import kafka.log.remote.RemoteLogManager import kafka.log.LeaderOffsetIncremented -import org.apache.kafka.common.message.OffsetForLeaderEpochResponseData.EpochEndOffset -import org.apache.kafka.common.protocol.Errors import org.apache.kafka.common.record.MemoryRecords -import org.apache.kafka.common.requests._ -import org.apache.kafka.common.utils.Utils -import org.apache.kafka.common.{KafkaException, TopicPartition} -import org.apache.kafka.server.common.CheckpointFile.CheckpointReadBuffer +import org.apache.kafka.common.requests.FetchResponse +import org.apache.kafka.common.TopicPartition import org.apache.kafka.server.common.OffsetAndEpoch import org.apache.kafka.server.common.MetadataVersion -import org.apache.kafka.server.log.remote.storage.{RemoteLogSegmentMetadata, RemoteStorageException, RemoteStorageManager} -import org.apache.kafka.storage.internals.checkpoint.LeaderEpochCheckpointFile -import org.apache.kafka.storage.internals.log.{EpochEntry, LogAppendInfo, LogFileUtils} +import org.apache.kafka.storage.internals.log.LogAppendInfo -import java.io.{BufferedReader, File, InputStreamReader} -import java.nio.charset.StandardCharsets -import java.nio.file.{Files, StandardCopyOption} import scala.collection.mutable class ReplicaFetcherThread(name: String, @@ -49,6 +39,7 @@ class ReplicaFetcherThread(name: String, clientId = name, leader = leader, failedPartitions, + fetchTierStateMachine = new ReplicaFetcherTierStateMachine(leader, replicaMgr), fetchBackOffMs = brokerConfig.replicaFetchBackoffMs, isInterruptible = false, replicaMgr.brokerTopicStats) { @@ -203,141 +194,4 @@ class ReplicaFetcherThread(name: String, val partition = replicaMgr.getPartitionOrException(topicPartition) partition.truncateFullyAndStartAt(offset, isFuture = false) } - - private def buildProducerSnapshotFile(snapshotFile: File, remoteLogSegmentMetadata: RemoteLogSegmentMetadata, rlm: RemoteLogManager): Unit = { - val tmpSnapshotFile = new File(snapshotFile.getAbsolutePath + ".tmp") - // Copy it to snapshot file in atomic manner. - Files.copy(rlm.storageManager().fetchIndex(remoteLogSegmentMetadata, RemoteStorageManager.IndexType.PRODUCER_SNAPSHOT), - tmpSnapshotFile.toPath, StandardCopyOption.REPLACE_EXISTING) - Utils.atomicMoveWithFallback(tmpSnapshotFile.toPath, snapshotFile.toPath, false) - } - - /** - * It tries to build the required state for this partition from leader and remote storage so that it can start - * fetching records from the leader. - */ - override protected def buildRemoteLogAuxState(partition: TopicPartition, - currentLeaderEpoch: Int, - leaderLocalLogStartOffset: Long, - epochForLeaderLocalLogStartOffset: Int, - leaderLogStartOffset: Long): Long = { - - def fetchEarlierEpochEndOffset(epoch: Int): EpochEndOffset = { - val previousEpoch = epoch - 1 - // Find the end-offset for the epoch earlier to the given epoch from the leader - val partitionsWithEpochs = Map(partition -> new EpochData().setPartition(partition.partition()) - .setCurrentLeaderEpoch(currentLeaderEpoch) - .setLeaderEpoch(previousEpoch)) - val maybeEpochEndOffset = leader.fetchEpochEndOffsets(partitionsWithEpochs).get(partition) - if (maybeEpochEndOffset.isEmpty) { - throw new KafkaException("No response received for partition: " + partition); - } - - val epochEndOffset = maybeEpochEndOffset.get - if (epochEndOffset.errorCode() != Errors.NONE.code()) { - throw Errors.forCode(epochEndOffset.errorCode()).exception() - } - - epochEndOffset - } - - val log = replicaMgr.localLogOrException(partition) - val nextOffset = { - if (log.remoteStorageSystemEnable && log.config.remoteLogConfig.remoteStorageEnable) { - if (replicaMgr.remoteLogManager.isEmpty) throw new IllegalStateException("RemoteLogManager is not yet instantiated") - - val rlm = replicaMgr.remoteLogManager.get - - // Find the respective leader epoch for (leaderLocalLogStartOffset - 1). We need to build the leader epoch cache - // until that offset - val previousOffsetToLeaderLocalLogStartOffset = leaderLocalLogStartOffset - 1 - val targetEpoch: Int = { - // If the existing epoch is 0, no need to fetch from earlier epoch as the desired offset(leaderLogStartOffset - 1) - // will have the same epoch. - if (epochForLeaderLocalLogStartOffset == 0) { - epochForLeaderLocalLogStartOffset - } else { - // Fetch the earlier epoch/end-offset(exclusive) from the leader. - val earlierEpochEndOffset = fetchEarlierEpochEndOffset(epochForLeaderLocalLogStartOffset) - // Check if the target offset lies with in the range of earlier epoch. Here, epoch's end-offset is exclusive. - if (earlierEpochEndOffset.endOffset > previousOffsetToLeaderLocalLogStartOffset) { - // Always use the leader epoch from returned earlierEpochEndOffset. - // This gives the respective leader epoch, that will handle any gaps in epochs. - // For ex, leader epoch cache contains: - // leader-epoch start-offset - // 0 20 - // 1 85 - // <2> - gap no messages were appended in this leader epoch. - // 3 90 - // 4 98 - // There is a gap in leader epoch. For leaderLocalLogStartOffset as 90, leader-epoch is 3. - // fetchEarlierEpochEndOffset(2) will return leader-epoch as 1, end-offset as 90. - // So, for offset 89, we should return leader epoch as 1 like below. - earlierEpochEndOffset.leaderEpoch() - } else epochForLeaderLocalLogStartOffset - } - } - - val maybeRlsm = rlm.fetchRemoteLogSegmentMetadata(partition, targetEpoch, previousOffsetToLeaderLocalLogStartOffset) - - if (maybeRlsm.isPresent) { - val remoteLogSegmentMetadata = maybeRlsm.get() - // Build leader epoch cache, producer snapshots until remoteLogSegmentMetadata.endOffset() and start - // segments from (remoteLogSegmentMetadata.endOffset() + 1) - val nextOffset = remoteLogSegmentMetadata.endOffset() + 1 - - // Truncate the existing local log before restoring the leader epoch cache and producer snapshots. - truncateFullyAndStartAt(partition, nextOffset) - - // Build leader epoch cache. - log.maybeIncrementLogStartOffset(leaderLogStartOffset, LeaderOffsetIncremented) - val epochs = readLeaderEpochCheckpoint(rlm, remoteLogSegmentMetadata) - log.leaderEpochCache.foreach { cache => - cache.assign(epochs) - } - - debug(s"Updated the epoch cache from remote tier till offset: $leaderLocalLogStartOffset " + - s"with size: ${epochs.size} for $partition") - - // Restore producer snapshot - val snapshotFile = LogFileUtils.producerSnapshotFile(log.dir, nextOffset) - buildProducerSnapshotFile(snapshotFile, remoteLogSegmentMetadata, rlm) - - // Reload producer snapshots. - log.producerStateManager.truncateFullyAndReloadSnapshots() - log.loadProducerState(nextOffset) - debug(s"Built the leader epoch cache and producer snapshots from remote tier for $partition, with " + - s"active producers size: ${log.producerStateManager.activeProducers.size}, " + - s"leaderLogStartOffset: $leaderLogStartOffset, and logEndOffset: $nextOffset") - - // Return the offset from which next fetch should happen. - nextOffset - } else { - throw new RemoteStorageException(s"Couldn't build the state from remote store for partition: $partition, " + - s"currentLeaderEpoch: $currentLeaderEpoch, leaderLocalLogStartOffset: $leaderLocalLogStartOffset, " + - s"leaderLogStartOffset: $leaderLogStartOffset, epoch: $targetEpoch as the previous remote log segment " + - s"metadata was not found") - } - } else { - // If the tiered storage is not enabled throw an exception back so tht it will retry until the tiered storage - // is set as expected. - throw new RemoteStorageException(s"Couldn't build the state from remote store for partition $partition, as " + - s"remote log storage is not yet enabled") - } - } - - nextOffset - } - - private def readLeaderEpochCheckpoint(rlm: RemoteLogManager, remoteLogSegmentMetadata: RemoteLogSegmentMetadata): java.util.List[EpochEntry] = { - val inputStream = rlm.storageManager().fetchIndex(remoteLogSegmentMetadata, RemoteStorageManager.IndexType.LEADER_EPOCH) - val bufferedReader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8)) - try { - val readBuffer = new CheckpointReadBuffer[EpochEntry]("", bufferedReader, 0, LeaderEpochCheckpointFile.FORMATTER) - readBuffer.read() - } finally { - bufferedReader.close() - } - } - } diff --git a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala index d363106326c67..661295e280c2b 100644 --- a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala +++ b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala @@ -730,7 +730,6 @@ class DynamicBrokerReconfigurationTest extends QuorumTestHarness with SaslSetup } @Test - @Disabled // TODO: To be re-enabled once we can make it less flaky: KAFKA-8280 def testUncleanLeaderElectionEnable(): Unit = { val controller = servers.find(_.config.brokerId == TestUtils.waitUntilControllerElected(zkClient)).get val controllerId = controller.config.brokerId diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala index 25bf9633438b2..11b7ceb2df4b8 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala @@ -21,6 +21,7 @@ import kafka.cluster.BrokerEndPoint import kafka.server.AbstractFetcherThread.{ReplicaFetch, ResultWithPartitions} import kafka.utils.Implicits.MapExtensionMethods import kafka.utils.TestUtils +import org.apache.kafka.common.message.FetchResponseData.PartitionData import org.apache.kafka.common.message.OffsetForLeaderEpochResponseData.EpochEndOffset import org.apache.kafka.common.requests.FetchRequest import org.apache.kafka.common.utils.Utils @@ -32,6 +33,7 @@ import org.junit.jupiter.api.Assertions._ import org.junit.jupiter.api.{BeforeEach, Test} import org.mockito.Mockito.{mock, verify, when} +import java.util.Optional import scala.collection.{Map, Set, mutable} import scala.jdk.CollectionConverters._ @@ -235,7 +237,7 @@ class AbstractFetcherManagerTest { val failedTopicPartitions = makeTopicPartition(2, 5, "topic_failed") val fetcherManager = new AbstractFetcherManager[AbstractFetcherThread]("fetcher-manager", "fetcher-manager", currentFetcherSize) { override def createFetcherThread(fetcherId: Int, sourceBroker: BrokerEndPoint): AbstractFetcherThread = { - new TestResizeFetcherThread(sourceBroker, failedPartitions) + new TestResizeFetcherThread(sourceBroker, failedPartitions, new MockResizeFetcherTierStateMachine) } } try { @@ -311,12 +313,21 @@ class AbstractFetcherManagerTest { override def fetchEarliestLocalOffset(topicPartition: TopicPartition, currentLeaderEpoch: Int): OffsetAndEpoch = new OffsetAndEpoch(1L, 0) } - private class TestResizeFetcherThread(sourceBroker: BrokerEndPoint, failedPartitions: FailedPartitions) + private class MockResizeFetcherTierStateMachine extends TierStateMachine { + override def start(topicPartition: TopicPartition, currentFetchState: PartitionFetchState, fetchPartitionData: PartitionData): PartitionFetchState = { + throw new UnsupportedOperationException("Materializing tier state is not supported in this test.") + } + + override def maybeAdvanceState(tp: TopicPartition, currentFetchState: PartitionFetchState): Optional[PartitionFetchState] = Optional.empty[PartitionFetchState] + } + + private class TestResizeFetcherThread(sourceBroker: BrokerEndPoint, failedPartitions: FailedPartitions, fetchTierStateMachine: TierStateMachine) extends AbstractFetcherThread( name = "test-resize-fetcher", clientId = "mock-fetcher", leader = new MockLeaderEndPoint(sourceBroker), failedPartitions, + fetchTierStateMachine, fetchBackOffMs = 0, brokerTopicStats = new BrokerTopicStats) { @@ -337,8 +348,6 @@ class AbstractFetcherManagerTest { override protected def endOffsetForEpoch(topicPartition: TopicPartition, epoch: Int): Option[OffsetAndEpoch] = Some(new OffsetAndEpoch(1, 0)) override protected val isOffsetForLeaderEpochSupported: Boolean = false - - override protected def buildRemoteLogAuxState(partition: TopicPartition, currentLeaderEpoch: Int, fetchOffset: Long, epochForFetchOffset: Int, leaderLogStartOffset: Long): Long = 1 } } diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index 9c8d5323c97eb..e959251b6eabb 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -76,7 +76,9 @@ class AbstractFetcherThreadTest { @Test def testMetricsRemovedOnShutdown(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) // add one partition to create the consumer lag metric fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) @@ -104,7 +106,9 @@ class AbstractFetcherThreadTest { @Test def testConsumerLagRemovedWithPartition(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) // add one partition to create the consumer lag metric fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) @@ -127,7 +131,9 @@ class AbstractFetcherThreadTest { @Test def testSimpleFetch(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) fetcher.addPartitions(Map(partition -> initialFetchState(topicIds.get(partition.topic), 0L, leaderEpoch = 0))) @@ -150,11 +156,13 @@ class AbstractFetcherThreadTest { val partition = new TopicPartition("topic", 0) val fetchBackOffMs = 250 - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndpoint = new MockLeaderEndPoint { override def fetch(fetchRequest: FetchRequest.Builder): Map[TopicPartition, FetchData] = { throw new UnknownTopicIdException("Topic ID was unknown as expected for this test") } - }, fetchBackOffMs = fetchBackOffMs) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine, fetchBackOffMs = fetchBackOffMs) fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) fetcher.addPartitions(Map(partition -> initialFetchState(Some(Uuid.randomUuid()), 0L, leaderEpoch = 0))) @@ -190,13 +198,15 @@ class AbstractFetcherThreadTest { val partition3 = new TopicPartition("topic3", 0) val fetchBackOffMs = 250 - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndPoint = new MockLeaderEndPoint { override def fetch(fetchRequest: FetchRequest.Builder): Map[TopicPartition, FetchData] = { Map(partition1 -> new FetchData().setErrorCode(Errors.UNKNOWN_TOPIC_ID.code), partition2 -> new FetchData().setErrorCode(Errors.INCONSISTENT_TOPIC_ID.code), partition3 -> new FetchData().setErrorCode(Errors.NONE.code)) } - }, fetchBackOffMs = fetchBackOffMs) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndPoint) + val fetcher = new MockFetcherThread(mockLeaderEndPoint, mockTierStateMachine, fetchBackOffMs = fetchBackOffMs) fetcher.setReplicaState(partition1, PartitionState(leaderEpoch = 0)) fetcher.addPartitions(Map(partition1 -> initialFetchState(Some(Uuid.randomUuid()), 0L, leaderEpoch = 0))) @@ -231,7 +241,9 @@ class AbstractFetcherThreadTest { @Test def testFencedTruncation(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) fetcher.addPartitions(Map(partition -> initialFetchState(topicIds.get(partition.topic), 0L, leaderEpoch = 0))) @@ -257,7 +269,9 @@ class AbstractFetcherThreadTest { @Test def testFencedFetch(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) val replicaState = PartitionState(leaderEpoch = 0) fetcher.setReplicaState(partition, replicaState) @@ -288,7 +302,9 @@ class AbstractFetcherThreadTest { @Test def testUnknownLeaderEpochInTruncation(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) // The replica's leader epoch is ahead of the leader val replicaState = PartitionState(leaderEpoch = 1) @@ -319,7 +335,9 @@ class AbstractFetcherThreadTest { @Test def testUnknownLeaderEpochWhileFetching(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) // This test is contrived because it shouldn't be possible to to see unknown leader epoch // in the Fetching state as the leader must validate the follower's epoch when it checks @@ -360,7 +378,9 @@ class AbstractFetcherThreadTest { @Test def testTruncation(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) val replicaLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("a".getBytes)), @@ -394,11 +414,14 @@ class AbstractFetcherThreadTest { def testTruncateToHighWatermarkIfLeaderEpochRequestNotSupported(): Unit = { val highWatermark = 2L val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { - override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = - throw new UnsupportedOperationException - override val isTruncationOnFetchSupported: Boolean = false - }) { + val mockLeaderEndPoint = new MockLeaderEndPoint { + override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = + throw new UnsupportedOperationException + + override val isTruncationOnFetchSupported: Boolean = false + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndPoint) + val fetcher = new MockFetcherThread(mockLeaderEndPoint, mockTierStateMachine) { override def truncate(topicPartition: TopicPartition, truncationState: OffsetTruncationState): Unit = { assertEquals(highWatermark, truncationState.offset) assertTrue(truncationState.truncationCompleted) @@ -428,10 +451,12 @@ class AbstractFetcherThreadTest { def testTruncateToHighWatermarkIfLeaderEpochInfoNotAvailable(): Unit = { val highWatermark = 2L val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { - override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = - throw new UnsupportedOperationException - }) { + val mockLeaderEndPoint = new MockLeaderEndPoint { + override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = + throw new UnsupportedOperationException + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndPoint) + val fetcher = new MockFetcherThread(mockLeaderEndPoint, mockTierStateMachine) { override def truncate(topicPartition: TopicPartition, truncationState: OffsetTruncationState): Unit = { assertEquals(highWatermark, truncationState.offset) assertTrue(truncationState.truncationCompleted) @@ -462,7 +487,10 @@ class AbstractFetcherThreadTest { def testTruncateToHighWatermarkDuringRemovePartitions(): Unit = { val highWatermark = 2L val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) { + + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) { override def truncateToHighWatermark(partitions: Set[TopicPartition]): Unit = { removePartitions(Set(partition)) super.truncateToHighWatermark(partitions) @@ -492,7 +520,9 @@ class AbstractFetcherThreadTest { val partition = new TopicPartition("topic", 0) var truncations = 0 - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) { + val mockLeaderEndpoint = new MockLeaderEndPoint() + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) { override def truncate(topicPartition: TopicPartition, truncationState: OffsetTruncationState): Unit = { truncations += 1 super.truncate(topicPartition, truncationState) @@ -535,7 +565,9 @@ class AbstractFetcherThreadTest { assumeTrue(truncateOnFetch) val partition = new TopicPartition("topic", 0) var truncations = 0 - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) { + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) { override def truncate(topicPartition: TopicPartition, truncationState: OffsetTruncationState): Unit = { truncations += 1 super.truncate(topicPartition, truncationState) @@ -575,7 +607,9 @@ class AbstractFetcherThreadTest { @Test def testFollowerFetchOutOfRangeHigh(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) val replicaLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("a".getBytes)), @@ -616,7 +650,6 @@ class AbstractFetcherThreadTest { @Test def testFollowerFetchMovedToTieredStore(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) val replicaLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("a".getBytes)), @@ -624,6 +657,11 @@ class AbstractFetcherThreadTest { mkBatch(baseOffset = 2, leaderEpoch = 4, new SimpleRecord("c".getBytes))) val replicaState = PartitionState(replicaLog, leaderEpoch = 5, highWatermark = 0L, rlmEnabled = true) + + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + fetcher.setReplicaState(partition, replicaState) fetcher.addPartitions(Map(partition -> initialFetchState(topicIds.get(partition.topic), 3L, leaderEpoch = 5))) @@ -633,7 +671,6 @@ class AbstractFetcherThreadTest { mkBatch(baseOffset = 7, leaderEpoch = 5, new SimpleRecord("h".getBytes)), mkBatch(baseOffset = 8, leaderEpoch = 5, new SimpleRecord("i".getBytes))) - val leaderState = PartitionState(leaderLog, leaderEpoch = 5, highWatermark = 8L, rlmEnabled = true) // Overriding the log start offset to zero for mocking the scenario of segment 0-4 moved to remote store. leaderState.logStartOffset = 0 @@ -664,16 +701,14 @@ class AbstractFetcherThreadTest { def testFencedOffsetResetAfterMovedToRemoteTier(): Unit = { val partition = new TopicPartition("topic", 0) var isErrorHandled = false - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) { - override protected def buildRemoteLogAuxState(partition: TopicPartition, - currentLeaderEpoch: Int, - fetchOffset: Long, - epochForFetchOffset: Int, - leaderLogStartOffset: Long): Long = { + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) { + override def start(topicPartition: TopicPartition, currentFetchState: PartitionFetchState, fetchPartitionData: FetchResponseData.PartitionData): PartitionFetchState = { isErrorHandled = true - throw new FencedLeaderEpochException(s"Epoch $currentLeaderEpoch is fenced") + throw new FencedLeaderEpochException(s"Epoch ${currentFetchState.currentLeaderEpoch} is fenced") } } + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) val replicaLog = Seq( mkBatch(baseOffset = 1, leaderEpoch = 2, new SimpleRecord("b".getBytes)), @@ -704,16 +739,19 @@ class AbstractFetcherThreadTest { val partition = new TopicPartition("topic", 0) var fetchedEarliestOffset = false - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndPoint = new MockLeaderEndPoint { override def fetchEarliestOffset(topicPartition: TopicPartition, leaderEpoch: Int): OffsetAndEpoch = { fetchedEarliestOffset = true throw new FencedLeaderEpochException(s"Epoch $leaderEpoch is fenced") } + override def fetchEarliestLocalOffset(topicPartition: TopicPartition, leaderEpoch: Int): OffsetAndEpoch = { fetchedEarliestOffset = true throw new FencedLeaderEpochException(s"Epoch $leaderEpoch is fenced") } - }) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndPoint) + val fetcher = new MockFetcherThread(mockLeaderEndPoint, mockTierStateMachine) val replicaLog = Seq() val replicaState = PartitionState(replicaLog, leaderEpoch = 4, highWatermark = 0L) @@ -738,7 +776,9 @@ class AbstractFetcherThreadTest { @Test def testFollowerFetchOutOfRangeLow(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) // The follower begins from an offset which is behind the leader's log start offset val replicaLog = Seq( @@ -779,14 +819,16 @@ class AbstractFetcherThreadTest { @Test def testRetryAfterUnknownLeaderEpochInLatestOffsetFetch(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher: MockFetcherThread = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndPoint = new MockLeaderEndPoint { val tries = new AtomicInteger(0) override def fetchLatestOffset(topicPartition: TopicPartition, leaderEpoch: Int): OffsetAndEpoch = { if (tries.getAndIncrement() == 0) throw new UnknownLeaderEpochException("Unexpected leader epoch") super.fetchLatestOffset(topicPartition, leaderEpoch) } - }) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndPoint) + val fetcher: MockFetcherThread = new MockFetcherThread(mockLeaderEndPoint, mockTierStateMachine) // The follower begins from an offset which is behind the leader's log start offset val replicaLog = Seq( @@ -821,7 +863,7 @@ class AbstractFetcherThreadTest { def testCorruptMessage(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndPoint = new MockLeaderEndPoint { var fetchedOnce = false override def fetch(fetchRequest: FetchRequest.Builder): Map[TopicPartition, FetchData] = { val fetchedData = super.fetch(fetchRequest) @@ -834,7 +876,9 @@ class AbstractFetcherThreadTest { } fetchedData } - }) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndPoint) + val fetcher = new MockFetcherThread(mockLeaderEndPoint, mockTierStateMachine) fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) fetcher.addPartitions(Map(partition -> initialFetchState(topicIds.get(partition.topic), 0L, leaderEpoch = 0))) @@ -875,8 +919,9 @@ class AbstractFetcherThreadTest { val initialLeaderEpochOnFollower = 0 val nextLeaderEpochOnFollower = initialLeaderEpochOnFollower + 1 - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndpoint = new MockLeaderEndPoint { var fetchEpochsFromLeaderOnce = false + override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = { val fetchedEpochs = super.fetchEpochEndOffsets(partitions) if (!fetchEpochsFromLeaderOnce) { @@ -885,7 +930,9 @@ class AbstractFetcherThreadTest { } fetchedEpochs } - }) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) def changeLeaderEpochWhileFetchEpoch(): Unit = { fetcher.removePartitions(Set(partition)) @@ -928,13 +975,15 @@ class AbstractFetcherThreadTest { val initialLeaderEpochOnFollower = 0 val nextLeaderEpochOnFollower = initialLeaderEpochOnFollower + 1 - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndpoint = new MockLeaderEndPoint { override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = { val fetchedEpochs = super.fetchEpochEndOffsets(partitions) responseCallback.apply() fetchedEpochs } - }) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) def changeLeaderEpochDuringFetchEpoch(): Unit = { // leader epoch changes while fetching epochs from leader @@ -972,7 +1021,7 @@ class AbstractFetcherThreadTest { @Test def testTruncationThrowsExceptionIfLeaderReturnsPartitionsNotRequestedInFetchEpochs(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint { + val mockLeaderEndPoint = new MockLeaderEndPoint { override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = { val unrequestedTp = new TopicPartition("topic2", 0) super.fetchEpochEndOffsets(partitions).toMap + (unrequestedTp -> new EpochEndOffset() @@ -981,7 +1030,9 @@ class AbstractFetcherThreadTest { .setLeaderEpoch(0) .setEndOffset(0)) } - }) + } + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndPoint) + val fetcher = new MockFetcherThread(mockLeaderEndPoint, mockTierStateMachine) fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) fetcher.addPartitions(Map(partition -> initialFetchState(topicIds.get(partition.topic), 0L, leaderEpoch = 0)), forceTruncation = true) @@ -994,7 +1045,9 @@ class AbstractFetcherThreadTest { @Test def testFetcherThreadHandlingPartitionFailureDuringAppending(): Unit = { - val fetcherForAppend = new MockFetcherThread(new MockLeaderEndPoint) { + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcherForAppend = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) { override def processPartitionData(topicPartition: TopicPartition, fetchOffset: Long, partitionData: FetchData): Option[LogAppendInfo] = { if (topicPartition == partition1) { throw new KafkaException() @@ -1008,7 +1061,9 @@ class AbstractFetcherThreadTest { @Test def testFetcherThreadHandlingPartitionFailureDuringTruncation(): Unit = { - val fetcherForTruncation = new MockFetcherThread(new MockLeaderEndPoint) { + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcherForTruncation = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) { override def truncate(topicPartition: TopicPartition, truncationState: OffsetTruncationState): Unit = { if(topicPartition == partition1) throw new Exception() @@ -1057,7 +1112,9 @@ class AbstractFetcherThreadTest { @Test def testDivergingEpochs(): Unit = { val partition = new TopicPartition("topic", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) val replicaLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("a".getBytes)), @@ -1097,7 +1154,9 @@ class AbstractFetcherThreadTest { var truncateCalls = 0 var processPartitionDataCalls = 0 - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) { + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) { override def processPartitionData(topicPartition: TopicPartition, fetchOffset: Long, partitionData: FetchData): Option[LogAppendInfo] = { processPartitionDataCalls += 1 super.processPartitionData(topicPartition, fetchOffset, partitionData) @@ -1163,7 +1222,9 @@ class AbstractFetcherThreadTest { @Test def testMaybeUpdateTopicIds(): Unit = { val partition = new TopicPartition("topic1", 0) - val fetcher = new MockFetcherThread(new MockLeaderEndPoint) + val mockLeaderEndpoint = new MockLeaderEndPoint + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) // Start with no topic IDs fetcher.setReplicaState(partition, PartitionState(leaderEpoch = 0)) @@ -1405,6 +1466,30 @@ class AbstractFetcherThreadTest { } } + class MockTierStateMachine(leader: LeaderEndPoint) extends ReplicaFetcherTierStateMachine(leader, null) { + + var fetcher : MockFetcherThread = null + override def start(topicPartition: TopicPartition, + currentFetchState: PartitionFetchState, + fetchPartitionData: FetchResponseData.PartitionData): PartitionFetchState = { + val leaderEndOffset = leader.fetchLatestOffset(topicPartition, currentFetchState.currentLeaderEpoch).offset + val offsetToFetch = leader.fetchEarliestLocalOffset(topicPartition, currentFetchState.currentLeaderEpoch).offset + val initialLag = leaderEndOffset - offsetToFetch + fetcher.truncateFullyAndStartAt(topicPartition, offsetToFetch) + PartitionFetchState(currentFetchState.topicId, offsetToFetch, Option.apply(initialLag), currentFetchState.currentLeaderEpoch, + Fetching, Some(currentFetchState.currentLeaderEpoch)) + } + + override def maybeAdvanceState(topicPartition: TopicPartition, + currentFetchState: PartitionFetchState): Optional[PartitionFetchState] = { + Optional.of(currentFetchState) + } + + def setFetcher(mockFetcherThread: MockFetcherThread): Unit = { + fetcher = mockFetcherThread + } + } + class PartitionState(var log: mutable.Buffer[RecordBatch], var leaderEpoch: Int, var logStartOffset: Long, @@ -1425,17 +1510,24 @@ class AbstractFetcherThreadTest { } } - class MockFetcherThread(val mockLeader : MockLeaderEndPoint, val replicaId: Int = 0, val leaderId: Int = 1, fetchBackOffMs: Int = 0) + class MockFetcherThread(val mockLeader : MockLeaderEndPoint, + val mockTierStateMachine: MockTierStateMachine, + val replicaId: Int = 0, + val leaderId: Int = 1, + fetchBackOffMs: Int = 0) extends AbstractFetcherThread("mock-fetcher", clientId = "mock-fetcher", leader = mockLeader, failedPartitions, + mockTierStateMachine, fetchBackOffMs = fetchBackOffMs, brokerTopicStats = new BrokerTopicStats) { private val replicaPartitionStates = mutable.Map[TopicPartition, PartitionState]() private var latestEpochDefault: Option[Int] = Some(0) + mockTierStateMachine.setFetcher(this) + def setReplicaState(topicPartition: TopicPartition, state: PartitionState): Unit = { replicaPartitionStates.put(topicPartition, state) } @@ -1554,18 +1646,6 @@ class AbstractFetcherThreadTest { } override protected val isOffsetForLeaderEpochSupported: Boolean = true - - override protected def buildRemoteLogAuxState(topicPartition: TopicPartition, - currentLeaderEpoch: Int, - fetchOffset: Long, - epochForFetchOffset: Int, - leaderLogStartOffset: Long): Long = { - truncateFullyAndStartAt(topicPartition, fetchOffset) - replicaPartitionState(topicPartition).logStartOffset = leaderLogStartOffset - // skipped building leader epoch cache and producer snapshots as they are not verified. - leaderLogStartOffset - } - } } diff --git a/core/src/test/scala/unit/kafka/server/FetchRequestDownConversionConfigTest.scala b/core/src/test/scala/unit/kafka/server/FetchRequestDownConversionConfigTest.scala index 3f8ba8e099a68..d01228c1d139d 100644 --- a/core/src/test/scala/unit/kafka/server/FetchRequestDownConversionConfigTest.scala +++ b/core/src/test/scala/unit/kafka/server/FetchRequestDownConversionConfigTest.scala @@ -18,6 +18,7 @@ package kafka.server import java.util import java.util.{Optional, Properties} +import kafka.network.RequestMetrics.{MessageConversionsTimeMs, TemporaryMemoryBytes} import kafka.utils.{TestInfoUtils, TestUtils} import org.apache.kafka.clients.producer.{KafkaProducer, ProducerRecord} import org.apache.kafka.common.config.TopicConfig @@ -163,7 +164,12 @@ class FetchRequestDownConversionConfigTest extends BaseRequestTest { } def testV1Fetch(isFollowerFetch: Boolean): Unit = { + val fetchRequest = "request=Fetch" + val fetchTemporaryMemoryBytesMetricName = s"$TemporaryMemoryBytes,$fetchRequest" + val fetchMessageConversionsTimeMsMetricName = s"$MessageConversionsTimeMs,$fetchRequest" val initialFetchMessageConversionsPerSec = TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec) + val initialFetchMessageConversionsTimeMs = TestUtils.metersCount(fetchMessageConversionsTimeMsMetricName) + val initialFetchTemporaryMemoryBytes = TestUtils.metersCount(fetchTemporaryMemoryBytesMetricName) val topicWithDownConversionEnabled = "foo" val topicWithDownConversionDisabled = "bar" val replicaIds = brokers.map(_.config.brokerId) @@ -216,15 +222,28 @@ class FetchRequestDownConversionConfigTest extends BaseRequestTest { Errors.forCode(fetchResponseData.get(tp).errorCode) } + def verifyMetrics(): Unit = { + TestUtils.waitUntilTrue(() => TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec) > initialFetchMessageConversionsPerSec, + s"The `FetchMessageConversionsPerSec` metric count is not incremented after 5 seconds. " + + s"init: $initialFetchMessageConversionsPerSec final: ${TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec)}", 5000) + + TestUtils.waitUntilTrue(() => TestUtils.metersCount(fetchMessageConversionsTimeMsMetricName) > initialFetchMessageConversionsTimeMs, + s"The `MessageConversionsTimeMs` in fetch request metric count is not incremented after 5 seconds. " + + s"init: $initialFetchMessageConversionsTimeMs final: ${TestUtils.metersCount(fetchMessageConversionsTimeMsMetricName)}", 5000) + + TestUtils.waitUntilTrue(() => TestUtils.metersCount(fetchTemporaryMemoryBytesMetricName) > initialFetchTemporaryMemoryBytes, + s"The `TemporaryMemoryBytes` in fetch request metric count is not incremented after 5 seconds. " + + s"init: $initialFetchTemporaryMemoryBytes final: ${TestUtils.metersCount(fetchTemporaryMemoryBytesMetricName)}", 5000) + } + assertEquals(Errors.NONE, error(partitionWithDownConversionEnabled)) if (isFollowerFetch) { assertEquals(Errors.NONE, error(partitionWithDownConversionDisabled)) } else { assertEquals(Errors.UNSUPPORTED_VERSION, error(partitionWithDownConversionDisabled)) } - TestUtils.waitUntilTrue(() => TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec) > initialFetchMessageConversionsPerSec, - s"The `FetchMessageConversionsPerSec` metric count is not incremented after 5 seconds. " + - s"init: $initialFetchMessageConversionsPerSec final: ${TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec)}", 5000) + + verifyMetrics() } private def sendFetch( diff --git a/core/src/test/scala/unit/kafka/utils/TestUtils.scala b/core/src/test/scala/unit/kafka/utils/TestUtils.scala index 4cec7718c8e10..68a14bb9ffc38 100755 --- a/core/src/test/scala/unit/kafka/utils/TestUtils.scala +++ b/core/src/test/scala/unit/kafka/utils/TestUtils.scala @@ -28,7 +28,7 @@ import java.util import java.util.concurrent.atomic.{AtomicBoolean, AtomicInteger} import java.util.concurrent.{Callable, CompletableFuture, ExecutionException, Executors, TimeUnit} import java.util.{Arrays, Collections, Optional, Properties} -import com.yammer.metrics.core.{Gauge, Meter} +import com.yammer.metrics.core.{Gauge, Histogram, Meter} import javax.net.ssl.X509TrustManager import kafka.api._ @@ -2112,8 +2112,12 @@ object TestUtils extends Logging { def metersCount(metricName: String): Long = { KafkaYammerMetrics.defaultRegistry.allMetrics.asScala - .filter { case (k, _) => k.getMBeanName.endsWith(metricName)} - .values.map(_.asInstanceOf[Meter].count()).sum + .filter { case (k, _) => k.getMBeanName.endsWith(metricName) } + .values.map { + case histogram: Histogram => histogram.count() + case meter: Meter => meter.count() + case _ => 0 + }.sum } def clearYammerMetrics(): Unit = { diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index a537b4ff0c666..c9b1c59b3636a 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -62,13 +62,13 @@ versions += [ checkstyle: "8.36.2", commonsCli: "1.4", dropwizardMetrics: "4.1.12.1", - gradle: "7.6", + gradle: "8.0.1", grgit: "4.1.1", httpclient: "4.5.13", easymock: "4.3", jackson: "2.13.4", jacksonDatabind: "2.13.4.2", - jacoco: "0.8.7", + jacoco: "0.8.8", javassist: "3.27.0-GA", jetty: "9.4.48.v20220622", jersey: "2.34", diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index e9ee3c5440606..454aca1242237 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,7 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionSha=312eb12875e1747e05c2f81a4789902d7e4ec5defbd1eefeaccc08acf096505d -distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-all.zip +distributionSha=948d1e4ccc2f6ae36cbfa087d827aaca51403acae5411e664a6000bb47946f22 +distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.1-all.zip +networkTimeout=10000 zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/gradlew b/gradlew index a2e85319111cf..cd19cf521d57b 100755 --- a/gradlew +++ b/gradlew @@ -55,7 +55,7 @@ # Darwin, MinGW, and NonStop. # # (3) This script is generated from the Groovy template -# https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt +# https://github.com/gradle/gradle/blob/HEAD/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt # within the Gradle project. # # You can find Gradle at https://github.com/gradle/gradle/. @@ -80,10 +80,10 @@ do esac done -APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit - -APP_NAME="Gradle" +# This is normally unused +# shellcheck disable=SC2034 APP_BASE_NAME=${0##*/} +APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit # Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' @@ -118,7 +118,7 @@ esac # Loop in case we encounter an error. for attempt in 1 2 3; do if [ ! -e "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" ]; then - if ! curl -s -S --retry 3 -L -o "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" "https://raw.githubusercontent.com/gradle/gradle/v7.6.0/gradle/wrapper/gradle-wrapper.jar"; then + if ! curl -s -S --retry 3 -L -o "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" "https://raw.githubusercontent.com/gradle/gradle/v8.0.1/gradle/wrapper/gradle-wrapper.jar"; then rm -f "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" # Pause for a bit before looping in case the server throttled us. sleep 5 @@ -156,12 +156,16 @@ fi if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then case $MAX_FD in #( max*) + # In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked. + # shellcheck disable=SC3045 MAX_FD=$( ulimit -H -n ) || warn "Could not query maximum file descriptor limit" esac case $MAX_FD in #( '' | soft) :;; #( *) + # In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked. + # shellcheck disable=SC3045 ulimit -n "$MAX_FD" || warn "Could not set maximum file descriptor limit to $MAX_FD" esac diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/ChangelogReader.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/ChangelogReader.java index 03199d294caae..1cf8ef628dac3 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/ChangelogReader.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/ChangelogReader.java @@ -28,8 +28,10 @@ public interface ChangelogReader extends ChangelogRegister { /** * Restore all registered state stores by reading from their changelogs + * + * @return the total number of records restored in this call */ - void restore(final Map tasks); + long restore(final Map tasks); /** * Transit to restore active changelogs mode @@ -41,6 +43,12 @@ public interface ChangelogReader extends ChangelogRegister { */ void transitToUpdateStandby(); + /** + * @return true if the reader is in restoring active changelog mode; + * false if the reader is in updating standby changelog mode + */ + boolean isRestoringActive(); + /** * @return the changelog partitions that have been completed restoring */ diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdater.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdater.java index ae6618c304f70..5e912c99a5b0b 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdater.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdater.java @@ -16,8 +16,15 @@ */ package org.apache.kafka.streams.processor.internals; +import org.apache.kafka.common.MetricName; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.errors.InterruptException; +import org.apache.kafka.common.metrics.Metrics; +import org.apache.kafka.common.metrics.Sensor; +import org.apache.kafka.common.metrics.Sensor.RecordingLevel; +import org.apache.kafka.common.metrics.stats.Avg; +import org.apache.kafka.common.metrics.stats.Rate; +import org.apache.kafka.common.metrics.stats.WindowedCount; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.Time; import org.apache.kafka.streams.StreamsConfig; @@ -32,7 +39,9 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Deque; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -51,6 +60,10 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl.RATE_DESCRIPTION; +import static org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl.RATIO_DESCRIPTION; +import static org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl.THREAD_ID_TAG; + public class DefaultStateUpdater implements StateUpdater { private final static String BUG_ERROR_MESSAGE = "This indicates a bug. " + @@ -59,14 +72,20 @@ public class DefaultStateUpdater implements StateUpdater { private class StateUpdaterThread extends Thread { private final ChangelogReader changelogReader; + private final StateUpdaterMetrics updaterMetrics; private final AtomicBoolean isRunning = new AtomicBoolean(true); private final Map updatingTasks = new ConcurrentHashMap<>(); private final Map pausedTasks = new ConcurrentHashMap<>(); private final Logger log; - public StateUpdaterThread(final String name, final ChangelogReader changelogReader) { + private long totalCheckpointLatency = 0L; + + public StateUpdaterThread(final String name, + final Metrics metrics, + final ChangelogReader changelogReader) { super(name); this.changelogReader = changelogReader; + this.updaterMetrics = new StateUpdaterMetrics(metrics, name); final String logPrefix = String.format("state-updater [%s] ", name); final LogContext logContext = new LogContext(logPrefix); @@ -92,6 +111,30 @@ public Collection getPausedTasks() { return pausedTasks.values(); } + public long getNumUpdatingStandbyTasks() { + return updatingTasks.values().stream() + .filter(t -> !t.isActive()) + .count(); + } + + public long getNumRestoringActiveTasks() { + return updatingTasks.values().stream() + .filter(Task::isActive) + .count(); + } + + public long getNumPausedStandbyTasks() { + return pausedTasks.values().stream() + .filter(t -> !t.isActive()) + .count(); + } + + public long getNumPausedActiveTasks() { + return pausedTasks.values().stream() + .filter(Task::isActive) + .count(); + } + @Override public void run() { log.info("State updater thread started"); @@ -109,17 +152,41 @@ public void run() { Thread.interrupted(); // Clear the interrupted flag. removeAddedTasksFromInputQueue(); removeUpdatingAndPausedTasks(); + updaterMetrics.clear(); shutdownGate.countDown(); log.info("State updater thread shutdown"); } } + // In each iteration: + // 1) check if updating tasks need to be paused + // 2) check if paused tasks need to be resumed + // 3) restore those updating tasks + // 4) checkpoint those updating task states + // 5) idle waiting if there is no more tasks to be restored + // + // Note that, 1-3) are measured as restoring time, while 4) and 5) measured separately + // as checkpointing time and idle time private void runOnce() throws InterruptedException { + final long totalStartTimeMs = time.milliseconds(); performActionsOnTasks(); + resumeTasks(); - restoreTasks(); - checkAllUpdatingTaskStates(time.milliseconds()); + pauseTasks(); + restoreTasks(totalStartTimeMs); + + final long checkpointStartTimeMs = time.milliseconds(); + maybeCheckpointTasks(checkpointStartTimeMs); + + final long waitStartTimeMs = time.milliseconds(); + waitIfAllChangelogsCompletelyRead(); + + final long endTimeMs = time.milliseconds(); + final long totalWaitTime = Math.max(0L, endTimeMs - waitStartTimeMs); + final long totalTime = Math.max(0L, endTimeMs - totalStartTimeMs); + + recordMetrics(endTimeMs, totalTime, totalWaitTime); } private void performActionsOnTasks() { @@ -151,9 +218,18 @@ private void resumeTasks() { } } - private void restoreTasks() { + private void pauseTasks() { + for (final Task task : updatingTasks.values()) { + if (topologyMetadata.isPaused(task.id().topologyName())) { + pauseTask(task); + } + } + } + + private void restoreTasks(final long now) { try { - changelogReader.restore(updatingTasks); + final long restored = changelogReader.restore(updatingTasks); + updaterMetrics.restoreSensor.record(restored, now); } catch (final TaskCorruptedException taskCorruptedException) { handleTaskCorruptedException(taskCorruptedException); } catch (final StreamsException streamsException) { @@ -193,7 +269,7 @@ private void removeCheckpointForCorruptedTask(final Task task) { task.markChangelogAsCorrupted(task.changelogPartitions()); // we need to enforce a checkpoint that removes the corrupted partitions - task.maybeCheckpoint(true); + measureCheckpointLatency(() -> task.maybeCheckpoint(true)); } private void handleStreamsException(final StreamsException streamsException) { @@ -251,10 +327,10 @@ private void waitIfAllChangelogsCompletelyRead() throws InterruptedException { private void removeUpdatingAndPausedTasks() { changelogReader.clear(); - updatingTasks.forEach((id, task) -> { + measureCheckpointLatency(() -> updatingTasks.forEach((id, task) -> { task.maybeCheckpoint(true); removedTasks.add(task); - }); + })); updatingTasks.clear(); pausedTasks.forEach((id, task) -> { removedTasks.add(task); @@ -313,7 +389,7 @@ private void removeTask(final TaskId taskId) { final Task task; if (updatingTasks.containsKey(taskId)) { task = updatingTasks.get(taskId); - task.maybeCheckpoint(true); + measureCheckpointLatency(() -> task.maybeCheckpoint(true)); final Collection changelogPartitions = task.changelogPartitions(); changelogReader.unregister(changelogPartitions); removedTasks.add(task); @@ -339,7 +415,7 @@ private void removeTask(final TaskId taskId) { private void pauseTask(final Task task) { final TaskId taskId = task.id(); // do not need to unregister changelog partitions for paused tasks - task.maybeCheckpoint(true); + measureCheckpointLatency(() -> task.maybeCheckpoint(true)); pausedTasks.put(taskId, task); updatingTasks.remove(taskId); if (task.isActive()) { @@ -373,7 +449,7 @@ private void maybeCompleteRestoration(final StreamTask task, final Set restoredChangelogs) { final Collection changelogPartitions = task.changelogPartitions(); if (restoredChangelogs.containsAll(changelogPartitions)) { - task.maybeCheckpoint(true); + measureCheckpointLatency(() -> task.maybeCheckpoint(true)); changelogReader.unregister(changelogPartitions); addToRestoredTasks(task); updatingTasks.remove(task.id()); @@ -399,31 +475,55 @@ private void addToRestoredTasks(final StreamTask task) { } } - private void checkAllUpdatingTaskStates(final long now) { + private void maybeCheckpointTasks(final long now) { final long elapsedMsSinceLastCommit = now - lastCommitMs; if (elapsedMsSinceLastCommit > commitIntervalMs) { if (log.isDebugEnabled()) { - log.debug("Checking all restoring task states since {}ms has elapsed (commit interval is {}ms)", + log.debug("Checkpointing state of all restoring tasks since {}ms has elapsed (commit interval is {}ms)", elapsedMsSinceLastCommit, commitIntervalMs); } - for (final Task task : updatingTasks.values()) { - if (topologyMetadata.isPaused(task.id().topologyName())) { - pauseTask(task); - } else { - log.debug("Try to checkpoint current restoring progress for task {}", task.id()); + measureCheckpointLatency(() -> { + for (final Task task : updatingTasks.values()) { // do not enforce checkpointing during restoration if its position has not advanced much task.maybeCheckpoint(false); } - } + }); lastCommitMs = now; } } + + private void measureCheckpointLatency(final Runnable actionToMeasure) { + final long startMs = time.milliseconds(); + try { + actionToMeasure.run(); + } finally { + totalCheckpointLatency += Math.max(0L, time.milliseconds() - startMs); + } + } + + private void recordMetrics(final long now, final long totalLatency, final long totalWaitLatency) { + final long totalRestoreLatency = Math.max(0L, totalLatency - totalWaitLatency - totalCheckpointLatency); + + updaterMetrics.idleRatioSensor.record((double) totalWaitLatency / totalLatency, now); + updaterMetrics.checkpointRatioSensor.record((double) totalCheckpointLatency / totalLatency, now); + + if (changelogReader.isRestoringActive()) { + updaterMetrics.activeRestoreRatioSensor.record((double) totalRestoreLatency / totalLatency, now); + updaterMetrics.standbyRestoreRatioSensor.record(0.0d, now); + } else { + updaterMetrics.standbyRestoreRatioSensor.record((double) totalRestoreLatency / totalLatency, now); + updaterMetrics.activeRestoreRatioSensor.record(0.0d, now); + } + + totalCheckpointLatency = 0L; + } } private final Time time; private final String name; + private final Metrics metrics; private final ChangelogReader changelogReader; private final TopologyMetadata topologyMetadata; private final Queue tasksAndActions = new LinkedList<>(); @@ -443,12 +543,14 @@ private void checkAllUpdatingTaskStates(final long now) { private CountDownLatch shutdownGate; public DefaultStateUpdater(final String name, + final Metrics metrics, final StreamsConfig config, final ChangelogReader changelogReader, final TopologyMetadata topologyMetadata, final Time time) { this.time = time; this.name = name; + this.metrics = metrics; this.changelogReader = changelogReader; this.topologyMetadata = topologyMetadata; this.commitIntervalMs = config.getLong(StreamsConfig.COMMIT_INTERVAL_MS_CONFIG); @@ -456,7 +558,7 @@ public DefaultStateUpdater(final String name, public void start() { if (stateUpdaterThread == null) { - stateUpdaterThread = new StateUpdaterThread(name, changelogReader); + stateUpdaterThread = new StateUpdaterThread(name, metrics, changelogReader); stateUpdaterThread.start(); shutdownGate = new CountDownLatch(1); @@ -686,4 +788,92 @@ private Stream getStreamOfNonPausedTasks() { exceptionsAndFailedTasks.stream().flatMap(exceptionAndTasks -> exceptionAndTasks.getTasks().stream()), removedTasks.stream())))); } + + private class StateUpdaterMetrics { + private static final String STATE_LEVEL_GROUP = "stream-state-updater-metrics"; + + private static final String IDLE_RATIO_DESCRIPTION = RATIO_DESCRIPTION + "being idle"; + private static final String RESTORE_RATIO_DESCRIPTION = RATIO_DESCRIPTION + "restoring active tasks"; + private static final String UPDATE_RATIO_DESCRIPTION = RATIO_DESCRIPTION + "updating standby tasks"; + private static final String CHECKPOINT_RATIO_DESCRIPTION = RATIO_DESCRIPTION + "checkpointing tasks restored progress"; + private static final String RESTORE_RECORDS_RATE_DESCRIPTION = RATE_DESCRIPTION + "records restored"; + private static final String RESTORE_RATE_DESCRIPTION = RATE_DESCRIPTION + "restore calls triggered"; + + private final Sensor restoreSensor; + private final Sensor idleRatioSensor; + private final Sensor activeRestoreRatioSensor; + private final Sensor standbyRestoreRatioSensor; + private final Sensor checkpointRatioSensor; + + private final Deque allSensorNames = new LinkedList<>(); + private final Deque allMetricNames = new LinkedList<>(); + + private StateUpdaterMetrics(final Metrics metrics, final String threadId) { + final Map threadLevelTags = new LinkedHashMap<>(); + threadLevelTags.put(THREAD_ID_TAG, threadId); + + MetricName metricName = metrics.metricName("active-restoring-tasks", + STATE_LEVEL_GROUP, + "The number of active tasks currently undergoing restoration", + threadLevelTags); + metrics.addMetric(metricName, (config, now) -> stateUpdaterThread != null ? + stateUpdaterThread.getNumRestoringActiveTasks() : 0); + allMetricNames.push(metricName); + + metricName = metrics.metricName("standby-updating-tasks", + STATE_LEVEL_GROUP, + "The number of standby tasks currently undergoing state update", + threadLevelTags); + metrics.addMetric(metricName, (config, now) -> stateUpdaterThread != null ? + stateUpdaterThread.getNumUpdatingStandbyTasks() : 0); + allMetricNames.push(metricName); + + metricName = metrics.metricName("active-paused-tasks", + STATE_LEVEL_GROUP, + "The number of active tasks paused restoring", + threadLevelTags); + metrics.addMetric(metricName, (config, now) -> stateUpdaterThread != null ? + stateUpdaterThread.getNumPausedActiveTasks() : 0); + allMetricNames.push(metricName); + + metricName = metrics.metricName("standby-paused-tasks", + STATE_LEVEL_GROUP, + "The number of standby tasks paused state update", + threadLevelTags); + metrics.addMetric(metricName, (config, now) -> stateUpdaterThread != null ? + stateUpdaterThread.getNumPausedStandbyTasks() : 0); + allMetricNames.push(metricName); + + this.idleRatioSensor = metrics.sensor("idle-ratio", RecordingLevel.INFO); + this.idleRatioSensor.add(new MetricName("idle-ratio", STATE_LEVEL_GROUP, IDLE_RATIO_DESCRIPTION, threadLevelTags), new Avg()); + allSensorNames.add("idle-ratio"); + + this.activeRestoreRatioSensor = metrics.sensor("active-restore-ratio", RecordingLevel.INFO); + this.activeRestoreRatioSensor.add(new MetricName("active-restore-ratio", STATE_LEVEL_GROUP, RESTORE_RATIO_DESCRIPTION, threadLevelTags), new Avg()); + allSensorNames.add("active-restore-ratio"); + + this.standbyRestoreRatioSensor = metrics.sensor("standby-update-ratio", RecordingLevel.INFO); + this.standbyRestoreRatioSensor.add(new MetricName("standby-update-ratio", STATE_LEVEL_GROUP, UPDATE_RATIO_DESCRIPTION, threadLevelTags), new Avg()); + allSensorNames.add("standby-update-ratio"); + + this.checkpointRatioSensor = metrics.sensor("checkpoint-ratio", RecordingLevel.INFO); + this.checkpointRatioSensor.add(new MetricName("checkpoint-ratio", STATE_LEVEL_GROUP, CHECKPOINT_RATIO_DESCRIPTION, threadLevelTags), new Avg()); + allSensorNames.add("checkpoint-ratio"); + + this.restoreSensor = metrics.sensor("restore-records", RecordingLevel.INFO); + this.restoreSensor.add(new MetricName("restore-records-rate", STATE_LEVEL_GROUP, RESTORE_RECORDS_RATE_DESCRIPTION, threadLevelTags), new Rate()); + this.restoreSensor.add(new MetricName("restore-call-rate", STATE_LEVEL_GROUP, RESTORE_RATE_DESCRIPTION, threadLevelTags), new Rate(new WindowedCount())); + allSensorNames.add("restore-records"); + } + + void clear() { + while (!allSensorNames.isEmpty()) { + metrics.removeSensor(allSensorNames.pop()); + } + + while (!allMetricNames.isEmpty()) { + metrics.removeMetric(allMetricNames.pop()); + } + } + } } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreChangelogReader.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreChangelogReader.java index be580f3575cfc..701ecb9b5676b 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreChangelogReader.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreChangelogReader.java @@ -330,6 +330,11 @@ public void transitToUpdateStandby() { state = ChangelogReaderState.STANDBY_UPDATING; } + @Override + public boolean isRestoringActive() { + return state == ChangelogReaderState.ACTIVE_RESTORING; + } + /** * Since it is shared for multiple tasks and hence multiple state managers, the registration would take its * corresponding state manager as well for restoring. @@ -423,49 +428,22 @@ public Set completedChangelogs() { // 2. if all changelogs have finished, return early; // 3. if there are any restoring changelogs, try to read from the restore consumer and process them. @Override - public void restore(final Map tasks) { - - // If we are updating only standby tasks, and are not using a separate thread, we should - // use a non-blocking poll to unblock the processing as soon as possible. - final boolean useNonBlockingPoll = state == ChangelogReaderState.STANDBY_UPDATING && !stateUpdaterEnabled; - + public long restore(final Map tasks) { initializeChangelogs(tasks, registeredChangelogs()); if (!activeRestoringChangelogs().isEmpty() && state == ChangelogReaderState.STANDBY_UPDATING) { throw new IllegalStateException("Should not be in standby updating state if there are still un-completed active changelogs"); } + long totalRestored = 0L; if (allChangelogsCompleted()) { log.debug("Finished restoring all changelogs {}", changelogs.keySet()); - return; + return totalRestored; } final Set restoringChangelogs = restoringChangelogs(); if (!restoringChangelogs.isEmpty()) { - final ConsumerRecords polledRecords; - - try { - pauseResumePartitions(tasks, restoringChangelogs); - - polledRecords = restoreConsumer.poll(useNonBlockingPoll ? Duration.ZERO : pollTime); - - // TODO (?) If we cannot fetch records during restore, should we trigger `task.timeout.ms` ? - // TODO (?) If we cannot fetch records for standby task, should we trigger `task.timeout.ms` ? - } catch (final InvalidOffsetException e) { - log.warn("Encountered " + e.getClass().getName() + - " fetching records from restore consumer for partitions " + e.partitions() + ", it is likely that " + - "the consumer's position has fallen out of the topic partition offset range because the topic was " + - "truncated or compacted on the broker, marking the corresponding tasks as corrupted and re-initializing" + - " it later.", e); - - final Set corruptedTasks = new HashSet<>(); - e.partitions().forEach(partition -> corruptedTasks.add(changelogs.get(partition).stateManager.taskId())); - throw new TaskCorruptedException(corruptedTasks, e); - } catch (final InterruptException interruptException) { - throw interruptException; - } catch (final KafkaException e) { - throw new StreamsException("Restore consumer get unexpected error polling records.", e); - } + final ConsumerRecords polledRecords = pollRecordsFromRestoreConsumer(tasks, restoringChangelogs); for (final TopicPartition partition : polledRecords.partitions()) { bufferChangelogRecords(restoringChangelogByPartition(partition), polledRecords.records(partition)); @@ -479,12 +457,15 @@ public void restore(final Map tasks) { // small batches; this can be optimized in the future, e.g. wait longer for larger batches. final TaskId taskId = changelogs.get(partition).stateManager.taskId(); try { - if (restoreChangelog(changelogs.get(partition))) { + final ChangelogMetadata changelogMetadata = changelogs.get(partition); + final int restored = restoreChangelog(changelogMetadata); + if (restored > 0 || changelogMetadata.state().equals(ChangelogState.COMPLETED)) { final Task task = tasks.get(taskId); if (task != null) { task.clearTaskTimeout(); } } + totalRestored += restored; } catch (final TimeoutException timeoutException) { tasks.get(taskId).maybeInitTaskTimeoutOrThrow( time.milliseconds(), @@ -497,6 +478,41 @@ public void restore(final Map tasks) { maybeLogRestorationProgress(); } + + return totalRestored; + } + + private ConsumerRecords pollRecordsFromRestoreConsumer(final Map tasks, + final Set restoringChangelogs) { + // If we are updating only standby tasks, and are not using a separate thread, we should + // use a non-blocking poll to unblock the processing as soon as possible. + final boolean useNonBlockingPoll = state == ChangelogReaderState.STANDBY_UPDATING && !stateUpdaterEnabled; + final ConsumerRecords polledRecords; + + try { + pauseResumePartitions(tasks, restoringChangelogs); + + polledRecords = restoreConsumer.poll(useNonBlockingPoll ? Duration.ZERO : pollTime); + + // TODO (?) If we cannot fetch records during restore, should we trigger `task.timeout.ms` ? + // TODO (?) If we cannot fetch records for standby task, should we trigger `task.timeout.ms` ? + } catch (final InvalidOffsetException e) { + log.warn("Encountered " + e.getClass().getName() + + " fetching records from restore consumer for partitions " + e.partitions() + ", it is likely that " + + "the consumer's position has fallen out of the topic partition offset range because the topic was " + + "truncated or compacted on the broker, marking the corresponding tasks as corrupted and re-initializing " + + "it later.", e); + + final Set corruptedTasks = new HashSet<>(); + e.partitions().forEach(partition -> corruptedTasks.add(changelogs.get(partition).stateManager.taskId())); + throw new TaskCorruptedException(corruptedTasks, e); + } catch (final InterruptException interruptException) { + throw interruptException; + } catch (final KafkaException e) { + throw new StreamsException("Restore consumer get unexpected error polling records.", e); + } + + return polledRecords; } private void pauseResumePartitions(final Map tasks, @@ -623,19 +639,17 @@ private void bufferChangelogRecords(final ChangelogMetadata changelogMetadata, f /** * restore a changelog with its buffered records if there's any; for active changelogs also check if * it has completed the restoration and can transit to COMPLETED state and trigger restore callbacks + * + * @return number of records restored */ - private boolean restoreChangelog(final ChangelogMetadata changelogMetadata) { + private int restoreChangelog(final ChangelogMetadata changelogMetadata) { final ProcessorStateManager stateManager = changelogMetadata.stateManager; final StateStoreMetadata storeMetadata = changelogMetadata.storeMetadata; final TopicPartition partition = storeMetadata.changelogPartition(); final String storeName = storeMetadata.store().name(); final int numRecords = changelogMetadata.bufferedLimitIndex; - boolean madeProgress = false; - if (numRecords != 0) { - madeProgress = true; - final List> records = changelogMetadata.bufferedRecords.subList(0, numRecords); stateManager.restore(storeMetadata, records); @@ -650,7 +664,7 @@ private boolean restoreChangelog(final ChangelogMetadata changelogMetadata) { final Long currentOffset = storeMetadata.offset(); log.trace("Restored {} records from changelog {} to store {}, end offset is {}, current offset is {}", - partition, storeName, numRecords, recordEndOffset(changelogMetadata.restoreEndOffset), currentOffset); + numRecords, partition, storeName, recordEndOffset(changelogMetadata.restoreEndOffset), currentOffset); changelogMetadata.bufferedLimitIndex = 0; changelogMetadata.totalRestored += numRecords; @@ -667,8 +681,6 @@ private boolean restoreChangelog(final ChangelogMetadata changelogMetadata) { // we should check even if there's nothing restored, but do not check completed if we are processing standby tasks if (changelogMetadata.stateManager.taskType() == Task.TaskType.ACTIVE && hasRestoredToEnd(changelogMetadata)) { - madeProgress = true; - log.info("Finished restoring changelog {} to store {} with a total number of {} records", partition, storeName, changelogMetadata.totalRestored); @@ -682,7 +694,7 @@ private boolean restoreChangelog(final ChangelogMetadata changelogMetadata) { } } - return madeProgress; + return numRecords; } private Set getTasksFromPartitions(final Map tasks, 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 02bd74a027d30..1f2a91d27b721 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 @@ -404,7 +404,7 @@ public static StreamThread create(final TopologyMetadata topologyMetadata, topologyMetadata, adminClient, stateDirectory, - maybeCreateAndStartStateUpdater(stateUpdaterEnabled, config, changelogReader, topologyMetadata, time, clientId, threadIdx) + maybeCreateAndStartStateUpdater(stateUpdaterEnabled, streamsMetrics, config, changelogReader, topologyMetadata, time, clientId, threadIdx) ); referenceContainer.taskManager = taskManager; @@ -448,6 +448,7 @@ public static StreamThread create(final TopologyMetadata topologyMetadata, } private static StateUpdater maybeCreateAndStartStateUpdater(final boolean stateUpdaterEnabled, + final StreamsMetricsImpl streamsMetrics, final StreamsConfig streamsConfig, final ChangelogReader changelogReader, final TopologyMetadata topologyMetadata, @@ -456,7 +457,7 @@ private static StateUpdater maybeCreateAndStartStateUpdater(final boolean stateU final int threadIdx) { if (stateUpdaterEnabled) { final String name = clientId + "-StateUpdater-" + threadIdx; - final StateUpdater stateUpdater = new DefaultStateUpdater(name, streamsConfig, changelogReader, topologyMetadata, time); + final StateUpdater stateUpdater = new DefaultStateUpdater(name, streamsMetrics.metricsRegistry(), streamsConfig, changelogReader, topologyMetadata, time); stateUpdater.start(); return stateUpdater; } else { 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 cf83cb27ebaff..c2f3b5253ac49 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 @@ -1552,7 +1552,9 @@ Map allTasks() { /** * Returns tasks owned by the stream thread. With state updater disabled, these are all tasks. With * state updater enabled, this does not return any tasks currently owned by the state updater. - * @return + * + * TODO: after we complete switching to state updater, we could rename this function as allRunningTasks + * to be differentiated from allTasks including running and restoring tasks */ Map allOwnedTasks() { // not bothering with an unmodifiable map, since the tasks themselves are mutable, but diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/metrics/StreamsMetricsImpl.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/metrics/StreamsMetricsImpl.java index 3260bfc1b8276..f3cd0982eaa30 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/metrics/StreamsMetricsImpl.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/metrics/StreamsMetricsImpl.java @@ -146,6 +146,7 @@ public int hashCode() { public static final String OPERATIONS = " operations"; public static final String TOTAL_DESCRIPTION = "The total number of "; public static final String RATE_DESCRIPTION = "The average per-second number of "; + public static final String RATIO_DESCRIPTION = "The fraction of time the thread spent on "; public static final String AVG_LATENCY_DESCRIPTION = "The average latency of "; public static final String MAX_LATENCY_DESCRIPTION = "The maximum latency of "; public static final String RATE_DESCRIPTION_PREFIX = "The average number of "; @@ -177,6 +178,10 @@ public Version version() { return version; } + public Metrics metricsRegistry() { + return metrics; + } + public RocksDBMetricsRecordingTrigger rocksDBMetricsRecordingTrigger() { return rocksDBMetricsRecordingTrigger; } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java index 9f8ca1c72a91b..7144edb3e73cc 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java @@ -81,14 +81,14 @@ public class MeteredKeyValueStore protected Sensor putSensor; private Sensor putIfAbsentSensor; protected Sensor getSensor; - private Sensor deleteSensor; + protected Sensor deleteSensor; private Sensor putAllSensor; private Sensor allSensor; private Sensor rangeSensor; private Sensor prefixScanSensor; private Sensor flushSensor; private Sensor e2eLatencySensor; - private InternalProcessorContext context; + protected InternalProcessorContext context; private StreamsMetricsImpl streamsMetrics; private TaskId taskId; @@ -246,9 +246,9 @@ public Position getPosition() { } @SuppressWarnings("unchecked") - private QueryResult runRangeQuery(final Query query, - final PositionBound positionBound, - final QueryConfig config) { + protected QueryResult runRangeQuery(final Query query, + final PositionBound positionBound, + final QueryConfig config) { final QueryResult result; final RangeQuery typedQuery = (RangeQuery) query; @@ -289,9 +289,9 @@ private QueryResult runRangeQuery(final Query query, @SuppressWarnings("unchecked") - private QueryResult runKeyQuery(final Query query, - final PositionBound positionBound, - final QueryConfig config) { + protected QueryResult runKeyQuery(final Query query, + final PositionBound positionBound, + final QueryConfig config) { final QueryResult result; final KeyQuery typedKeyQuery = (KeyQuery) query; final KeyQuery rawKeyQuery = diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredVersionedKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredVersionedKeyValueStore.java new file mode 100644 index 0000000000000..f6c132cf09c3a --- /dev/null +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredVersionedKeyValueStore.java @@ -0,0 +1,231 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.streams.state.internals; + +import static org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl.maybeMeasureLatency; + +import java.util.Objects; +import org.apache.kafka.common.serialization.Serde; +import org.apache.kafka.common.utils.Time; +import org.apache.kafka.streams.errors.ProcessorStateException; +import org.apache.kafka.streams.processor.ProcessorContext; +import org.apache.kafka.streams.processor.StateStore; +import org.apache.kafka.streams.processor.StateStoreContext; +import org.apache.kafka.streams.processor.internals.SerdeGetter; +import org.apache.kafka.streams.query.Position; +import org.apache.kafka.streams.query.PositionBound; +import org.apache.kafka.streams.query.Query; +import org.apache.kafka.streams.query.QueryConfig; +import org.apache.kafka.streams.query.QueryResult; +import org.apache.kafka.streams.state.KeyValueStore; +import org.apache.kafka.streams.state.TimestampedKeyValueStore; +import org.apache.kafka.streams.state.ValueAndTimestamp; +import org.apache.kafka.streams.state.VersionedBytesStore; +import org.apache.kafka.streams.state.VersionedKeyValueStore; +import org.apache.kafka.streams.state.VersionedRecord; + +/** + * A metered {@link VersionedKeyValueStore} wrapper that is used for recording operation + * metrics, and hence its inner {@link VersionedBytesStore} implementation does not need to provide + * its own metrics collecting functionality. The inner {@code VersionedBytesStore} of this class + * is a {@link KeyValueStore} of type <Bytes,byte[]>, so we use {@link Serde}s + * to convert from <K,ValueAndTimestamp<V>> to <Bytes,byte[]>. In particular, + * {@link NullableValueAndTimestampSerde} is used since putting a tombstone to a versioned key-value + * store requires putting a null value associated with a timestamp. + * + * @param The key type + * @param The (raw) value type + */ +public class MeteredVersionedKeyValueStore + extends WrappedStateStore + implements VersionedKeyValueStore { + + private final MeteredVersionedKeyValueStoreInternal internal; + + MeteredVersionedKeyValueStore(final VersionedBytesStore inner, + final String metricScope, + final Time time, + final Serde keySerde, + final Serde> valueSerde) { + super(inner); + internal = new MeteredVersionedKeyValueStoreInternal(inner, metricScope, time, keySerde, valueSerde); + } + + /** + * Conceptually, {@link MeteredVersionedKeyValueStore} should {@code extend} + * {@link MeteredKeyValueStore}, but due to type conflicts, we cannot do this. (Specifically, + * the first needs to be {@link VersionedKeyValueStore} while the second is {@link KeyValueStore} + * and the two interfaces conflict.) Thus, we use an internal instance of + * {@code MeteredKeyValueStore} to mimic inheritance instead. + *

+ * It's not ideal because it requires an extra step to translate between the APIs of + * {@link VersionedKeyValueStore} in {@link MeteredVersionedKeyValueStore} and + * the APIs of {@link TimestampedKeyValueStore} in {@link MeteredVersionedKeyValueStoreInternal}. + * This extra step is all that the methods of {@code MeteredVersionedKeyValueStoreInternal} do. + *

+ * Note that the addition of {@link #get(Object, long)} and {@link #delete(Object, long)} in + * this class are to match the interface of {@link VersionedKeyValueStore}. + */ + private class MeteredVersionedKeyValueStoreInternal + extends MeteredKeyValueStore> { + + private final VersionedBytesStore inner; + + MeteredVersionedKeyValueStoreInternal(final VersionedBytesStore inner, + final String metricScope, + final Time time, + final Serde keySerde, + final Serde> valueSerde) { + super(inner, metricScope, time, keySerde, valueSerde); + this.inner = inner; + } + + @Override + public void put(final K key, final ValueAndTimestamp value) { + if (value == null) { + throw new IllegalStateException("Versioned store requires timestamp associated with all puts, including tombstones/deletes"); + } + super.put(key, value); + } + + public ValueAndTimestamp get(final K key, final long asOfTimestamp) { + Objects.requireNonNull(key, "key cannot be null"); + try { + return maybeMeasureLatency(() -> outerValue(inner.get(keyBytes(key), asOfTimestamp)), time, getSensor); + } catch (final ProcessorStateException e) { + final String message = String.format(e.getMessage(), key); + throw new ProcessorStateException(message, e); + } + } + + public ValueAndTimestamp delete(final K key, final long timestamp) { + Objects.requireNonNull(key, "key cannot be null"); + try { + return maybeMeasureLatency(() -> outerValue(inner.delete(keyBytes(key), timestamp)), time, deleteSensor); + } catch (final ProcessorStateException e) { + final String message = String.format(e.getMessage(), key); + throw new ProcessorStateException(message, e); + } + } + + @Override + protected QueryResult runRangeQuery(final Query query, + final PositionBound positionBound, + final QueryConfig config) { + // throw exception for now to reserve the ability to implement this in the future + // without clashing with users' custom implementations in the meantime + throw new UnsupportedOperationException("Versioned stores do not support RangeQuery queries at this time."); + } + + @Override + protected QueryResult runKeyQuery(final Query query, + final PositionBound positionBound, + final QueryConfig config) { + // throw exception for now to reserve the ability to implement this in the future + // without clashing with users' custom implementations in the meantime + throw new UnsupportedOperationException("Versioned stores do not support KeyQuery queries at this time."); + } + + @SuppressWarnings("unchecked") + @Override + protected Serde> prepareValueSerdeForStore( + final Serde> valueSerde, + final SerdeGetter getter + ) { + if (valueSerde == null) { + return new NullableValueAndTimestampSerde<>((Serde) getter.valueSerde()); + } else { + return super.prepareValueSerdeForStore(valueSerde, getter); + } + } + } + + @Override + public void put(final K key, final V value, final long timestamp) { + internal.put(key, ValueAndTimestamp.makeAllowNullable(value, timestamp)); + } + + @Override + public VersionedRecord delete(final K key, final long timestamp) { + final ValueAndTimestamp valueAndTimestamp = internal.delete(key, timestamp); + return valueAndTimestamp == null + ? null + : new VersionedRecord<>(valueAndTimestamp.value(), valueAndTimestamp.timestamp()); + } + + @Override + public VersionedRecord get(final K key) { + final ValueAndTimestamp valueAndTimestamp = internal.get(key); + return valueAndTimestamp == null + ? null + : new VersionedRecord<>(valueAndTimestamp.value(), valueAndTimestamp.timestamp()); + } + + @Override + public VersionedRecord get(final K key, final long asOfTimestamp) { + final ValueAndTimestamp valueAndTimestamp = internal.get(key, asOfTimestamp); + return valueAndTimestamp == null + ? null + : new VersionedRecord<>(valueAndTimestamp.value(), valueAndTimestamp.timestamp()); + } + + @Override + public String name() { + return internal.name(); + } + + @Deprecated + @Override + public void init(final ProcessorContext context, final StateStore root) { + internal.init(context, root); + } + + @Override + public void init(final StateStoreContext context, final StateStore root) { + internal.init(context, root); + } + + @Override + public void flush() { + internal.flush(); + } + + @Override + public void close() { + internal.close(); + } + + @Override + public boolean persistent() { + return internal.persistent(); + } + + @Override + public boolean isOpen() { + return internal.isOpen(); + } + + @Override + public QueryResult query(final Query query, final PositionBound positionBound, final QueryConfig config) { + return internal.query(query, positionBound, config); + } + + @Override + public Position getPosition() { + return internal.getPosition(); + } +} \ No newline at end of file diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdaterTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdaterTest.java index b0c0ba7c1567e..b340711420909 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdaterTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/DefaultStateUpdaterTest.java @@ -17,7 +17,9 @@ package org.apache.kafka.streams.processor.internals; import org.apache.kafka.clients.producer.ProducerConfig; +import org.apache.kafka.common.MetricName; import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.metrics.Metrics; import org.apache.kafka.common.utils.MockTime; import org.apache.kafka.common.utils.Time; import org.apache.kafka.streams.StreamsConfig; @@ -26,6 +28,7 @@ import org.apache.kafka.streams.processor.TaskId; import org.apache.kafka.streams.processor.internals.StateUpdater.ExceptionAndTasks; import org.apache.kafka.streams.processor.internals.Task.State; +import org.hamcrest.Matcher; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.mockito.InOrder; @@ -35,6 +38,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Properties; @@ -51,6 +55,10 @@ import static org.apache.kafka.test.StreamsTestUtils.TaskBuilder.statelessTask; import static org.apache.kafka.test.StreamsTestUtils.TopologyMetadataBuilder.unnamedTopology; import static org.apache.kafka.test.TestUtils.waitForCondition; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -58,7 +66,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.Mockito.atLeast; -import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -76,6 +84,7 @@ class DefaultStateUpdaterTest { private final static TopicPartition TOPIC_PARTITION_A_0 = new TopicPartition("topicA", 0); private final static TopicPartition TOPIC_PARTITION_A_1 = new TopicPartition("topicA", 1); private final static TopicPartition TOPIC_PARTITION_B_0 = new TopicPartition("topicB", 0); + private final static TopicPartition TOPIC_PARTITION_B_1 = new TopicPartition("topicB", 1); private final static TopicPartition TOPIC_PARTITION_C_0 = new TopicPartition("topicC", 0); private final static TopicPartition TOPIC_PARTITION_D_0 = new TopicPartition("topicD", 0); private final static TaskId TASK_0_0 = new TaskId(0, 0); @@ -84,14 +93,18 @@ class DefaultStateUpdaterTest { private final static TaskId TASK_1_0 = new TaskId(1, 0); private final static TaskId TASK_1_1 = new TaskId(1, 1); private final static TaskId TASK_A_0_0 = new TaskId(0, 0, "A"); + private final static TaskId TASK_A_0_1 = new TaskId(0, 1, "A"); private final static TaskId TASK_B_0_0 = new TaskId(0, 0, "B"); + private final static TaskId TASK_B_0_1 = new TaskId(0, 1, "B"); // need an auto-tick timer to work for draining with timeout private final Time time = new MockTime(1L); + private final Metrics metrics = new Metrics(time); private final StreamsConfig config = new StreamsConfig(configProps(COMMIT_INTERVAL)); private final ChangelogReader changelogReader = mock(ChangelogReader.class); private final TopologyMetadata topologyMetadata = unnamedTopology().build(); - private DefaultStateUpdater stateUpdater = new DefaultStateUpdater("test-state-updater", config, changelogReader, topologyMetadata, time); + private DefaultStateUpdater stateUpdater = + new DefaultStateUpdater("test-state-updater", metrics, config, changelogReader, topologyMetadata, time); @AfterEach public void tearDown() { @@ -149,7 +162,7 @@ public void shouldRemoveTasksFromAndClearInputQueueOnShutdown() throws Exception @Test public void shouldRemoveUpdatingTasksOnShutdown() throws Exception { stateUpdater.shutdown(Duration.ofMillis(Long.MAX_VALUE)); - stateUpdater = new DefaultStateUpdater("test-state-updater", new StreamsConfig(configProps(Integer.MAX_VALUE)), changelogReader, topologyMetadata, time); + stateUpdater = new DefaultStateUpdater("test-state-updater", metrics, new StreamsConfig(configProps(Integer.MAX_VALUE)), changelogReader, topologyMetadata, time); final StreamTask activeTask = statefulTask(TASK_0_0, mkSet(TOPIC_PARTITION_A_0)).inState(State.RESTORING).build(); final StandbyTask standbyTask = standbyTask(TASK_0_2, mkSet(TOPIC_PARTITION_C_0)).inState(State.RUNNING).build(); when(changelogReader.completedChangelogs()).thenReturn(Collections.emptySet()); @@ -643,7 +656,7 @@ public void shouldUpdateStandbyTaskAfterAllActiveStatefulTasksFailed() throws Ex mkEntry(activeTask2.id(), activeTask2), mkEntry(standbyTask.id(), standbyTask) ); - doThrow(taskCorruptedException).doNothing().when(changelogReader).restore(updatingTasks1); + doThrow(taskCorruptedException).doReturn(0L).when(changelogReader).restore(updatingTasks1); when(changelogReader.allChangelogsCompleted()).thenReturn(false); stateUpdater.start(); @@ -670,7 +683,7 @@ public void shouldNotTransitToStandbyAgainAfterStandbyTaskFailed() throws Except final TaskCorruptedException taskCorruptedException = new TaskCorruptedException(mkSet(task1.id())); final ExceptionAndTasks expectedExceptionAndTasks = new ExceptionAndTasks(mkSet(task1), taskCorruptedException); when(changelogReader.allChangelogsCompleted()).thenReturn(false); - doThrow(taskCorruptedException).doNothing().when(changelogReader).restore(updatingTasks); + doThrow(taskCorruptedException).doReturn(0L).when(changelogReader).restore(updatingTasks); stateUpdater.start(); stateUpdater.add(task1); @@ -828,7 +841,7 @@ private void shouldNotRemoveTaskFromFailedTasks(final Task task) throws Exceptio mkEntry(controlTask.id(), controlTask) ); doThrow(streamsException) - .doNothing() + .doReturn(0L) .when(changelogReader).restore(updatingTasks); stateUpdater.start(); @@ -970,7 +983,7 @@ private void shouldNotPauseTaskInFailedTasks(final Task task) throws Exception { mkEntry(controlTask.id(), controlTask) ); doThrow(streamsException) - .doNothing() + .doReturn(0L) .when(changelogReader).restore(updatingTasks); stateUpdater.start(); @@ -1133,7 +1146,7 @@ private void shouldNotResumeTaskInFailedTasks(final Task task) throws Exception mkEntry(controlTask.id(), controlTask) ); doThrow(streamsException) - .doNothing() + .doReturn(0L) .when(changelogReader).restore(updatingTasks); stateUpdater.start(); @@ -1184,7 +1197,7 @@ public void shouldAddFailedTasksToQueueWhenRestoreThrowsStreamsExceptionWithoutT mkEntry(task1.id(), task1), mkEntry(task2.id(), task2) ); - doNothing().doThrow(streamsException).when(changelogReader).restore(updatingTasks); + doReturn(0L).doThrow(streamsException).when(changelogReader).restore(updatingTasks); stateUpdater.start(); stateUpdater.add(task1); @@ -1215,10 +1228,10 @@ public void shouldAddFailedTasksToQueueWhenRestoreThrowsStreamsExceptionWithTask mkEntry(task2.id(), task2), mkEntry(task3.id(), task3) ); - doNothing() + doReturn(0L) .doThrow(streamsException1) .when(changelogReader).restore(updatingTasksBeforeFirstThrow); - doNothing() + doReturn(0L) .doThrow(streamsException2) .when(changelogReader).restore(updatingTasksBeforeSecondThrow); stateUpdater.start(); @@ -1248,7 +1261,7 @@ public void shouldAddFailedTasksToQueueWhenRestoreThrowsTaskCorruptedException() mkEntry(task2.id(), task2), mkEntry(task3.id(), task3) ); - doNothing().doThrow(taskCorruptedException).doNothing().when(changelogReader).restore(updatingTasks); + doReturn(0L).doThrow(taskCorruptedException).doReturn(0L).when(changelogReader).restore(updatingTasks); stateUpdater.start(); stateUpdater.add(task1); @@ -1362,7 +1375,7 @@ public void shouldAutoCheckpointTasksOnInterval() throws Exception { public void shouldNotAutoCheckpointTasksIfIntervalNotElapsed() { // we need to use a non auto-ticking timer here to control how much time elapsed exactly final Time time = new MockTime(); - final DefaultStateUpdater stateUpdater = new DefaultStateUpdater("test-state-updater", config, changelogReader, topologyMetadata, time); + final DefaultStateUpdater stateUpdater = new DefaultStateUpdater("test-state-updater", metrics, config, changelogReader, topologyMetadata, time); try { final StreamTask task1 = statefulTask(TASK_0_0, mkSet(TOPIC_PARTITION_A_0)).inState(State.RESTORING).build(); final StreamTask task2 = statefulTask(TASK_0_2, mkSet(TOPIC_PARTITION_B_0)).inState(State.RESTORING).build(); @@ -1466,11 +1479,11 @@ public void shouldGetTasksFromExceptionsAndFailedTasks() throws Exception { mkEntry(standbyTask1.id(), standbyTask1), mkEntry(standbyTask2.id(), standbyTask2) ); - doNothing().doThrow(taskCorruptedException).doNothing().when(changelogReader).restore(updatingTasks1); + doReturn(0L).doThrow(taskCorruptedException).doReturn(0L).when(changelogReader).restore(updatingTasks1); final Map updatingTasks2 = mkMap( mkEntry(activeTask1.id(), activeTask1) ); - doNothing().doThrow(streamsException).doNothing().when(changelogReader).restore(updatingTasks2); + doReturn(0L).doThrow(streamsException).doReturn(0L).when(changelogReader).restore(updatingTasks2); stateUpdater.start(); stateUpdater.add(standbyTask1); stateUpdater.add(activeTask1); @@ -1526,6 +1539,114 @@ public void shouldGetTasksFromPausedTasks() throws Exception { verifyGetTasks(mkSet(activeTask), mkSet(standbyTask)); } + @Test + public void shouldRecordMetrics() throws Exception { + final StreamTask activeTask1 = statefulTask(TASK_A_0_0, mkSet(TOPIC_PARTITION_A_0)).inState(State.RESTORING).build(); + final StreamTask activeTask2 = statefulTask(TASK_B_0_0, mkSet(TOPIC_PARTITION_B_0)).inState(State.RESTORING).build(); + final StandbyTask standbyTask3 = standbyTask(TASK_A_0_1, mkSet(TOPIC_PARTITION_A_1)).inState(State.RUNNING).build(); + final StandbyTask standbyTask4 = standbyTask(TASK_B_0_1, mkSet(TOPIC_PARTITION_B_1)).inState(State.RUNNING).build(); + final Map tasks1234 = mkMap( + mkEntry(activeTask1.id(), activeTask1), + mkEntry(activeTask2.id(), activeTask2), + mkEntry(standbyTask3.id(), standbyTask3), + mkEntry(standbyTask4.id(), standbyTask4) + ); + final Map tasks13 = mkMap( + mkEntry(activeTask1.id(), activeTask1), + mkEntry(standbyTask3.id(), standbyTask3) + ); + + when(topologyMetadata.isPaused("B")).thenReturn(true); + when(changelogReader.completedChangelogs()).thenReturn(Collections.emptySet()); + when(changelogReader.allChangelogsCompleted()).thenReturn(false); + when(changelogReader.restore(tasks1234)).thenReturn(1L); + when(changelogReader.restore(tasks13)).thenReturn(1L); + when(changelogReader.isRestoringActive()).thenReturn(true); + stateUpdater.start(); + stateUpdater.add(activeTask1); + stateUpdater.add(activeTask2); + stateUpdater.add(standbyTask3); + stateUpdater.add(standbyTask4); + + verifyPausedTasks(activeTask2, standbyTask4); + assertThat(metrics.metrics().size(), is(11)); + + final Map tagMap = new LinkedHashMap<>(); + tagMap.put("thread-id", "test-state-updater"); + + MetricName metricName = new MetricName("active-restoring-tasks", + "stream-state-updater-metrics", + "The number of active tasks currently undergoing restoration", + tagMap); + verifyMetric(metrics, metricName, is(1.0)); + + metricName = new MetricName("standby-updating-tasks", + "stream-state-updater-metrics", + "The number of standby tasks currently undergoing state update", + tagMap); + verifyMetric(metrics, metricName, is(1.0)); + + metricName = new MetricName("active-paused-tasks", + "stream-state-updater-metrics", + "The number of active tasks paused restoring", + tagMap); + verifyMetric(metrics, metricName, is(1.0)); + + metricName = new MetricName("standby-paused-tasks", + "stream-state-updater-metrics", + "The number of standby tasks paused state update", + tagMap); + verifyMetric(metrics, metricName, is(1.0)); + + metricName = new MetricName("idle-ratio", + "stream-state-updater-metrics", + "The fraction of time the thread spent on being idle", + tagMap); + verifyMetric(metrics, metricName, greaterThanOrEqualTo(0.0d)); + + metricName = new MetricName("active-restore-ratio", + "stream-state-updater-metrics", + "The fraction of time the thread spent on restoring active tasks", + tagMap); + verifyMetric(metrics, metricName, greaterThanOrEqualTo(0.0d)); + + metricName = new MetricName("standby-update-ratio", + "stream-state-updater-metrics", + "The fraction of time the thread spent on updating standby tasks", + tagMap); + verifyMetric(metrics, metricName, is(0.0d)); + + metricName = new MetricName("checkpoint-ratio", + "stream-state-updater-metrics", + "The fraction of time the thread spent on checkpointing tasks restored progress", + tagMap); + verifyMetric(metrics, metricName, greaterThanOrEqualTo(0.0d)); + + metricName = new MetricName("restore-records-rate", + "stream-state-updater-metrics", + "The average per-second number of records restored", + tagMap); + verifyMetric(metrics, metricName, not(0.0d)); + + metricName = new MetricName("restore-call-rate", + "stream-state-updater-metrics", + "The average per-second number of restore calls triggered", + tagMap); + verifyMetric(metrics, metricName, not(0.0d)); + + stateUpdater.shutdown(Duration.ofMinutes(1)); + assertThat(metrics.metrics().size(), is(1)); + } + + @SuppressWarnings("unchecked") + private static void verifyMetric(final Metrics metrics, + final MetricName metricName, + final Matcher matcher) { + assertThat(metrics.metrics().get(metricName).metricName().description(), is(metricName.description())); + assertThat(metrics.metrics().get(metricName).metricName().tags(), is(metricName.tags())); + assertThat((T) metrics.metrics().get(metricName).metricValue(), matcher); + } + private void verifyGetTasks(final Set expectedActiveTasks, final Set expectedStandbyTasks) { final Set tasks = stateUpdater.getTasks(); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/MockChangelogReader.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/MockChangelogReader.java index a70420cb44b1f..8d0f8c7a6b094 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/MockChangelogReader.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/MockChangelogReader.java @@ -46,8 +46,9 @@ public void register(final Set changelogPartitions, final Proces } @Override - public void restore(final Map tasks) { + public long restore(final Map tasks) { // do nothing + return 0L; } @Override @@ -60,6 +61,11 @@ public void transitToUpdateStandby() { // do nothing } + @Override + public boolean isRestoringActive() { + return true; + } + @Override public Set completedChangelogs() { // assuming all restoring partitions are completed 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 a8768ddd3eb0d..6050de77be0b7 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 @@ -1245,7 +1245,7 @@ public void shouldNotReturnDataAfterTaskMigrated() { final ChangelogReader changelogReader = new MockChangelogReader() { @Override - public void restore(final Map tasks) { + public long restore(final Map tasks) { consumer.addRecord(new ConsumerRecord<>(topic1, 1, 11, new byte[0], new byte[0])); consumer.addRecord(new ConsumerRecord<>(topic1, 1, 12, new byte[1], new byte[0])); diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/MeteredVersionedKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/MeteredVersionedKeyValueStoreTest.java new file mode 100644 index 0000000000000..e850e1ee7deb6 --- /dev/null +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/MeteredVersionedKeyValueStoreTest.java @@ -0,0 +1,357 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.streams.state.internals; + +import static org.apache.kafka.common.utils.Utils.mkEntry; +import static org.apache.kafka.common.utils.Utils.mkMap; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.kafka.common.MetricName; +import org.apache.kafka.common.metrics.KafkaMetric; +import org.apache.kafka.common.metrics.Metrics; +import org.apache.kafka.common.metrics.Sensor; +import org.apache.kafka.common.serialization.Deserializer; +import org.apache.kafka.common.serialization.Serde; +import org.apache.kafka.common.serialization.Serdes.StringSerde; +import org.apache.kafka.common.serialization.Serializer; +import org.apache.kafka.common.utils.Bytes; +import org.apache.kafka.common.utils.MockTime; +import org.apache.kafka.common.utils.Time; +import org.apache.kafka.streams.StreamsConfig; +import org.apache.kafka.streams.processor.ProcessorContext; +import org.apache.kafka.streams.processor.StateStoreContext; +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.InternalProcessorContext; +import org.apache.kafka.streams.processor.internals.ProcessorStateManager; +import org.apache.kafka.streams.processor.internals.metrics.StreamsMetricsImpl; +import org.apache.kafka.streams.query.KeyQuery; +import org.apache.kafka.streams.query.Position; +import org.apache.kafka.streams.query.PositionBound; +import org.apache.kafka.streams.query.Query; +import org.apache.kafka.streams.query.QueryConfig; +import org.apache.kafka.streams.query.QueryResult; +import org.apache.kafka.streams.query.RangeQuery; +import org.apache.kafka.streams.state.ValueAndTimestamp; +import org.apache.kafka.streams.state.VersionedBytesStore; +import org.apache.kafka.streams.state.VersionedRecord; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.StrictStubs.class) +public class MeteredVersionedKeyValueStoreTest { + + private static final String STORE_NAME = "versioned_store"; + private static final Serde STRING_SERDE = new StringSerde(); + private static final Serde> VALUE_AND_TIMESTAMP_SERDE = new NullableValueAndTimestampSerde<>(STRING_SERDE); + private static final String METRICS_SCOPE = "scope"; + private static final String STORE_LEVEL_GROUP = "stream-state-metrics"; + private static final String APPLICATION_ID = "test-app"; + private static final TaskId TASK_ID = new TaskId(0, 0, "My-Topology"); + + private static final String KEY = "k"; + private static final String VALUE = "v"; + private static final long TIMESTAMP = 10L; + private static final Bytes RAW_KEY = new Bytes(STRING_SERDE.serializer().serialize(null, KEY)); + private static final byte[] RAW_VALUE_AND_TIMESTAMP = VALUE_AND_TIMESTAMP_SERDE.serializer() + .serialize(null, ValueAndTimestamp.make(VALUE, TIMESTAMP)); + + private final VersionedBytesStore inner = mock(VersionedBytesStore.class); + private final Metrics metrics = new Metrics(); + private final Time mockTime = new MockTime(); + private final String threadId = Thread.currentThread().getName(); + private InternalProcessorContext context = mock(InternalProcessorContext.class); + private Map tags; + + private MeteredVersionedKeyValueStore store; + + @Before + public void setUp() { + when(inner.name()).thenReturn(STORE_NAME); + when(context.metrics()).thenReturn(new StreamsMetricsImpl(metrics, "test", StreamsConfig.METRICS_LATEST, mockTime)); + when(context.applicationId()).thenReturn(APPLICATION_ID); + when(context.taskId()).thenReturn(TASK_ID); + + metrics.config().recordLevel(Sensor.RecordingLevel.DEBUG); + tags = mkMap( + mkEntry("thread-id", threadId), + mkEntry("task-id", TASK_ID.toString()), + mkEntry(METRICS_SCOPE + "-state-id", STORE_NAME) + ); + + store = newMeteredStore(inner); + store.init((StateStoreContext) context, store); + } + + private MeteredVersionedKeyValueStore newMeteredStore(final VersionedBytesStore inner) { + return new MeteredVersionedKeyValueStore<>( + inner, + METRICS_SCOPE, + mockTime, + STRING_SERDE, + VALUE_AND_TIMESTAMP_SERDE + ); + } + + @SuppressWarnings("deprecation") + @Test + public void shouldDelegateDeprecatedInit() { + // recreate store in order to re-init + store.close(); + final VersionedBytesStore mockInner = mock(VersionedBytesStore.class); + store = newMeteredStore(mockInner); + + store.init((ProcessorContext) context, store); + + verify(mockInner).init((ProcessorContext) context, store); + } + + @Test + public void shouldDelegateInit() { + // init is already called in setUp() + verify(inner).init((StateStoreContext) context, store); + } + + @Test + public void shouldPassChangelogTopicNameToStateStoreSerde() { + final String changelogTopicName = "changelog-topic"; + when(context.changelogFor(STORE_NAME)).thenReturn(changelogTopicName); + doShouldPassChangelogTopicNameToStateStoreSerde(changelogTopicName); + } + + @Test + public void shouldPassDefaultChangelogTopicNameToStateStoreSerdeIfLoggingDisabled() { + final String defaultChangelogTopicName = ProcessorStateManager.storeChangelogTopic(APPLICATION_ID, STORE_NAME, TASK_ID.topologyName()); + when(context.changelogFor(STORE_NAME)).thenReturn(null); + doShouldPassChangelogTopicNameToStateStoreSerde(defaultChangelogTopicName); + } + + @SuppressWarnings("unchecked") + private void doShouldPassChangelogTopicNameToStateStoreSerde(final String changelogTopicName) { + // recreate store with mock serdes + final Serde keySerde = mock(Serde.class); + final Serializer keySerializer = mock(Serializer.class); + final Serde valueSerde = mock(Serde.class); + final Serializer valueSerializer = mock(Serializer.class); + final Deserializer valueDeserializer = mock(Deserializer.class); + when(keySerde.serializer()).thenReturn(keySerializer); + when(valueSerde.serializer()).thenReturn(valueSerializer); + when(valueSerde.deserializer()).thenReturn(valueDeserializer); + + store.close(); + store = new MeteredVersionedKeyValueStore<>( + inner, + METRICS_SCOPE, + mockTime, + keySerde, + new NullableValueAndTimestampSerde<>(valueSerde) + ); + store.init((StateStoreContext) context, store); + + store.put(KEY, VALUE, TIMESTAMP); + + verify(keySerializer).serialize(changelogTopicName, KEY); + verify(valueSerializer).serialize(changelogTopicName, VALUE); + } + + @Test + public void shouldRecordMetricsOnInit() { + // init is called in setUp(). it suffices to verify one restore metric since all restore + // metrics are recorded by the same sensor, and the sensor is tested elsewhere. + assertThat((Double) getMetric("restore-rate").metricValue(), greaterThan(0.0)); + } + + @Test + public void shouldDelegateAndRecordMetricsOnPut() { + store.put(KEY, VALUE, TIMESTAMP); + + verify(inner).put(RAW_KEY, RAW_VALUE_AND_TIMESTAMP); + assertThat((Double) getMetric("put-rate").metricValue(), greaterThan(0.0)); + } + + @Test + public void shouldDelegateAndRecordMetricsOnDelete() { + when(inner.delete(RAW_KEY, TIMESTAMP)).thenReturn(RAW_VALUE_AND_TIMESTAMP); + + final VersionedRecord result = store.delete(KEY, TIMESTAMP); + + assertThat(result, is(new VersionedRecord<>(VALUE, TIMESTAMP))); + assertThat((Double) getMetric("delete-rate").metricValue(), greaterThan(0.0)); + } + + @Test + public void shouldDelegateAndRecordMetricsOnGet() { + when(inner.get(RAW_KEY)).thenReturn(RAW_VALUE_AND_TIMESTAMP); + + final VersionedRecord result = store.get(KEY); + + assertThat(result, is(new VersionedRecord<>(VALUE, TIMESTAMP))); + assertThat((Double) getMetric("get-rate").metricValue(), greaterThan(0.0)); + } + + @Test + public void shouldDelegateAndRecordMetricsOnGetWithTimestamp() { + when(inner.get(RAW_KEY, TIMESTAMP)).thenReturn(RAW_VALUE_AND_TIMESTAMP); + + final VersionedRecord result = store.get(KEY, TIMESTAMP); + + assertThat(result, is(new VersionedRecord<>(VALUE, TIMESTAMP))); + assertThat((Double) getMetric("get-rate").metricValue(), greaterThan(0.0)); + } + + @Test + public void shouldDelegateAndRecordMetricsOnFlush() { + store.flush(); + + verify(inner).flush(); + assertThat((Double) getMetric("flush-rate").metricValue(), greaterThan(0.0)); + } + + @Test + public void shouldDelegateAndRemoveMetricsOnClose() { + assertThat(storeMetrics(), not(empty())); + + store.close(); + + verify(inner).close(); + assertThat(storeMetrics(), empty()); + } + + @Test + public void shouldRemoveMetricsOnCloseEvenIfInnerThrows() { + doThrow(new RuntimeException("uh oh")).when(inner).close(); + assertThat(storeMetrics(), not(empty())); + + assertThrows(RuntimeException.class, () -> store.close()); + + assertThat(storeMetrics(), empty()); + } + + @Test + public void shouldNotSetFlushListenerIfInnerIsNotCaching() { + assertThat(store.setFlushListener(null, false), is(false)); + } + + @Test + public void shouldThrowNullPointerOnPutIfKeyIsNull() { + assertThrows(NullPointerException.class, () -> store.put(null, VALUE, TIMESTAMP)); + } + + @Test + public void shouldThrowNullPointerOnDeleteIfKeyIsNull() { + assertThrows(NullPointerException.class, () -> store.delete(null, TIMESTAMP)); + } + + @Test + public void shouldThrowNullPointerOnGetIfKeyIsNull() { + assertThrows(NullPointerException.class, () -> store.get(null)); + } + + @Test + public void shouldThrowNullPointerOnGetWithTimestampIfKeyIsNull() { + assertThrows(NullPointerException.class, () -> store.get(null, TIMESTAMP)); + } + + @SuppressWarnings("unchecked") + @Test + public void shouldThrowOnIQv2RangeQuery() { + assertThrows(UnsupportedOperationException.class, () -> store.query(mock(RangeQuery.class), null, null)); + } + + @SuppressWarnings("unchecked") + @Test + public void shouldThrowOnIQv2KeyQuery() { + assertThrows(UnsupportedOperationException.class, () -> store.query(mock(KeyQuery.class), null, null)); + } + + @SuppressWarnings("unchecked") + @Test + public void shouldDelegateAndAddExecutionInfoOnCustomQuery() { + final Query query = mock(Query.class); + final PositionBound positionBound = mock(PositionBound.class); + final QueryConfig queryConfig = mock(QueryConfig.class); + final QueryResult result = mock(QueryResult.class); + when(inner.query(query, positionBound, queryConfig)).thenReturn(result); + when(queryConfig.isCollectExecutionInfo()).thenReturn(true); + + assertThat(store.query(query, positionBound, queryConfig), is(result)); + verify(result).addExecutionInfo(anyString()); + } + + @Test + public void shouldDelegateName() { + when(inner.name()).thenReturn(STORE_NAME); + + assertThat(store.name(), is(STORE_NAME)); + } + + @Test + public void shouldDelegatePersistent() { + // `persistent = true` case + when(inner.persistent()).thenReturn(true); + assertThat(store.persistent(), is(true)); + + // `persistent = false` case + when(inner.persistent()).thenReturn(false); + assertThat(store.persistent(), is(false)); + } + + @Test + public void shouldDelegateIsOpen() { + // `isOpen = true` case + when(inner.isOpen()).thenReturn(true); + assertThat(store.isOpen(), is(true)); + + // `isOpen = false` case + when(inner.isOpen()).thenReturn(false); + assertThat(store.isOpen(), is(false)); + } + + @Test + public void shouldDelegateGetPosition() { + final Position position = mock(Position.class); + when(inner.getPosition()).thenReturn(position); + + assertThat(store.getPosition(), is(position)); + } + + private KafkaMetric getMetric(final String name) { + return metrics.metric(new MetricName(name, STORE_LEVEL_GROUP, "", tags)); + } + + private List storeMetrics() { + return metrics.metrics() + .keySet() + .stream() + .filter(name -> name.group().equals(STORE_LEVEL_GROUP) && name.tags().equals(tags)) + .collect(Collectors.toList()); + } +} \ No newline at end of file