From 97228a5fbdb9b8d64aa609d59c0b64e4fe7bbe0c Mon Sep 17 00:00:00 2001 From: John Roesler Date: Tue, 21 Apr 2020 17:53:51 -0500 Subject: [PATCH 01/12] KAFKA-6145: KIP-441: Add TaskAssignor class config * add a config to set the TaskAssignor * set the default assignor to HighAvailabilityTaskAssignor * fix broken tests --- build.gradle | 4 +- .../org/apache/kafka/common/utils/Utils.java | 9 + .../java/org/apache/kafka/test/TestUtils.java | 4 +- .../internals/StreamsPartitionAssignor.java | 33 +- .../assignment/AssignorConfiguration.java | 22 +- .../internals/assignment/ClientState.java | 22 +- .../HighAvailabilityTaskAssignor.java | 56 +- .../assignment/PriorTaskAssignor.java | 40 ++ .../assignment/StickyTaskAssignor.java | 36 +- .../internals/assignment/TaskAssignor.java | 11 +- .../integration/EosIntegrationTest.java | 121 +++-- .../integration/LagFetchIntegrationTest.java | 349 ------------- ...ilabilityStreamsPartitionAssignorTest.java | 326 ++++++++++++ .../StreamsPartitionAssignorTest.java | 122 +---- .../HighAvailabilityTaskAssignorTest.java | 491 +++++++++--------- .../assignment/PriorTaskAssignorTest.java | 74 +++ .../assignment/StickyTaskAssignorTest.java | 323 ++++++------ .../TaskAssignorConvergenceTest.java | 9 +- 18 files changed, 1063 insertions(+), 989 deletions(-) create mode 100644 streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignor.java delete mode 100644 streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java create mode 100644 streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java create mode 100644 streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignorTest.java diff --git a/build.gradle b/build.gradle index 644d3ebaeed82..b8a88626f3900 100644 --- a/build.gradle +++ b/build.gradle @@ -236,8 +236,10 @@ subprojects { def logStreams = new HashMap() beforeTest { TestDescriptor td -> def tid = testId(td) + // truncate the file name if it's too long def logFile = new File( - "${projectDir}/build/reports/testOutput/${tid}.test.stdout") + "${projectDir}/build/reports/testOutput/${tid.substring(0, Math.min(tid.size(),240))}.test.stdout" + ) logFile.parentFile.mkdirs() logFiles.put(tid, logFile) logStreams.put(tid, new FileOutputStream(logFile)) 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 ee627c91ef2c3..87c749a91203b 100755 --- a/clients/src/main/java/org/apache/kafka/common/utils/Utils.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/Utils.java @@ -1146,4 +1146,13 @@ public Set characteristics() { } }; } + + @SafeVarargs + public static Set union(final Supplier> constructor, final Set... set) { + final Set result = constructor.get(); + for (final Set s : set) { + result.addAll(s); + } + return result; + } } diff --git a/clients/src/test/java/org/apache/kafka/test/TestUtils.java b/clients/src/test/java/org/apache/kafka/test/TestUtils.java index fe0b4a0cdc321..e404cfd66cdf1 100644 --- a/clients/src/test/java/org/apache/kafka/test/TestUtils.java +++ b/clients/src/test/java/org/apache/kafka/test/TestUtils.java @@ -361,9 +361,9 @@ public static void waitForCondition(final TestCondition testCondition, final lon * avoid transient failures due to slow or overloaded machines. */ public static void waitForCondition(final TestCondition testCondition, final long maxWaitMs, Supplier conditionDetailsSupplier) throws InterruptedException { - String conditionDetailsSupplied = conditionDetailsSupplier != null ? conditionDetailsSupplier.get() : null; - String conditionDetails = conditionDetailsSupplied != null ? conditionDetailsSupplied : ""; retryOnExceptionWithTimeout(maxWaitMs, () -> { + String conditionDetailsSupplied = conditionDetailsSupplier != null ? conditionDetailsSupplier.get() : null; + String conditionDetails = conditionDetailsSupplied != null ? conditionDetailsSupplied : ""; assertThat("Condition not met within timeout " + maxWaitMs + ". " + conditionDetails, testCondition.conditionMet()); }); diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java index d285e31a9eca9..2bf489e570da0 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java @@ -39,8 +39,7 @@ import org.apache.kafka.streams.processor.internals.assignment.AssignorError; import org.apache.kafka.streams.processor.internals.assignment.ClientState; import org.apache.kafka.streams.processor.internals.assignment.CopartitionedTopicsEnforcer; -import org.apache.kafka.streams.processor.internals.assignment.HighAvailabilityTaskAssignor; -import org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.PriorTaskAssignor; import org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo; import org.apache.kafka.streams.processor.internals.assignment.TaskAssignor; import org.apache.kafka.streams.state.HostInfo; @@ -64,6 +63,7 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; import java.util.stream.Collectors; import static java.util.UUID.randomUUID; @@ -171,7 +171,7 @@ public String toString() { private CopartitionedTopicsEnforcer copartitionedTopicsEnforcer; private RebalanceProtocol rebalanceProtocol; - private boolean highAvailabilityEnabled; + private Supplier taskAssignor; /** * We need to have the PartitionAssignor and its StreamThread to be mutually accessible since the former needs @@ -201,7 +201,7 @@ public void configure(final Map configs) { internalTopicManager = assignorConfiguration.getInternalTopicManager(); copartitionedTopicsEnforcer = assignorConfiguration.getCopartitionedTopicsEnforcer(); rebalanceProtocol = assignorConfiguration.rebalanceProtocol(); - highAvailabilityEnabled = assignorConfiguration.isHighAvailabilityEnabled(); + taskAssignor = assignorConfiguration::getTaskAssignor; } @Override @@ -713,23 +713,18 @@ private boolean assignTasksToClients(final Set allSourceTopics, allTasks, clientStates, numStandbyReplicas()); final TaskAssignor taskAssignor; - if (highAvailabilityEnabled) { - if (lagComputationSuccessful) { - taskAssignor = new HighAvailabilityTaskAssignor( - clientStates, - allTasks, - statefulTasks, - assignmentConfigs); - } else { - log.info("Failed to fetch end offsets for changelogs, will return previous assignment to clients and " - + "trigger another rebalance to retry."); - setAssignmentErrorCode(AssignorError.REBALANCE_NEEDED.code()); - taskAssignor = new StickyTaskAssignor(clientStates, allTasks, statefulTasks, assignmentConfigs, true); - } + if (!lagComputationSuccessful) { + log.info("Failed to fetch end offsets for changelogs, will return previous assignment to clients and " + + "trigger another rebalance to retry."); + setAssignmentErrorCode(AssignorError.REBALANCE_NEEDED.code()); + taskAssignor = new PriorTaskAssignor(); } else { - taskAssignor = new StickyTaskAssignor(clientStates, allTasks, statefulTasks, assignmentConfigs, false); + taskAssignor = this.taskAssignor.get(); } - final boolean followupRebalanceNeeded = taskAssignor.assign(); + final boolean followupRebalanceNeeded = taskAssignor.assign(clientStates, + allTasks, + statefulTasks, + assignmentConfigs); log.info("Assigned tasks to clients as {}{}.", Utils.NL, clientStates.entrySet().stream().map(Map.Entry::toString).collect(Collectors.joining(Utils.NL))); diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java index d12640a4e9e04..3a4b2c9578fce 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java @@ -25,6 +25,7 @@ import org.apache.kafka.common.config.ConfigException; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.common.utils.Time; +import org.apache.kafka.common.utils.Utils; import org.apache.kafka.streams.StreamsConfig; import org.apache.kafka.streams.StreamsConfig.InternalConfig; import org.apache.kafka.streams.internals.QuietStreamsConfig; @@ -41,8 +42,8 @@ import static org.apache.kafka.streams.processor.internals.assignment.StreamsAssignmentProtocolVersions.LATEST_SUPPORTED_VERSION; public final class AssignorConfiguration { - public static final String HIGH_AVAILABILITY_ENABLED_CONFIG = "internal.high.availability.enabled"; - private final boolean highAvailabilityEnabled; + public static final String INTERNAL_TASK_ASSIGNOR_CLASS = "internal.task.assignor.class"; + private final String taskAssignorClass; private final String logPrefix; private final Logger log; @@ -162,11 +163,11 @@ public AssignorConfiguration(final Map configs) { copartitionedTopicsEnforcer = new CopartitionedTopicsEnforcer(logPrefix); { - final Object o = configs.get(HIGH_AVAILABILITY_ENABLED_CONFIG); + final String o = (String) configs.get(INTERNAL_TASK_ASSIGNOR_CLASS); if (o == null) { - highAvailabilityEnabled = false; + taskAssignorClass = HighAvailabilityTaskAssignor.class.getName(); } else { - highAvailabilityEnabled = (Boolean) o; + taskAssignorClass = o; } } } @@ -328,8 +329,15 @@ public AssignmentConfigs getAssignmentConfigs() { return assignmentConfigs; } - public boolean isHighAvailabilityEnabled() { - return highAvailabilityEnabled; + public TaskAssignor getTaskAssignor() { + try { + return Utils.newInstance(taskAssignorClass, TaskAssignor.class); + } catch (final ClassNotFoundException e) { + throw new IllegalArgumentException( + "Expected an instantiable class name for " + INTERNAL_TASK_ASSIGNOR_CLASS, + e + ); + } } public static class AssignmentConfigs { diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/ClientState.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/ClientState.java index 5b8857c78743e..3f64592a4c502 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/ClientState.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/ClientState.java @@ -29,6 +29,10 @@ import java.util.Set; import java.util.UUID; +import static java.util.Collections.emptyMap; +import static java.util.Collections.unmodifiableMap; +import static java.util.Collections.unmodifiableSet; +import static org.apache.kafka.common.utils.Utils.union; import static org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo.UNKNOWN_OFFSET_SUM; public class ClientState { @@ -86,6 +90,22 @@ private ClientState(final Set activeTasks, this.capacity = capacity; } + public ClientState(final Set previousActiveTasks, + final Set previousStandbyTasks, + final Map taskLagTotals, + final int capacity) { + activeTasks = new HashSet<>(); + standbyTasks = new HashSet<>(); + assignedTasks = new HashSet<>(); + prevActiveTasks = unmodifiableSet(new HashSet<>(previousActiveTasks)); + prevStandbyTasks = unmodifiableSet(new HashSet<>(previousStandbyTasks)); + prevAssignedTasks = unmodifiableSet(union(HashSet::new, previousActiveTasks, previousStandbyTasks)); + ownedPartitions = emptyMap(); + taskOffsetSums = emptyMap(); + this.taskLagTotals = unmodifiableMap(taskLagTotals); + this.capacity = capacity; + } + public ClientState copy() { return new ClientState( new HashSet<>(activeTasks), @@ -258,7 +278,7 @@ boolean hasUnfulfilledQuota(final int tasksPerThread) { } boolean hasMoreAvailableCapacityThan(final ClientState other) { - if (this.capacity <= 0) { + if (capacity <= 0) { throw new IllegalStateException("Capacity of this ClientState must be greater than 0."); } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java index b1570fb7e8ef6..860e33ef08ee9 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java @@ -16,49 +16,50 @@ */ package org.apache.kafka.streams.processor.internals.assignment; -import static org.apache.kafka.streams.processor.internals.assignment.AssignmentUtils.taskIsCaughtUpOnClientOrNoCaughtUpClientsExist; -import static org.apache.kafka.streams.processor.internals.assignment.RankedClient.buildClientRankingsByTask; -import static org.apache.kafka.streams.processor.internals.assignment.RankedClient.tasksToCaughtUpClients; -import static org.apache.kafka.streams.processor.internals.assignment.TaskMovement.assignTaskMovements; +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.SortedMap; import java.util.SortedSet; import java.util.TreeSet; import java.util.UUID; import java.util.stream.Collectors; -import org.apache.kafka.streams.processor.TaskId; -import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import java.util.Map; -import java.util.Set; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentUtils.taskIsCaughtUpOnClientOrNoCaughtUpClientsExist; +import static org.apache.kafka.streams.processor.internals.assignment.RankedClient.buildClientRankingsByTask; +import static org.apache.kafka.streams.processor.internals.assignment.RankedClient.tasksToCaughtUpClients; +import static org.apache.kafka.streams.processor.internals.assignment.TaskMovement.assignTaskMovements; public class HighAvailabilityTaskAssignor implements TaskAssignor { private static final Logger log = LoggerFactory.getLogger(HighAvailabilityTaskAssignor.class); - private final Map clientStates; - private final Map clientsToNumberOfThreads; - private final SortedSet sortedClients; + private Map clientStates; + private Map clientsToNumberOfThreads; + private SortedSet sortedClients; - private final Set allTasks; - private final SortedSet statefulTasks; - private final SortedSet statelessTasks; + private Set allTasks; + private SortedSet statefulTasks; + private SortedSet statelessTasks; - private final AssignmentConfigs configs; + private AssignmentConfigs configs; - private final SortedMap> statefulTasksToRankedCandidates; - private final Map> tasksToCaughtUpClients; + private SortedMap> statefulTasksToRankedCandidates; + private Map> tasksToCaughtUpClients; - public HighAvailabilityTaskAssignor(final Map clientStates, - final Set allTasks, - final Set statefulTasks, - final AssignmentConfigs configs) { + @Override + public boolean assign(final Map clientStates, + final Set allTasks, + final Set statefulTasks, + final AssignmentConfigs configs) { this.configs = configs; this.clientStates = clientStates; this.allTasks = allTasks; @@ -77,10 +78,8 @@ public HighAvailabilityTaskAssignor(final Map clientStates, statefulTasksToRankedCandidates = buildClientRankingsByTask(statefulTasks, clientStates, configs.acceptableRecoveryLag); tasksToCaughtUpClients = tasksToCaughtUpClients(statefulTasksToRankedCandidates); - } - @Override - public boolean assign() { + if (shouldUsePreviousAssignment()) { assignPreviousTasksToClientStates(); return false; @@ -95,6 +94,11 @@ public boolean assign() { assignStatelessActiveTasks(); + log.info("Decided on assignment: " + + clientStates + + " with " + + (followupRebalanceNeeded ? "" : "no") + + " followup rebalance."); return followupRebalanceNeeded; } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignor.java new file mode 100644 index 0000000000000..581637f583311 --- /dev/null +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignor.java @@ -0,0 +1,40 @@ +/* + * 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.processor.internals.assignment; + +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs; + +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +public class PriorTaskAssignor implements TaskAssignor { + private final StickyTaskAssignor delegate; + + public PriorTaskAssignor() { + delegate = new StickyTaskAssignor(true); + } + + @Override + public boolean assign(final Map clients, + final Set allTaskIds, + final Set standbyTaskIds, + final AssignmentConfigs configs) { + return delegate.assign(clients, allTaskIds, standbyTaskIds, configs); + } +} diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignor.java index 2f2c77e7d3dc4..50b9381ad918c 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignor.java @@ -16,7 +16,6 @@ */ package org.apache.kafka.streams.processor.internals.assignment; -import java.util.UUID; import org.apache.kafka.streams.processor.TaskId; import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs; import org.slf4j.Logger; @@ -32,40 +31,43 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.UUID; public class StickyTaskAssignor implements TaskAssignor { private static final Logger log = LoggerFactory.getLogger(StickyTaskAssignor.class); - private final Map clients; - private final Set allTaskIds; - private final Set standbyTaskIds; + private Map clients; + private Set allTaskIds; + private Set standbyTaskIds; private final Map previousActiveTaskAssignment = new HashMap<>(); private final Map> previousStandbyTaskAssignment = new HashMap<>(); - private final TaskPairs taskPairs; - private final int numStandbyReplicas; + private TaskPairs taskPairs; private final boolean mustPreserveActiveTaskAssignment; - public StickyTaskAssignor(final Map clients, - final Set allTaskIds, - final Set standbyTaskIds, - final AssignmentConfigs configs, - final boolean mustPreserveActiveTaskAssignment) { + public StickyTaskAssignor() { + this(false); + } + + StickyTaskAssignor(final boolean mustPreserveActiveTaskAssignment) { + this.mustPreserveActiveTaskAssignment = mustPreserveActiveTaskAssignment; + } + + @Override + public boolean assign(final Map clients, + final Set allTaskIds, + final Set standbyTaskIds, + final AssignmentConfigs configs) { this.clients = clients; this.allTaskIds = allTaskIds; this.standbyTaskIds = standbyTaskIds; - numStandbyReplicas = configs.numStandbyReplicas; - this.mustPreserveActiveTaskAssignment = mustPreserveActiveTaskAssignment; final int maxPairs = allTaskIds.size() * (allTaskIds.size() - 1) / 2; taskPairs = new TaskPairs(maxPairs); mapPreviousTaskAssignment(clients); - } - @Override - public boolean assign() { assignActive(); - assignStandby(numStandbyReplicas); + assignStandby(configs.numStandbyReplicas); return false; } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java index cbecc24ae38df..64522872f967e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java @@ -16,9 +16,18 @@ */ package org.apache.kafka.streams.processor.internals.assignment; +import org.apache.kafka.streams.processor.TaskId; + +import java.util.Map; +import java.util.Set; +import java.util.UUID; + public interface TaskAssignor { /** * @return whether the generated assignment requires a followup rebalance to satisfy all conditions */ - boolean assign(); + boolean assign(Map clients, + Set allTaskIds, + Set standbyTaskIds, + AssignorConfiguration.AssignmentConfigs configs); } diff --git a/streams/src/test/java/org/apache/kafka/streams/integration/EosIntegrationTest.java b/streams/src/test/java/org/apache/kafka/streams/integration/EosIntegrationTest.java index 747ac40cc3fce..e8ab668ee31a3 100644 --- a/streams/src/test/java/org/apache/kafka/streams/integration/EosIntegrationTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/integration/EosIntegrationTest.java @@ -59,6 +59,8 @@ import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.File; import java.util.ArrayList; @@ -74,6 +76,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import static org.apache.kafka.common.utils.Utils.mkEntry; import static org.apache.kafka.common.utils.Utils.mkMap; @@ -89,6 +92,7 @@ @RunWith(Parameterized.class) @Category({IntegrationTest.class}) public class EosIntegrationTest { + private static final Logger LOG = LoggerFactory.getLogger(EosIntegrationTest.class); private static final int NUM_BROKERS = 3; private static final int MAX_POLL_INTERVAL_MS = 5 * 1000; private static final int MAX_WAIT_TIME_MS = 60 * 1000; @@ -111,8 +115,9 @@ public class EosIntegrationTest { private final String storeName = "store"; private AtomicBoolean errorInjected; - private AtomicBoolean gcInjected; - private volatile boolean doGC = true; + private AtomicBoolean stallInjected; + private AtomicReference stallingHost; + private volatile boolean doStall = true; private AtomicInteger commitRequested; private Throwable uncaughtException; @@ -382,7 +387,7 @@ public void shouldNotViolateEosIfOneTaskFails() throws Exception { // -> the failure only kills one thread // after fail over, we should read 40 committed records (even if 50 record got written) - try (final KafkaStreams streams = getKafkaStreams(false, "appDir", 2, eosConfig)) { + try (final KafkaStreams streams = getKafkaStreams("dummy", false, "appDir", 2, eosConfig)) { startKafkaStreamsAndWaitForRunningState(streams, MAX_WAIT_TIME_MS); final List> committedDataBeforeFailure = prepareData(0L, 10L, 0L, 1L); @@ -450,7 +455,7 @@ public void shouldNotViolateEosIfOneTaskFailsWithState() throws Exception { // after fail over, we should read 40 committed records and the state stores should contain the correct sums // per key (even if some records got processed twice) - try (final KafkaStreams streams = getKafkaStreams(true, "appDir", 2, eosConfig)) { + try (final KafkaStreams streams = getKafkaStreams("dummy", true, "appDir", 2, eosConfig)) { startKafkaStreamsAndWaitForRunningState(streams, MAX_WAIT_TIME_MS); final List> committedDataBeforeFailure = prepareData(0L, 10L, 0L, 1L); @@ -515,84 +520,114 @@ public void shouldNotViolateEosIfOneTaskGetsFencedUsingIsolatedAppInstances() th // the app is supposed to copy all 60 records into the output topic // the app commits after each 10 records per partition, and thus will have 2*5 uncommitted writes // - // a GC pause gets inject after 20 committed and 30 uncommitted records got received - // -> the GC pause only affects one thread and should trigger a rebalance + // a stall gets injected after 20 committed and 30 uncommitted records got received + // -> the stall only affects one thread and should trigger a rebalance // after rebalancing, we should read 40 committed records (even if 50 record got written) // // afterwards, the "stalling" thread resumes, and another rebalance should get triggered // we write the remaining 20 records and verify to read 60 result records try ( - final KafkaStreams streams1 = getKafkaStreams(false, "appDir1", 1, eosConfig); - final KafkaStreams streams2 = getKafkaStreams(false, "appDir2", 1, eosConfig) + final KafkaStreams streams1 = getKafkaStreams("streams1", false, "appDir1", 1, eosConfig); + final KafkaStreams streams2 = getKafkaStreams("streams2", false, "appDir2", 1, eosConfig) ) { startKafkaStreamsAndWaitForRunningState(streams1, MAX_WAIT_TIME_MS); startKafkaStreamsAndWaitForRunningState(streams2, MAX_WAIT_TIME_MS); - final List> committedDataBeforeGC = prepareData(0L, 10L, 0L, 1L); - final List> uncommittedDataBeforeGC = prepareData(10L, 15L, 0L, 1L); + final List> committedDataBeforeStall = prepareData(0L, 10L, 0L, 1L); + final List> uncommittedDataBeforeStall = prepareData(10L, 15L, 0L, 1L); - final List> dataBeforeGC = new ArrayList<>(); - dataBeforeGC.addAll(committedDataBeforeGC); - dataBeforeGC.addAll(uncommittedDataBeforeGC); + final List> dataBeforeStall = new ArrayList<>(); + dataBeforeStall.addAll(committedDataBeforeStall); + dataBeforeStall.addAll(uncommittedDataBeforeStall); final List> dataToTriggerFirstRebalance = prepareData(15L, 20L, 0L, 1L); final List> dataAfterSecondRebalance = prepareData(20L, 30L, 0L, 1L); - writeInputData(committedDataBeforeGC); + writeInputData(committedDataBeforeStall); waitForCondition( () -> commitRequested.get() == 2, MAX_WAIT_TIME_MS, "SteamsTasks did not request commit."); - writeInputData(uncommittedDataBeforeGC); + writeInputData(uncommittedDataBeforeStall); - final List> uncommittedRecords = readResult(dataBeforeGC.size(), null); - final List> committedRecords = readResult(committedDataBeforeGC.size(), CONSUMER_GROUP_ID); + final List> uncommittedRecords = readResult(dataBeforeStall.size(), null); + final List> committedRecords = readResult(committedDataBeforeStall.size(), CONSUMER_GROUP_ID); - checkResultPerKey(committedRecords, committedDataBeforeGC); - checkResultPerKey(uncommittedRecords, dataBeforeGC); + checkResultPerKey(committedRecords, committedDataBeforeStall); + checkResultPerKey(uncommittedRecords, dataBeforeStall); - gcInjected.set(true); + LOG.info("Injecting Stall"); + stallInjected.set(true); writeInputData(dataToTriggerFirstRebalance); + LOG.info("Input Data Written"); + waitForCondition( + () -> stallingHost.get() != null, + MAX_WAIT_TIME_MS, + "Expected a host to start stalling" + ); + final String observedStallingHost = stallingHost.get(); + final KafkaStreams stallingInstance; + final KafkaStreams remainingInstance; + if ("streams1".equals(observedStallingHost)) { + stallingInstance = streams1; + remainingInstance = streams2; + } else if ("streams2".equals(observedStallingHost)) { + stallingInstance = streams2; + remainingInstance = streams1; + } else { + throw new IllegalArgumentException("unexpected host name: " + observedStallingHost); + } + // the stalling instance won't have an updated view, and it doesn't matter what it thinks + // the assignment is. We only really care that the remaining instance only sees one host + // that owns both partitions. waitForCondition( - () -> streams1.allMetadata().size() == 1 - && streams2.allMetadata().size() == 1 - && (streams1.allMetadata().iterator().next().topicPartitions().size() == 2 - || streams2.allMetadata().iterator().next().topicPartitions().size() == 2), - MAX_WAIT_TIME_MS, "Should have rebalanced."); + () -> stallingInstance.allMetadata().size() == 2 + && remainingInstance.allMetadata().size() == 1 + && remainingInstance.allMetadata().iterator().next().topicPartitions().size() == 2, + MAX_WAIT_TIME_MS, + () -> "Should have rebalanced.\n" + + "Streams1[" + streams1.allMetadata() + "]\n" + + "Streams2[" + streams2.allMetadata() + "]"); final List> committedRecordsAfterRebalance = readResult( - uncommittedDataBeforeGC.size() + dataToTriggerFirstRebalance.size(), + uncommittedDataBeforeStall.size() + dataToTriggerFirstRebalance.size(), CONSUMER_GROUP_ID); final List> expectedCommittedRecordsAfterRebalance = new ArrayList<>(); - expectedCommittedRecordsAfterRebalance.addAll(uncommittedDataBeforeGC); + expectedCommittedRecordsAfterRebalance.addAll(uncommittedDataBeforeStall); expectedCommittedRecordsAfterRebalance.addAll(dataToTriggerFirstRebalance); checkResultPerKey(committedRecordsAfterRebalance, expectedCommittedRecordsAfterRebalance); - doGC = false; + LOG.info("Releasing Stall"); + doStall = false; + // Once the stalling host rejoins the group, we expect both instances to see both instances. + // It doesn't really matter what the assignment is, but we might as well also assert that they + // both see both partitions assigned exactly once waitForCondition( - () -> streams1.allMetadata().size() == 1 - && streams2.allMetadata().size() == 1 - && streams1.allMetadata().iterator().next().topicPartitions().size() == 1 - && streams2.allMetadata().iterator().next().topicPartitions().size() == 1, + () -> streams1.allMetadata().size() == 2 + && streams2.allMetadata().size() == 2 + && streams1.allMetadata().stream().mapToLong(meta -> meta.topicPartitions().size()).sum() == 2 + && streams2.allMetadata().stream().mapToLong(meta -> meta.topicPartitions().size()).sum() == 2, MAX_WAIT_TIME_MS, - "Should have rebalanced."); + () -> "Should have rebalanced.\n" + + "Streams1[" + streams1.allMetadata() + "]\n" + + "Streams2[" + streams2.allMetadata() + "]"); writeInputData(dataAfterSecondRebalance); final List> allCommittedRecords = readResult( - committedDataBeforeGC.size() + uncommittedDataBeforeGC.size() + committedDataBeforeStall.size() + uncommittedDataBeforeStall.size() + dataToTriggerFirstRebalance.size() + dataAfterSecondRebalance.size(), CONSUMER_GROUP_ID + "_ALL"); final List> allExpectedCommittedRecordsAfterRecovery = new ArrayList<>(); - allExpectedCommittedRecordsAfterRecovery.addAll(committedDataBeforeGC); - allExpectedCommittedRecordsAfterRecovery.addAll(uncommittedDataBeforeGC); + allExpectedCommittedRecordsAfterRecovery.addAll(committedDataBeforeStall); + allExpectedCommittedRecordsAfterRecovery.addAll(uncommittedDataBeforeStall); allExpectedCommittedRecordsAfterRecovery.addAll(dataToTriggerFirstRebalance); allExpectedCommittedRecordsAfterRecovery.addAll(dataAfterSecondRebalance); @@ -614,13 +649,15 @@ private List> prepareData(final long fromInclusive, return data; } - private KafkaStreams getKafkaStreams(final boolean withState, + private KafkaStreams getKafkaStreams(final String dummyHostName, + final boolean withState, final String appDir, final int numberOfStreamsThreads, final String eosConfig) { commitRequested = new AtomicInteger(0); errorInjected = new AtomicBoolean(false); - gcInjected = new AtomicBoolean(false); + stallInjected = new AtomicBoolean(false); + stallingHost = new AtomicReference<>(); final StreamsBuilder builder = new StreamsBuilder(); String[] storeNames = new String[0]; @@ -653,8 +690,10 @@ public void init(final ProcessorContext context) { @Override public KeyValue transform(final Long key, final Long value) { - if (gcInjected.compareAndSet(true, false)) { - while (doGC) { + if (stallInjected.compareAndSet(true, false)) { + LOG.info(dummyHostName + " is executing the injected stall"); + stallingHost.set(dummyHostName); + while (doStall) { final StreamThread thread = (StreamThread) Thread.currentThread(); if (thread.isInterrupted() || !thread.isRunning()) { throw new RuntimeException("Detected we've been interrupted."); @@ -714,7 +753,7 @@ public void close() { } properties.put(StreamsConfig.consumerPrefix(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG), MAX_POLL_INTERVAL_MS); properties.put(StreamsConfig.CACHE_MAX_BYTES_BUFFERING_CONFIG, 0); properties.put(StreamsConfig.STATE_DIR_CONFIG, TestUtils.tempDirectory().getPath() + File.separator + appDir); - properties.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "dummy:2142"); + properties.put(StreamsConfig.APPLICATION_SERVER_CONFIG, dummyHostName + ":2142"); final Properties config = StreamsTestUtils.getStreamsConfig( applicationId, diff --git a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java deleted file mode 100644 index 14c8e4a360974..0000000000000 --- a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java +++ /dev/null @@ -1,349 +0,0 @@ -/* - * 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.integration; - -import static org.apache.kafka.common.utils.Utils.mkSet; -import static org.apache.kafka.streams.integration.utils.IntegrationTestUtils.startApplicationAndWaitUntilRunning; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.IsEqual.equalTo; -import static org.junit.Assert.assertTrue; - -import java.io.File; -import java.nio.file.Files; -import java.nio.file.Path; -import java.time.Duration; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.CyclicBarrier; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; -import kafka.utils.MockTime; -import org.apache.kafka.clients.consumer.ConsumerConfig; -import org.apache.kafka.common.TopicPartition; -import org.apache.kafka.common.serialization.LongDeserializer; -import org.apache.kafka.common.serialization.LongSerializer; -import org.apache.kafka.common.serialization.Serdes; -import org.apache.kafka.common.serialization.StringDeserializer; -import org.apache.kafka.common.serialization.StringSerializer; -import org.apache.kafka.streams.KafkaStreams; -import org.apache.kafka.streams.KafkaStreamsWrapper; -import org.apache.kafka.streams.KeyValue; -import org.apache.kafka.streams.LagInfo; -import org.apache.kafka.streams.StreamsBuilder; -import org.apache.kafka.streams.StreamsConfig; -import org.apache.kafka.streams.integration.utils.EmbeddedKafkaCluster; -import org.apache.kafka.streams.integration.utils.IntegrationTestUtils; -import org.apache.kafka.streams.kstream.KTable; -import org.apache.kafka.streams.kstream.Materialized; -import org.apache.kafka.streams.processor.StateRestoreListener; -import org.apache.kafka.streams.processor.internals.StreamThread; -import org.apache.kafka.test.IntegrationTest; -import org.apache.kafka.test.TestUtils; -import org.junit.After; -import org.junit.Before; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.junit.rules.TestName; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -@Category({IntegrationTest.class}) -public class LagFetchIntegrationTest { - - @ClassRule - public static final EmbeddedKafkaCluster CLUSTER = new EmbeddedKafkaCluster(1); - - private static final long WAIT_TIMEOUT_MS = 120000; - private static final Logger LOG = LoggerFactory.getLogger(LagFetchIntegrationTest.class); - - private final MockTime mockTime = CLUSTER.time; - private Properties streamsConfiguration; - private Properties consumerConfiguration; - private String inputTopicName; - private String outputTopicName; - private String stateStoreName; - - @Rule - public TestName name = new TestName(); - - @Before - public void before() { - inputTopicName = "input-topic-" + name.getMethodName(); - outputTopicName = "output-topic-" + name.getMethodName(); - stateStoreName = "lagfetch-test-store" + name.getMethodName(); - - streamsConfiguration = new Properties(); - streamsConfiguration.put(StreamsConfig.APPLICATION_ID_CONFIG, "lag-fetch-" + name.getMethodName()); - streamsConfiguration.put(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG, CLUSTER.bootstrapServers()); - streamsConfiguration.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); - streamsConfiguration.put(StreamsConfig.DEFAULT_KEY_SERDE_CLASS_CONFIG, Serdes.String().getClass()); - streamsConfiguration.put(StreamsConfig.DEFAULT_VALUE_SERDE_CLASS_CONFIG, Serdes.String().getClass()); - streamsConfiguration.put(StreamsConfig.COMMIT_INTERVAL_MS_CONFIG, 100); - - consumerConfiguration = new Properties(); - consumerConfiguration.setProperty(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, CLUSTER.bootstrapServers()); - consumerConfiguration.setProperty(ConsumerConfig.GROUP_ID_CONFIG, name.getMethodName() + "-consumer"); - consumerConfiguration.setProperty(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); - consumerConfiguration.setProperty(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class.getName()); - consumerConfiguration.setProperty(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, LongDeserializer.class.getName()); - } - - @After - public void shutdown() throws Exception { - IntegrationTestUtils.purgeLocalStreamsState(streamsConfiguration); - } - - private Map> getFirstNonEmptyLagMap(final KafkaStreams streams) throws InterruptedException { - final Map> offsetLagInfoMap = new HashMap<>(); - TestUtils.waitForCondition(() -> { - final Map> lagMap = streams.allLocalStorePartitionLags(); - if (lagMap.size() > 0) { - offsetLagInfoMap.putAll(lagMap); - } - return lagMap.size() > 0; - }, WAIT_TIMEOUT_MS, "Should obtain non-empty lag information eventually"); - return offsetLagInfoMap; - } - - private void shouldFetchLagsDuringRebalancing(final String optimization) throws Exception { - final CountDownLatch latchTillActiveIsRunning = new CountDownLatch(1); - final CountDownLatch latchTillStandbyIsRunning = new CountDownLatch(1); - final CountDownLatch latchTillStandbyHasPartitionsAssigned = new CountDownLatch(1); - final CyclicBarrier lagCheckBarrier = new CyclicBarrier(2); - final List streamsList = new ArrayList<>(); - - IntegrationTestUtils.produceKeyValuesSynchronously( - inputTopicName, - mkSet(new KeyValue<>("k1", 1L), new KeyValue<>("k2", 2L), new KeyValue<>("k3", 3L), new KeyValue<>("k4", 4L), new KeyValue<>("k5", 5L)), - TestUtils.producerConfig( - CLUSTER.bootstrapServers(), - StringSerializer.class, - LongSerializer.class, - new Properties()), - mockTime); - - // create stream threads - for (int i = 0; i < 2; i++) { - final Properties props = (Properties) streamsConfiguration.clone(); - props.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "localhost:" + i); - props.put(StreamsConfig.CLIENT_ID_CONFIG, "instance-" + i); - props.put(StreamsConfig.TOPOLOGY_OPTIMIZATION, optimization); - props.put(StreamsConfig.NUM_STANDBY_REPLICAS_CONFIG, 1); - props.put(StreamsConfig.STATE_DIR_CONFIG, TestUtils.tempDirectory(stateStoreName + i).getAbsolutePath()); - - final StreamsBuilder builder = new StreamsBuilder(); - final KTable t1 = builder.table(inputTopicName, Materialized.as(stateStoreName)); - t1.toStream().to(outputTopicName); - final KafkaStreamsWrapper streams = new KafkaStreamsWrapper(builder.build(props), props); - streamsList.add(streams); - } - - final KafkaStreamsWrapper activeStreams = streamsList.get(0); - final KafkaStreamsWrapper standbyStreams = streamsList.get(1); - activeStreams.setStreamThreadStateListener((thread, newState, oldState) -> { - if (newState == StreamThread.State.RUNNING) { - latchTillActiveIsRunning.countDown(); - } - }); - standbyStreams.setStreamThreadStateListener((thread, newState, oldState) -> { - if (oldState == StreamThread.State.PARTITIONS_ASSIGNED && newState == StreamThread.State.RUNNING) { - latchTillStandbyHasPartitionsAssigned.countDown(); - try { - lagCheckBarrier.await(60, TimeUnit.SECONDS); - } catch (final Exception e) { - throw new RuntimeException(e); - } - } else if (newState == StreamThread.State.RUNNING) { - latchTillStandbyIsRunning.countDown(); - } - }); - - try { - // First start up the active. - TestUtils.waitForCondition(() -> activeStreams.allLocalStorePartitionLags().size() == 0, - WAIT_TIMEOUT_MS, - "Should see empty lag map before streams is started."); - activeStreams.start(); - latchTillActiveIsRunning.await(60, TimeUnit.SECONDS); - - IntegrationTestUtils.waitUntilMinValuesRecordsReceived( - consumerConfiguration, - outputTopicName, - 5, - WAIT_TIMEOUT_MS); - // Check the active reports proper lag values. - Map> offsetLagInfoMap = getFirstNonEmptyLagMap(activeStreams); - assertThat(offsetLagInfoMap.size(), equalTo(1)); - assertThat(offsetLagInfoMap.keySet(), equalTo(mkSet(stateStoreName))); - assertThat(offsetLagInfoMap.get(stateStoreName).size(), equalTo(1)); - LagInfo lagInfo = offsetLagInfoMap.get(stateStoreName).get(0); - assertThat(lagInfo.currentOffsetPosition(), equalTo(5L)); - assertThat(lagInfo.endOffsetPosition(), equalTo(5L)); - assertThat(lagInfo.offsetLag(), equalTo(0L)); - - // start up the standby & make it pause right after it has partition assigned - standbyStreams.start(); - latchTillStandbyHasPartitionsAssigned.await(60, TimeUnit.SECONDS); - offsetLagInfoMap = getFirstNonEmptyLagMap(standbyStreams); - assertThat(offsetLagInfoMap.size(), equalTo(1)); - assertThat(offsetLagInfoMap.keySet(), equalTo(mkSet(stateStoreName))); - assertThat(offsetLagInfoMap.get(stateStoreName).size(), equalTo(1)); - lagInfo = offsetLagInfoMap.get(stateStoreName).get(0); - assertThat(lagInfo.currentOffsetPosition(), equalTo(0L)); - assertThat(lagInfo.endOffsetPosition(), equalTo(5L)); - assertThat(lagInfo.offsetLag(), equalTo(5L)); - // standby thread wont proceed to RUNNING before this barrier is crossed - lagCheckBarrier.await(60, TimeUnit.SECONDS); - - // wait till the lag goes down to 0, on the standby - TestUtils.waitForCondition(() -> standbyStreams.allLocalStorePartitionLags().get(stateStoreName).get(0).offsetLag() == 0, - WAIT_TIMEOUT_MS, - "Standby should eventually catchup and have zero lag."); - } finally { - for (final KafkaStreams streams : streamsList) { - streams.close(); - } - } - } - - @Test - public void shouldFetchLagsDuringRebalancingWithOptimization() throws Exception { - shouldFetchLagsDuringRebalancing(StreamsConfig.OPTIMIZE); - } - - @Test - public void shouldFetchLagsDuringRebalancingWithNoOptimization() throws Exception { - shouldFetchLagsDuringRebalancing(StreamsConfig.NO_OPTIMIZATION); - } - - @Test - public void shouldFetchLagsDuringRestoration() throws Exception { - IntegrationTestUtils.produceKeyValuesSynchronously( - inputTopicName, - mkSet(new KeyValue<>("k1", 1L), new KeyValue<>("k2", 2L), new KeyValue<>("k3", 3L), new KeyValue<>("k4", 4L), new KeyValue<>("k5", 5L)), - TestUtils.producerConfig( - CLUSTER.bootstrapServers(), - StringSerializer.class, - LongSerializer.class, - new Properties()), - mockTime); - - // create stream threads - final Properties props = (Properties) streamsConfiguration.clone(); - final File stateDir = TestUtils.tempDirectory(stateStoreName + "0"); - props.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "localhost:0"); - props.put(StreamsConfig.CLIENT_ID_CONFIG, "instance-0"); - props.put(StreamsConfig.STATE_DIR_CONFIG, stateDir.getAbsolutePath()); - - final StreamsBuilder builder = new StreamsBuilder(); - final KTable t1 = builder.table(inputTopicName, Materialized.as(stateStoreName)); - t1.toStream().to(outputTopicName); - final KafkaStreams streams = new KafkaStreams(builder.build(), props); - - try { - // First start up the active. - TestUtils.waitForCondition(() -> streams.allLocalStorePartitionLags().size() == 0, - WAIT_TIMEOUT_MS, - "Should see empty lag map before streams is started."); - - // Get the instance to fully catch up and reach RUNNING state - startApplicationAndWaitUntilRunning(Collections.singletonList(streams), Duration.ofSeconds(60)); - IntegrationTestUtils.waitUntilMinValuesRecordsReceived( - consumerConfiguration, - outputTopicName, - 5, - WAIT_TIMEOUT_MS); - - // check for proper lag values. - final AtomicReference zeroLagRef = new AtomicReference<>(); - TestUtils.waitForCondition(() -> { - final Map> offsetLagInfoMap = streams.allLocalStorePartitionLags(); - assertThat(offsetLagInfoMap.size(), equalTo(1)); - assertThat(offsetLagInfoMap.keySet(), equalTo(mkSet(stateStoreName))); - assertThat(offsetLagInfoMap.get(stateStoreName).size(), equalTo(1)); - - final LagInfo zeroLagInfo = offsetLagInfoMap.get(stateStoreName).get(0); - assertThat(zeroLagInfo.currentOffsetPosition(), equalTo(5L)); - assertThat(zeroLagInfo.endOffsetPosition(), equalTo(5L)); - assertThat(zeroLagInfo.offsetLag(), equalTo(0L)); - zeroLagRef.set(zeroLagInfo); - return true; - }, WAIT_TIMEOUT_MS, "Eventually should reach zero lag."); - - // Kill instance, delete state to force restoration. - assertThat("Streams instance did not close within timeout", streams.close(Duration.ofSeconds(60))); - IntegrationTestUtils.purgeLocalStreamsState(streamsConfiguration); - Files.walk(stateDir.toPath()).sorted(Comparator.reverseOrder()) - .map(Path::toFile) - .forEach(f -> assertTrue("Some state " + f + " could not be deleted", f.delete())); - - // wait till the lag goes down to 0 - final KafkaStreams restartedStreams = new KafkaStreams(builder.build(), props); - // set a state restoration listener to track progress of restoration - final CountDownLatch restorationEndLatch = new CountDownLatch(1); - final Map> restoreStartLagInfo = new HashMap<>(); - final Map> restoreEndLagInfo = new HashMap<>(); - restartedStreams.setGlobalStateRestoreListener(new StateRestoreListener() { - @Override - public void onRestoreStart(final TopicPartition topicPartition, final String storeName, final long startingOffset, final long endingOffset) { - try { - restoreStartLagInfo.putAll(getFirstNonEmptyLagMap(restartedStreams)); - } catch (final Exception e) { - LOG.error("Exception while trying to obtain lag map", e); - } - } - - @Override - public void onBatchRestored(final TopicPartition topicPartition, final String storeName, final long batchEndOffset, final long numRestored) { - } - - @Override - public void onRestoreEnd(final TopicPartition topicPartition, final String storeName, final long totalRestored) { - try { - restoreEndLagInfo.putAll(getFirstNonEmptyLagMap(restartedStreams)); - } catch (final Exception e) { - LOG.error("Exception while trying to obtain lag map", e); - } - restorationEndLatch.countDown(); - } - }); - - restartedStreams.start(); - restorationEndLatch.await(WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS); - TestUtils.waitForCondition(() -> restartedStreams.allLocalStorePartitionLags().get(stateStoreName).get(0).offsetLag() == 0, - WAIT_TIMEOUT_MS, - "Standby should eventually catchup and have zero lag."); - final LagInfo fullLagInfo = restoreStartLagInfo.get(stateStoreName).get(0); - assertThat(fullLagInfo.currentOffsetPosition(), equalTo(0L)); - assertThat(fullLagInfo.endOffsetPosition(), equalTo(5L)); - assertThat(fullLagInfo.offsetLag(), equalTo(5L)); - - assertThat(restoreEndLagInfo.get(stateStoreName).get(0), equalTo(zeroLagRef.get())); - } finally { - streams.close(); - streams.cleanUp(); - } - } -} diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java new file mode 100644 index 0000000000000..3f47b4868251e --- /dev/null +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java @@ -0,0 +1,326 @@ +/* + * 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.processor.internals; + +import org.apache.kafka.clients.admin.Admin; +import org.apache.kafka.clients.admin.AdminClient; +import org.apache.kafka.clients.admin.ListOffsetsResult; +import org.apache.kafka.clients.admin.ListOffsetsResult.ListOffsetsResultInfo; +import org.apache.kafka.clients.consumer.ConsumerPartitionAssignor.Assignment; +import org.apache.kafka.clients.consumer.ConsumerPartitionAssignor.GroupSubscription; +import org.apache.kafka.clients.consumer.ConsumerPartitionAssignor.Subscription; +import org.apache.kafka.common.Cluster; +import org.apache.kafka.common.Node; +import org.apache.kafka.common.PartitionInfo; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.internals.KafkaFutureImpl; +import org.apache.kafka.common.utils.MockTime; +import org.apache.kafka.streams.StreamsConfig; +import org.apache.kafka.streams.StreamsConfig.InternalConfig; +import org.apache.kafka.streams.errors.StreamsException; +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.assignment.AssignmentInfo; +import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration; +import org.apache.kafka.streams.processor.internals.assignment.AssignorError; +import org.apache.kafka.streams.processor.internals.assignment.HighAvailabilityTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo; +import org.apache.kafka.test.MockClientSupplier; +import org.apache.kafka.test.MockInternalTopicManager; +import org.apache.kafka.test.MockKeyValueStoreBuilder; +import org.apache.kafka.test.MockProcessorSupplier; +import org.easymock.EasyMock; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Collectors; + +import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static org.apache.kafka.common.utils.Utils.mkSet; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.EMPTY_CHANGELOG_END_OFFSETS; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.EMPTY_TASKS; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_0; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_1; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_2; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_1; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_2; +import static org.apache.kafka.streams.processor.internals.assignment.StreamsAssignmentProtocolVersions.LATEST_SUPPORTED_VERSION; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.expect; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; + +public class HighAvailabilityStreamsPartitionAssignorTest { + + private final List infos = asList( + new PartitionInfo("topic1", 0, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic1", 1, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic1", 2, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic2", 0, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic2", 1, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic2", 2, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic3", 0, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic3", 1, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic3", 2, Node.noNode(), new Node[0], new Node[0]), + new PartitionInfo("topic3", 3, Node.noNode(), new Node[0], new Node[0]) + ); + + private final Cluster metadata = new Cluster( + "cluster", + singletonList(Node.noNode()), + infos, + emptySet(), + emptySet()); + + private final StreamsPartitionAssignor partitionAssignor = new StreamsPartitionAssignor(); + private final MockClientSupplier mockClientSupplier = new MockClientSupplier(); + private static final String USER_END_POINT = "localhost:8080"; + private static final String APPLICATION_ID = "stream-partition-assignor-test"; + + private TaskManager taskManager; + private Admin adminClient; + private StreamsConfig streamsConfig = new StreamsConfig(configProps()); + private final InternalTopologyBuilder builder = new InternalTopologyBuilder(); + private final StreamsMetadataState streamsMetadataState = EasyMock.createNiceMock(StreamsMetadataState.class); + private final Map subscriptions = new HashMap<>(); + + private final AtomicInteger assignmentError = new AtomicInteger(); + private final AtomicLong nextProbingRebalanceMs = new AtomicLong(Long.MAX_VALUE); + private final MockTime time = new MockTime(); + + private Map configProps() { + final Map configurationMap = new HashMap<>(); + configurationMap.put(StreamsConfig.APPLICATION_ID_CONFIG, APPLICATION_ID); + configurationMap.put(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG, USER_END_POINT); + configurationMap.put(InternalConfig.TASK_MANAGER_FOR_PARTITION_ASSIGNOR, taskManager); + configurationMap.put(InternalConfig.STREAMS_METADATA_STATE_FOR_PARTITION_ASSIGNOR, streamsMetadataState); + configurationMap.put(InternalConfig.STREAMS_ADMIN_CLIENT, adminClient); + configurationMap.put(InternalConfig.ASSIGNMENT_ERROR_CODE, assignmentError); + configurationMap.put(InternalConfig.NEXT_PROBING_REBALANCE_MS, nextProbingRebalanceMs); + configurationMap.put(InternalConfig.TIME, time); + configurationMap.put(AssignorConfiguration.INTERNAL_TASK_ASSIGNOR_CLASS, HighAvailabilityTaskAssignor.class.getName()); + return configurationMap; + } + + // Make sure to complete setting up any mocks (such as TaskManager or AdminClient) before configuring the assignor + private void configureDefaultPartitionAssignor() { + configurePartitionAssignorWith(emptyMap()); + } + + // Make sure to complete setting up any mocks (such as TaskManager or AdminClient) before configuring the assignor + private void configurePartitionAssignorWith(final Map props) { + final Map configMap = configProps(); + configMap.putAll(props); + + streamsConfig = new StreamsConfig(configMap); + partitionAssignor.configure(configMap); + EasyMock.replay(taskManager, adminClient); + + overwriteInternalTopicManagerWithMock(); + } + + // Useful for tests that don't care about the task offset sums + private void createMockTaskManager(final Set activeTasks) { + createMockTaskManager(getTaskOffsetSums(activeTasks)); + } + + private void createMockTaskManager(final Map taskOffsetSums) { + taskManager = EasyMock.createNiceMock(TaskManager.class); + expect(taskManager.builder()).andReturn(builder).anyTimes(); + expect(taskManager.getTaskOffsetSums()).andReturn(taskOffsetSums).anyTimes(); + expect(taskManager.processId()).andReturn(UUID_1).anyTimes(); + builder.setApplicationId(APPLICATION_ID); + builder.buildTopology(); + } + + // If you don't care about setting the end offsets for each specific topic partition, the helper method + // getTopicPartitionOffsetMap is useful for building this input map for all partitions + private void createMockAdminClient(final Map changelogEndOffsets) { + adminClient = EasyMock.createMock(AdminClient.class); + + final ListOffsetsResult result = EasyMock.createNiceMock(ListOffsetsResult.class); + final KafkaFutureImpl> allFuture = new KafkaFutureImpl<>(); + allFuture.complete(changelogEndOffsets.entrySet().stream().collect(Collectors.toMap( + Entry::getKey, + t -> { + final ListOffsetsResultInfo info = EasyMock.createNiceMock(ListOffsetsResultInfo.class); + expect(info.offset()).andStubReturn(t.getValue()); + EasyMock.replay(info); + return info; + })) + ); + + expect(adminClient.listOffsets(anyObject())).andStubReturn(result); + expect(result.all()).andReturn(allFuture); + + EasyMock.replay(result); + } + + private void overwriteInternalTopicManagerWithMock() { + final MockInternalTopicManager mockInternalTopicManager = new MockInternalTopicManager(streamsConfig, mockClientSupplier.restoreConsumer); + partitionAssignor.setInternalTopicManager(mockInternalTopicManager); + } + + @Before + public void setUp() { + createMockAdminClient(EMPTY_CHANGELOG_END_OFFSETS); + } + + + @Test + public void shouldReturnAllActiveTasksToPreviousOwnerRegardlessOfBalanceAndTriggerRebalanceIfEndOffsetFetchFailsAndHighAvailabilityEnabled() { + builder.addSource(null, "source1", null, null, null, "topic1"); + builder.addProcessor("processor1", new MockProcessorSupplier<>(), "source1"); + builder.addStateStore(new MockKeyValueStoreBuilder("store1", false), "processor1"); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2); + + createMockTaskManager(allTasks); + adminClient = EasyMock.createMock(AdminClient.class); + expect(adminClient.listOffsets(anyObject())).andThrow(new StreamsException("Should be handled")); + configureDefaultPartitionAssignor(); + + final String firstConsumer = "consumer1"; + final String newConsumer = "consumer2"; + + subscriptions.put(firstConsumer, + new Subscription( + singletonList("source1"), + getInfo(UUID_1, allTasks).encode() + )); + subscriptions.put(newConsumer, + new Subscription( + singletonList("source1"), + getInfo(UUID_2, EMPTY_TASKS).encode() + )); + + final Map assignments = partitionAssignor + .assign(metadata, new GroupSubscription(subscriptions)) + .groupAssignment(); + + final List firstConsumerActiveTasks = + AssignmentInfo.decode(assignments.get(firstConsumer).userData()).activeTasks(); + final List newConsumerActiveTasks = + AssignmentInfo.decode(assignments.get(newConsumer).userData()).activeTasks(); + + assertThat(firstConsumerActiveTasks, equalTo(new ArrayList<>(allTasks))); + assertTrue(newConsumerActiveTasks.isEmpty()); + assertThat(assignmentError.get(), equalTo(AssignorError.REBALANCE_NEEDED.code())); + } + + @Test + public void shouldScheduleProbingRebalanceOnThisClientIfWarmupTasksRequired() { + final long rebalanceInterval = 5 * 60 * 1000L; + + builder.addSource(null, "source1", null, null, null, "topic1"); + builder.addProcessor("processor1", new MockProcessorSupplier<>(), "source1"); + builder.addStateStore(new MockKeyValueStoreBuilder("store1", false), "processor1"); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2); + + createMockTaskManager(allTasks); + createMockAdminClient(getTopicPartitionOffsetsMap( + singletonList(APPLICATION_ID + "-store1-changelog"), + singletonList(3))); + configurePartitionAssignorWith(singletonMap(StreamsConfig.PROBING_REBALANCE_INTERVAL_MS_CONFIG, rebalanceInterval)); + + final String firstConsumer = "consumer1"; + final String newConsumer = "consumer2"; + + subscriptions.put(firstConsumer, + new Subscription( + singletonList("source1"), + getInfo(UUID_1, allTasks).encode() + )); + subscriptions.put(newConsumer, + new Subscription( + singletonList("source1"), + getInfo(UUID_2, EMPTY_TASKS).encode() + )); + + final Map assignments = partitionAssignor + .assign(metadata, new GroupSubscription(subscriptions)) + .groupAssignment(); + + final List firstConsumerActiveTasks = + AssignmentInfo.decode(assignments.get(firstConsumer).userData()).activeTasks(); + final List newConsumerActiveTasks = + AssignmentInfo.decode(assignments.get(newConsumer).userData()).activeTasks(); + + assertThat(firstConsumerActiveTasks, equalTo(new ArrayList<>(allTasks))); + assertTrue(newConsumerActiveTasks.isEmpty()); + + assertThat(assignmentError.get(), equalTo(AssignorError.NONE.code())); + + final long nextScheduledRebalanceOnThisClient = + AssignmentInfo.decode(assignments.get(firstConsumer).userData()).nextRebalanceMs(); + final long nextScheduledRebalanceOnOtherClient = + AssignmentInfo.decode(assignments.get(newConsumer).userData()).nextRebalanceMs(); + + assertThat(nextScheduledRebalanceOnThisClient, equalTo(time.milliseconds() + rebalanceInterval)); + assertThat(nextScheduledRebalanceOnOtherClient, equalTo(Long.MAX_VALUE)); + } + + + /** + * Helper for building the input to createMockAdminClient in cases where we don't care about the actual offsets + * @param changelogTopics The names of all changelog topics in the topology + * @param topicsNumPartitions The number of partitions for the corresponding changelog topic, such that the number + * of partitions of the ith topic in changelogTopics is given by the ith element of topicsNumPartitions + */ + private static Map getTopicPartitionOffsetsMap(final List changelogTopics, + final List topicsNumPartitions) { + if (changelogTopics.size() != topicsNumPartitions.size()) { + throw new IllegalStateException("Passed in " + changelogTopics.size() + " changelog topic names, but " + + topicsNumPartitions.size() + " different numPartitions for the topics"); + } + final Map changelogEndOffsets = new HashMap<>(); + for (int i = 0; i < changelogTopics.size(); ++i) { + final String topic = changelogTopics.get(i); + final int numPartitions = topicsNumPartitions.get(i); + for (int partition = 0; partition < numPartitions; ++partition) { + changelogEndOffsets.put(new TopicPartition(topic, partition), Long.MAX_VALUE); + } + } + return changelogEndOffsets; + } + + private static SubscriptionInfo getInfo(final UUID processId, + final Set prevTasks) { + return new SubscriptionInfo( + LATEST_SUPPORTED_VERSION, LATEST_SUPPORTED_VERSION, processId, null, getTaskOffsetSums(prevTasks)); + } + + // Stub offset sums for when we only care about the prev/standby task sets, not the actual offsets + private static Map getTaskOffsetSums(final Set activeTasks) { + final Map taskOffsetSums = activeTasks.stream().collect(Collectors.toMap(t -> t, t -> Task.LATEST_OFFSET)); + taskOffsetSums.putAll(EMPTY_TASKS.stream().collect(Collectors.toMap(t -> t, t -> 0L))); + return taskOffsetSums; + } + +} diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java index d576b69d1c6f8..e2adf14a616db 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java @@ -40,7 +40,6 @@ import org.apache.kafka.streams.StreamsConfig; import org.apache.kafka.streams.StreamsConfig.InternalConfig; import org.apache.kafka.streams.TopologyWrapper; -import org.apache.kafka.streams.errors.StreamsException; import org.apache.kafka.streams.kstream.JoinWindows; import org.apache.kafka.streams.kstream.KStream; import org.apache.kafka.streams.kstream.KTable; @@ -52,7 +51,11 @@ import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration; import org.apache.kafka.streams.processor.internals.assignment.AssignorError; import org.apache.kafka.streams.processor.internals.assignment.ClientState; +import org.apache.kafka.streams.processor.internals.assignment.HighAvailabilityTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.PriorTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor; import org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo; +import org.apache.kafka.streams.processor.internals.assignment.TaskAssignor; import org.apache.kafka.streams.state.HostInfo; import org.apache.kafka.test.MockClientSupplier; import org.apache.kafka.test.MockInternalTopicManager; @@ -83,7 +86,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; -import static java.util.Collections.singletonMap; import static org.apache.kafka.common.utils.Utils.mkEntry; import static org.apache.kafka.common.utils.Utils.mkMap; import static org.apache.kafka.common.utils.Utils.mkSet; @@ -192,11 +194,10 @@ public class StreamsPartitionAssignorTest { private TaskManager taskManager; private Admin adminClient; - private StreamsConfig streamsConfig = new StreamsConfig(configProps()); private InternalTopologyBuilder builder = new InternalTopologyBuilder(); private StreamsMetadataState streamsMetadataState = EasyMock.createNiceMock(StreamsMetadataState.class); private final Map subscriptions = new HashMap<>(); - private final boolean highAvailabilityEnabled; + private final Class taskAssignor; private final AtomicInteger assignmentError = new AtomicInteger(); private final AtomicLong nextProbingRebalanceMs = new AtomicLong(Long.MAX_VALUE); @@ -212,7 +213,7 @@ private Map configProps() { configurationMap.put(StreamsConfig.InternalConfig.ASSIGNMENT_ERROR_CODE, assignmentError); configurationMap.put(StreamsConfig.InternalConfig.NEXT_PROBING_REBALANCE_MS, nextProbingRebalanceMs); configurationMap.put(InternalConfig.TIME, time); - configurationMap.put(AssignorConfiguration.HIGH_AVAILABILITY_ENABLED_CONFIG, highAvailabilityEnabled); + configurationMap.put(AssignorConfiguration.INTERNAL_TASK_ASSIGNOR_CLASS, taskAssignor.getName()); return configurationMap; } @@ -231,7 +232,6 @@ private MockInternalTopicManager configurePartitionAssignorWith(final Map configMap = configProps(); configMap.putAll(props); - streamsConfig = new StreamsConfig(configMap); partitionAssignor.configure(configMap); EasyMock.replay(taskManager, adminClient); @@ -282,21 +282,23 @@ private void createMockAdminClient(final Map changelogEndO } private MockInternalTopicManager overwriteInternalTopicManagerWithMock() { - final MockInternalTopicManager mockInternalTopicManager = new MockInternalTopicManager(streamsConfig, mockClientSupplier.restoreConsumer); + final MockInternalTopicManager mockInternalTopicManager = + new MockInternalTopicManager(new StreamsConfig(configProps()), mockClientSupplier.restoreConsumer); partitionAssignor.setInternalTopicManager(mockInternalTopicManager); return mockInternalTopicManager; } - @Parameterized.Parameters(name = "high availability enabled = {0}") + @Parameterized.Parameters(name = "task assignor = {0}") public static Collection parameters() { return asList( - new Object[]{true}, - new Object[]{false} + new Object[]{HighAvailabilityTaskAssignor.class}, + new Object[]{StickyTaskAssignor.class}, + new Object[]{PriorTaskAssignor.class} ); } - public StreamsPartitionAssignorTest(final boolean highAvailabilityEnabled) { - this.highAvailabilityEnabled = highAvailabilityEnabled; + public StreamsPartitionAssignorTest(final Class taskAssignor) { + this.taskAssignor = taskAssignor; createMockAdminClient(EMPTY_CHANGELOG_END_OFFSETS); } @@ -1832,102 +1834,6 @@ public void shouldThrowIllegalStateExceptionIfAnyTopicsMissingFromChangelogEndOf assertThrows(IllegalStateException.class, () -> partitionAssignor.assign(metadata, new GroupSubscription(subscriptions))); } - @Test - public void shouldReturnAllActiveTasksToPreviousOwnerRegardlessOfBalanceAndTriggerRebalanceIfEndOffsetFetchFailsAndHighAvailabilityEnabled() { - if (highAvailabilityEnabled) { - builder.addSource(null, "source1", null, null, null, "topic1"); - builder.addProcessor("processor1", new MockProcessorSupplier<>(), "source1"); - builder.addStateStore(new MockKeyValueStoreBuilder("store1", false), "processor1"); - final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2); - - createMockTaskManager(allTasks, EMPTY_TASKS); - adminClient = EasyMock.createMock(AdminClient.class); - expect(adminClient.listOffsets(anyObject())).andThrow(new StreamsException("Should be handled")); - configureDefaultPartitionAssignor(); - - final String firstConsumer = "consumer1"; - final String newConsumer = "consumer2"; - - subscriptions.put(firstConsumer, - new Subscription( - singletonList("source1"), - getInfo(UUID_1, allTasks, EMPTY_TASKS).encode() - )); - subscriptions.put(newConsumer, - new Subscription( - singletonList("source1"), - getInfo(UUID_2, EMPTY_TASKS, EMPTY_TASKS).encode() - )); - - final Map assignments = partitionAssignor - .assign(metadata, new GroupSubscription(subscriptions)) - .groupAssignment(); - - final List firstConsumerActiveTasks = - AssignmentInfo.decode(assignments.get(firstConsumer).userData()).activeTasks(); - final List newConsumerActiveTasks = - AssignmentInfo.decode(assignments.get(newConsumer).userData()).activeTasks(); - - assertThat(firstConsumerActiveTasks, equalTo(new ArrayList<>(allTasks))); - assertTrue(newConsumerActiveTasks.isEmpty()); - assertThat(assignmentError.get(), equalTo(AssignorError.REBALANCE_NEEDED.code())); - } - } - - @Test - public void shouldScheduleProbingRebalanceOnThisClientIfWarmupTasksRequired() { - if (highAvailabilityEnabled) { - final long rebalanceInterval = 5 * 60 * 1000L; - - builder.addSource(null, "source1", null, null, null, "topic1"); - builder.addProcessor("processor1", new MockProcessorSupplier<>(), "source1"); - builder.addStateStore(new MockKeyValueStoreBuilder("store1", false), "processor1"); - final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2); - - createMockTaskManager(allTasks, EMPTY_TASKS); - createMockAdminClient(getTopicPartitionOffsetsMap( - singletonList(APPLICATION_ID + "-store1-changelog"), - singletonList(3))); - configurePartitionAssignorWith(singletonMap(StreamsConfig.PROBING_REBALANCE_INTERVAL_MS_CONFIG, rebalanceInterval)); - - final String firstConsumer = "consumer1"; - final String newConsumer = "consumer2"; - - subscriptions.put(firstConsumer, - new Subscription( - singletonList("source1"), - getInfo(UUID_1, allTasks, EMPTY_TASKS).encode() - )); - subscriptions.put(newConsumer, - new Subscription( - singletonList("source1"), - getInfo(UUID_2, EMPTY_TASKS, EMPTY_TASKS).encode() - )); - - final Map assignments = partitionAssignor - .assign(metadata, new GroupSubscription(subscriptions)) - .groupAssignment(); - - final List firstConsumerActiveTasks = - AssignmentInfo.decode(assignments.get(firstConsumer).userData()).activeTasks(); - final List newConsumerActiveTasks = - AssignmentInfo.decode(assignments.get(newConsumer).userData()).activeTasks(); - - assertThat(firstConsumerActiveTasks, equalTo(new ArrayList<>(allTasks))); - assertTrue(newConsumerActiveTasks.isEmpty()); - - assertThat(assignmentError.get(), equalTo(AssignorError.NONE.code())); - - final long nextScheduledRebalanceOnThisClient = - AssignmentInfo.decode(assignments.get(firstConsumer).userData()).nextRebalanceMs(); - final long nextScheduledRebalanceOnOtherClient = - AssignmentInfo.decode(assignments.get(newConsumer).userData()).nextRebalanceMs(); - - assertThat(nextScheduledRebalanceOnThisClient, equalTo(time.milliseconds() + rebalanceInterval)); - assertThat(nextScheduledRebalanceOnOtherClient, equalTo(Long.MAX_VALUE)); - } - } - private static ByteBuffer encodeFutureSubscription() { final ByteBuffer buf = ByteBuffer.allocate(4 /* used version */ + 4 /* supported version */); buf.putInt(LATEST_SUPPORTED_VERSION + 1); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java index 098c6508f53bc..17d7c17f82ccc 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java @@ -16,6 +16,19 @@ */ package org.apache.kafka.streams.processor.internals.assignment; +import org.apache.kafka.streams.processor.TaskId; +import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs; +import org.easymock.EasyMock; +import org.junit.Test; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.stream.Collectors; + +import static java.util.Collections.emptySet; import static java.util.Collections.singleton; import static java.util.Collections.singletonMap; import static org.apache.kafka.common.utils.Utils.mkEntry; @@ -41,132 +54,107 @@ import static org.easymock.EasyMock.replay; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; -import java.util.UUID; -import org.apache.kafka.streams.processor.TaskId; -import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs; -import org.easymock.EasyMock; -import org.junit.Test; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; public class HighAvailabilityTaskAssignorTest { - private long acceptableRecoveryLag = 100L; - private int balanceFactor = 1; - private int maxWarmupReplicas = 2; - private int numStandbyReplicas = 0; - private long probingRebalanceInterval = 60 * 1000L; - - private Map clientStates = new HashMap<>(); - private Set allTasks = new HashSet<>(); - private Set statefulTasks = new HashSet<>(); - - private ClientState client1; - private ClientState client2; - private ClientState client3; - - private HighAvailabilityTaskAssignor taskAssignor; - - private void createTaskAssignor() { - final AssignmentConfigs configs = new AssignmentConfigs( - acceptableRecoveryLag, - balanceFactor, - maxWarmupReplicas, - numStandbyReplicas, - probingRebalanceInterval - ); - taskAssignor = new HighAvailabilityTaskAssignor( - clientStates, - allTasks, - statefulTasks, - configs); - } + private final AssignmentConfigs configWithoutStandbys = new AssignmentConfigs( + /*acceptableRecoveryLag*/ 100L, + /*balanceFactor*/ 1, + /*maxWarmupReplicas*/ 2, + /*numStandbyReplicas*/ 0, + /*probingRebalanceIntervalMs*/ 60 * 1000L + ); + + private final AssignmentConfigs configWithStandbys = new AssignmentConfigs( + /*acceptableRecoveryLag*/ 100L, + /*balanceFactor*/ 1, + /*maxWarmupReplicas*/ 2, + /*numStandbyReplicas*/ 1, + /*probingRebalanceIntervalMs*/ 60 * 1000L + ); - @Test - public void shouldDecidePreviousAssignmentIsInvalidIfThereAreUnassignedActiveTasks() { - client1 = EasyMock.createNiceMock(ClientState.class); - expect(client1.prevActiveTasks()).andReturn(singleton(TASK_0_0)); - expect(client1.prevStandbyTasks()).andStubReturn(EMPTY_TASKS); - replay(client1); - allTasks = mkSet(TASK_0_0, TASK_0_1); - clientStates = singletonMap(UUID_1, client1); - createTaskAssignor(); - assertFalse(taskAssignor.previousAssignmentIsValid()); + @Test + public void shouldComputeNewAssignmentIfThereAreUnassignedActiveTasks() { + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final ClientState client1 = new ClientState(singleton(TASK_0_0), emptySet(), singletonMap(TASK_0_0, 0L), 1); + final Map clientStates = singletonMap(UUID_1, client1); + + final boolean probingRebalanceNeeded = new HighAvailabilityTaskAssignor().assign(clientStates, + allTasks, + singleton(TASK_0_0), + configWithoutStandbys); + + assertThat(clientStates.get(UUID_1).activeTasks(), not(singleton(TASK_0_0))); + assertThat(clientStates.get(UUID_1).standbyTasks(), empty()); + assertThat(probingRebalanceNeeded, is(false)); } @Test - public void shouldDecidePreviousAssignmentIsInvalidIfThereAreUnassignedStandbyTasks() { - client1 = EasyMock.createNiceMock(ClientState.class); - expect(client1.prevActiveTasks()).andStubReturn(singleton(TASK_0_0)); - expect(client1.prevStandbyTasks()).andReturn(EMPTY_TASKS); - replay(client1); - allTasks = mkSet(TASK_0_0); - statefulTasks = mkSet(TASK_0_0); - clientStates = singletonMap(UUID_1, client1); - numStandbyReplicas = 1; - createTaskAssignor(); - - assertFalse(taskAssignor.previousAssignmentIsValid()); + public void shouldComputeNewAssignmentIfThereAreUnassignedStandbyTasks() { + final Set allTasks = mkSet(TASK_0_0); + final Set statefulTasks = mkSet(TASK_0_0); + final ClientState client1 = new ClientState(singleton(TASK_0_0), emptySet(), singletonMap(TASK_0_0, 0L), 1); + final ClientState client2 = new ClientState(emptySet(), emptySet(), singletonMap(TASK_0_0, 0L), 1); + final Map clientStates = mkMap(mkEntry(UUID_1, client1), mkEntry(UUID_2, client2)); + + final boolean probingRebalanceNeeded = new HighAvailabilityTaskAssignor().assign(clientStates, + allTasks, + statefulTasks, + configWithStandbys); + + assertThat(clientStates.get(UUID_2).standbyTasks(), not(empty())); + assertThat(probingRebalanceNeeded, is(false)); } @Test - public void shouldDecidePreviousAssignmentIsInvalidIfActiveTasksWasNotOnCaughtUpClient() { - client1 = EasyMock.createNiceMock(ClientState.class); - client2 = EasyMock.createNiceMock(ClientState.class); - expect(client1.prevStandbyTasks()).andStubReturn(EMPTY_TASKS); - expect(client2.prevStandbyTasks()).andStubReturn(EMPTY_TASKS); - - expect(client1.prevActiveTasks()).andReturn(singleton(TASK_0_0)); - expect(client2.prevActiveTasks()).andReturn(singleton(TASK_0_1)); - expect(client1.lagFor(TASK_0_0)).andReturn(500L); - expect(client2.lagFor(TASK_0_0)).andReturn(0L); - replay(client1, client2); - - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = mkSet(TASK_0_0); - clientStates = mkMap( + public void shouldComputeNewAssignmentIfActiveTasksWasNotOnCaughtUpClient() { + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = mkSet(TASK_0_0); + final ClientState client1 = new ClientState(singleton(TASK_0_0), emptySet(), singletonMap(TASK_0_0, 500L), 1); + final ClientState client2 = new ClientState(singleton(TASK_0_1), emptySet(), singletonMap(TASK_0_0, 0L), 1); + final Map clientStates = mkMap( mkEntry(UUID_1, client1), mkEntry(UUID_2, client2) ); - createTaskAssignor(); - assertFalse(taskAssignor.previousAssignmentIsValid()); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithoutStandbys); + + assertThat(clientStates.get(UUID_1).activeTasks(), is(singleton(TASK_0_1))); + assertThat(clientStates.get(UUID_2).activeTasks(), is(singleton(TASK_0_0))); + // we'll warm up task 0_0 on client1 because it's first in sorted order, + // although this isn't an optimal convergence + assertThat(probingRebalanceNeeded, is(true)); } @Test - public void shouldDecidePreviousAssignmentIsValid() { - client1 = EasyMock.createNiceMock(ClientState.class); - client2 = EasyMock.createNiceMock(ClientState.class); - expect(client1.prevStandbyTasks()).andStubReturn(EMPTY_TASKS); - expect(client2.prevStandbyTasks()).andStubReturn(EMPTY_TASKS); - - expect(client1.prevActiveTasks()).andReturn(singleton(TASK_0_0)); - expect(client2.prevActiveTasks()).andReturn(singleton(TASK_0_1)); - expect(client1.lagFor(TASK_0_0)).andReturn(0L); - expect(client2.lagFor(TASK_0_0)).andReturn(0L); - replay(client1, client2); - - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = mkSet(TASK_0_0); - clientStates = mkMap( + public void shouldReusePreviousAssignmentIfItIsAlreadyBalanced() { + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = mkSet(TASK_0_0); + final ClientState client1 = new ClientState(singleton(TASK_0_0), emptySet(), singletonMap(TASK_0_0, 0L), 1); + final ClientState client2 = + new ClientState(singleton(TASK_0_1), emptySet(), mkMap(mkEntry(TASK_0_0, 0L), mkEntry(TASK_0_1, 0L)), 1); + final Map clientStates = mkMap( mkEntry(UUID_1, client1), mkEntry(UUID_2, client2) ); - createTaskAssignor(); - assertTrue(taskAssignor.previousAssignmentIsValid()); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithoutStandbys); + + assertThat(clientStates.get(UUID_1).activeTasks(), is(singleton(TASK_0_0))); + assertThat(clientStates.get(UUID_2).activeTasks(), is(singleton(TASK_0_1))); + assertThat(probingRebalanceNeeded, is(false)); } @Test public void shouldComputeBalanceFactorAsDifferenceBetweenMostAndLeastLoadedClients() { - client1 = EasyMock.createNiceMock(ClientState.class); - client2 = EasyMock.createNiceMock(ClientState.class); - client3 = EasyMock.createNiceMock(ClientState.class); + final ClientState client1 = EasyMock.createNiceMock(ClientState.class); + final ClientState client2 = EasyMock.createNiceMock(ClientState.class); + final ClientState client3 = EasyMock.createNiceMock(ClientState.class); final Set states = mkSet(client1, client2, client3); final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_1_0, TASK_1_1, TASK_2_0, TASK_2_1, TASK_2_3); @@ -186,9 +174,9 @@ public void shouldComputeBalanceFactorAsDifferenceBetweenMostAndLeastLoadedClien @Test public void shouldComputeBalanceFactorWithDifferentClientCapacities() { - client1 = EasyMock.createNiceMock(ClientState.class); - client2 = EasyMock.createNiceMock(ClientState.class); - client3 = EasyMock.createNiceMock(ClientState.class); + final ClientState client1 = EasyMock.createNiceMock(ClientState.class); + final ClientState client2 = EasyMock.createNiceMock(ClientState.class); + final ClientState client3 = EasyMock.createNiceMock(ClientState.class); final Set states = mkSet(client1, client2, client3); final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_1_0, TASK_1_1, TASK_2_0, TASK_2_1, TASK_2_3); @@ -211,9 +199,9 @@ public void shouldComputeBalanceFactorWithDifferentClientCapacities() { @Test public void shouldComputeBalanceFactorBasedOnStatefulTasksOnly() { - client1 = EasyMock.createNiceMock(ClientState.class); - client2 = EasyMock.createNiceMock(ClientState.class); - client3 = EasyMock.createNiceMock(ClientState.class); + final ClientState client1 = EasyMock.createNiceMock(ClientState.class); + final ClientState client2 = EasyMock.createNiceMock(ClientState.class); + final ClientState client3 = EasyMock.createNiceMock(ClientState.class); final Set states = mkSet(client1, client2, client3); // 0_0 and 0_1 are stateless @@ -238,7 +226,7 @@ public void shouldComputeBalanceFactorBasedOnStatefulTasksOnly() { @Test public void shouldComputeBalanceFactorOfZeroWithOnlyOneClient() { final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - client1 = EasyMock.createNiceMock(ClientState.class); + final ClientState client1 = EasyMock.createNiceMock(ClientState.class); expect(client1.capacity()).andReturn(1); expect(client1.prevActiveTasks()).andReturn(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3)); replay(client1); @@ -247,239 +235,268 @@ public void shouldComputeBalanceFactorOfZeroWithOnlyOneClient() { @Test public void shouldAssignStandbysForStatefulTasks() { - numStandbyReplicas = 1; - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = mkSet(TASK_0_0, TASK_0_1); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1); + + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0), statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_1), statefulTasks); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0)); - client2 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_1)); + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithStandbys); - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - taskAssignor.assign(); assertThat(client1.activeTasks(), equalTo(mkSet(TASK_0_0))); assertThat(client2.activeTasks(), equalTo(mkSet(TASK_0_1))); assertThat(client1.standbyTasks(), equalTo(mkSet(TASK_0_1))); assertThat(client2.standbyTasks(), equalTo(mkSet(TASK_0_0))); + assertThat(probingRebalanceNeeded, is(false)); } @Test public void shouldNotAssignStandbysForStatelessTasks() { - numStandbyReplicas = 1; - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = EMPTY_TASKS; + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = EMPTY_TASKS; - client1 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithStandbys); - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - taskAssignor.assign(); assertThat(client1.activeTaskCount(), equalTo(1)); assertThat(client2.activeTaskCount(), equalTo(1)); assertHasNoStandbyTasks(client1, client2); + assertThat(probingRebalanceNeeded, is(false)); } @Test public void shouldAssignWarmupReplicasEvenIfNoStandbyReplicasConfigured() { - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = mkSet(TASK_0_0, TASK_0_1); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1)); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); - - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - taskAssignor.assign(); - + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1), statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithoutStandbys); + + assertThat(client1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1))); assertThat(client2.standbyTaskCount(), equalTo(1)); assertHasNoStandbyTasks(client1); assertHasNoActiveTasks(client2); + assertThat(probingRebalanceNeeded, is(true)); } - @Test public void shouldNotAssignMoreThanMaxWarmupReplicas() { - maxWarmupReplicas = 1; - allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3)); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3), statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = new HighAvailabilityTaskAssignor().assign( + clientStates, + allTasks, + statefulTasks, + new AssignmentConfigs( + /*acceptableRecoveryLag*/ 100L, + /*balanceFactor*/ 1, + /*maxWarmupReplicas*/ 1, + /*numStandbyReplicas*/ 0, + /*probingRebalanceIntervalMs*/ 60 * 1000L + ) + ); - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - taskAssignor.assign(); assertThat(client1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3))); assertThat(client2.standbyTaskCount(), equalTo(1)); assertHasNoStandbyTasks(client1); assertHasNoActiveTasks(client2); + assertThat(probingRebalanceNeeded, is(true)); } @Test public void shouldNotAssignWarmupAndStandbyToTheSameClient() { - numStandbyReplicas = 1; - maxWarmupReplicas = 1; - - allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3)); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3), statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - taskAssignor.assign(); + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = new HighAvailabilityTaskAssignor().assign( + clientStates, + allTasks, + statefulTasks, + new AssignmentConfigs( + /*acceptableRecoveryLag*/ 100L, + /*balanceFactor*/ 1, + /*maxWarmupReplicas*/ 1, + /*numStandbyReplicas*/ 1, + /*probingRebalanceIntervalMs*/ 60 * 1000L + ) + ); assertThat(client1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3))); assertThat(client2.standbyTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3))); assertHasNoStandbyTasks(client1); assertHasNoActiveTasks(client2); + assertThat(probingRebalanceNeeded, is(true)); } @Test public void shouldNotAssignAnyStandbysWithInsufficientCapacity() { - numStandbyReplicas = 1; - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = mkSet(TASK_0_0, TASK_0_1); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1)); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1), statefulTasks); - clientStates = getClientStatesMap(client1); - createTaskAssignor(); - taskAssignor.assign(); + final Map clientStates = getClientStatesMap(client1); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithStandbys); assertThat(client1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1))); assertHasNoStandbyTasks(client1); + assertThat(probingRebalanceNeeded, is(false)); } @Test public void shouldAssignActiveTasksToNotCaughtUpClientIfNoneExist() { - numStandbyReplicas = 1; - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = mkSet(TASK_0_0, TASK_0_1); - client1 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); - clientStates = getClientStatesMap(client1); - createTaskAssignor(); - taskAssignor.assign(); + final Map clientStates = getClientStatesMap(client1); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithStandbys); assertThat(client1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1))); assertHasNoStandbyTasks(client1); + assertThat(probingRebalanceNeeded, is(false)); } @Test public void shouldNotAssignMoreThanMaxWarmupReplicasWithStandbys() { - numStandbyReplicas = 1; - - allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3)); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); - client3 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(statefulTasks, statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + final ClientState client3 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); - clientStates = getClientStatesMap(client1, client2, client3); - createTaskAssignor(); - taskAssignor.assign(); + final Map clientStates = getClientStatesMap(client1, client2, client3); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithStandbys); assertThat(client1.activeTaskCount(), equalTo(4)); assertThat(client2.standbyTaskCount(), equalTo(3)); // 1 assertThat(client3.standbyTaskCount(), equalTo(3)); assertHasNoStandbyTasks(client1); assertHasNoActiveTasks(client2, client3); + assertThat(probingRebalanceNeeded, is(true)); } @Test public void shouldDistributeStatelessTasksToBalanceTotalTaskLoad() { - numStandbyReplicas = 1; - allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_1_0, TASK_1_1, TASK_1_2); - statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_1_0, TASK_1_1, TASK_1_2); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3)); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(statefulTasks, statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - taskAssignor.assign(); + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithStandbys); assertThat(client1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_1_0, TASK_1_2))); assertHasNoStandbyTasks(client1); assertThat(client2.activeTasks(), equalTo(mkSet(TASK_1_1))); assertThat(client2.standbyTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3))); + assertThat(probingRebalanceNeeded, is(true)); } @Test public void shouldDistributeStatefulActiveTasksToAllClients() { - allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_1_0, TASK_1_1, TASK_1_2, TASK_1_3, TASK_2_0); // 9 total - statefulTasks = new HashSet<>(allTasks); - client1 = getMockClientWithPreviousCaughtUpTasks(allTasks).withCapacity(100); - client2 = getMockClientWithPreviousCaughtUpTasks(allTasks).withCapacity(50); - client3 = getMockClientWithPreviousCaughtUpTasks(allTasks).withCapacity(1); - - clientStates = getClientStatesMap(client1, client2, client3); - createTaskAssignor(); - taskAssignor.assign(); - - assertFalse(client1.activeTasks().isEmpty()); - assertFalse(client2.activeTasks().isEmpty()); - assertFalse(client3.activeTasks().isEmpty()); + final Set allTasks = + mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_1_0, TASK_1_1, TASK_1_2, TASK_1_3, TASK_2_0); // 9 total + final Map allTaskLags = allTasks.stream().collect(Collectors.toMap(t -> t, t -> 0L)); + final Set statefulTasks = new HashSet<>(allTasks); + final ClientState client1 = new ClientState(emptySet(), emptySet(), allTaskLags, 100); + final ClientState client2 = new ClientState(emptySet(), emptySet(), allTaskLags, 50); + final ClientState client3 = new ClientState(emptySet(), emptySet(), allTaskLags, 1); + + final Map clientStates = getClientStatesMap(client1, client2, client3); + + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithoutStandbys); + + assertThat(client1.activeTasks(), not(empty())); + assertThat(client2.activeTasks(), not(empty())); + assertThat(client3.activeTasks(), not(empty())); + assertThat(probingRebalanceNeeded, is(false)); } @Test public void shouldReturnFalseIfPreviousAssignmentIsReused() { - allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - statefulTasks = new HashSet<>(allTasks); - client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_2)); - client2 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_1, TASK_0_3)); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final Set statefulTasks = new HashSet<>(allTasks); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_0, TASK_0_2), statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(mkSet(TASK_0_1, TASK_0_3), statefulTasks); - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - assertFalse(taskAssignor.assign()); + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithoutStandbys); + assertThat(probingRebalanceNeeded, is(false)); assertThat(client1.activeTasks(), equalTo(client1.prevActiveTasks())); assertThat(client2.activeTasks(), equalTo(client2.prevActiveTasks())); } @Test public void shouldReturnFalseIfNoWarmupTasksAreAssigned() { - allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - statefulTasks = EMPTY_TASKS; - client1 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); - - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - assertFalse(taskAssignor.assign()); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + final Set statefulTasks = EMPTY_TASKS; + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithoutStandbys); + assertThat(probingRebalanceNeeded, is(false)); assertHasNoStandbyTasks(client1, client2); } @Test public void shouldReturnTrueIfWarmupTasksAreAssigned() { - allTasks = mkSet(TASK_0_0, TASK_0_1); - statefulTasks = mkSet(TASK_0_0, TASK_0_1); - client1 = getMockClientWithPreviousCaughtUpTasks(allTasks); - client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS); - - clientStates = getClientStatesMap(client1, client2); - createTaskAssignor(); - assertTrue(taskAssignor.assign()); + final Set allTasks = mkSet(TASK_0_0, TASK_0_1); + final Set statefulTasks = mkSet(TASK_0_0, TASK_0_1); + final ClientState client1 = getMockClientWithPreviousCaughtUpTasks(allTasks, statefulTasks); + final ClientState client2 = getMockClientWithPreviousCaughtUpTasks(EMPTY_TASKS, statefulTasks); + + final Map clientStates = getClientStatesMap(client1, client2); + final boolean probingRebalanceNeeded = + new HighAvailabilityTaskAssignor().assign(clientStates, allTasks, statefulTasks, configWithoutStandbys); + assertThat(probingRebalanceNeeded, is(true)); assertThat(client2.standbyTaskCount(), equalTo(1)); } private static void assertHasNoActiveTasks(final ClientState... clients) { for (final ClientState client : clients) { - assertTrue(client.activeTasks().isEmpty()); + assertThat(client.activeTasks(), is(empty())); } } private static void assertHasNoStandbyTasks(final ClientState... clients) { for (final ClientState client : clients) { - assertTrue(client.standbyTasks().isEmpty()); + assertThat(client.standbyTasks(), is(empty())); } } - private MockClientState getMockClientWithPreviousCaughtUpTasks(final Set statefulActiveTasks) { + private static ClientState getMockClientWithPreviousCaughtUpTasks(final Set statefulActiveTasks, + final Set statefulTasks) { if (!statefulTasks.containsAll(statefulActiveTasks)) { throw new IllegalArgumentException("Need to initialize stateful tasks set before creating mock clients"); } @@ -491,32 +508,6 @@ private MockClientState getMockClientWithPreviousCaughtUpTasks(final Set taskLags.put(task, Long.MAX_VALUE); } } - final MockClientState client = new MockClientState(1, taskLags); - client.addPreviousActiveTasks(statefulActiveTasks); - return client; - } - - static class MockClientState extends ClientState { - private final Map taskLagTotals; - - private MockClientState(final int capacity, - final Map taskLagTotals) { - super(capacity); - this.taskLagTotals = taskLagTotals; - } - - @Override - long lagFor(final TaskId task) { - final Long totalLag = taskLagTotals.get(task); - if (totalLag == null) { - return Long.MAX_VALUE; - } else { - return totalLag; - } - } - - MockClientState withCapacity(final int capacity) { - return new MockClientState(capacity, taskLagTotals); - } + return new ClientState(statefulActiveTasks, emptySet(), taskLags, 1); } } diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignorTest.java new file mode 100644 index 0000000000000..5d256bbd8d7b6 --- /dev/null +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignorTest.java @@ -0,0 +1,74 @@ +/* + * 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.processor.internals.assignment; + +import org.apache.kafka.streams.processor.TaskId; +import org.junit.Test; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import java.util.UUID; + +import static java.util.Arrays.asList; +import static org.apache.kafka.common.utils.Utils.mkSet; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_0; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_1; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_2; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_1; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_2; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class PriorTaskAssignorTest { + + private final Map clients = new TreeMap<>(); + + @Test + public void shouldViolateBalanceToPreserveActiveTaskStickiness() { + final ClientState c1 = createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0, TASK_0_1, TASK_0_2); + final ClientState c2 = createClient(UUID_2, 1); + + final List taskIds = asList(TASK_0_0, TASK_0_1, TASK_0_2); + Collections.shuffle(taskIds); + final boolean followupRebalanceNeeded = new PriorTaskAssignor().assign( + clients, + new HashSet<>(taskIds), + new HashSet<>(taskIds), + new AssignorConfiguration.AssignmentConfigs(0L, 0, 0, 0, 0L) + ); + assertThat(followupRebalanceNeeded, is(false)); + + assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2))); + assertThat(c2.activeTasks(), empty()); + } + + private ClientState createClient(final UUID processId, final int capacity) { + return createClientWithPreviousActiveTasks(processId, capacity); + } + + private ClientState createClientWithPreviousActiveTasks(final UUID processId, final int capacity, final TaskId... taskIds) { + final ClientState clientState = new ClientState(capacity); + clientState.addPreviousActiveTasks(mkSet(taskIds)); + clients.put(processId, clientState); + return clientState; + } +} diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java index d241a57a3a127..3e4730c03173b 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java @@ -16,10 +16,7 @@ */ package org.apache.kafka.streams.processor.internals.assignment; -import java.util.UUID; -import org.apache.kafka.common.utils.Utils; import org.apache.kafka.streams.processor.TaskId; -import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration.AssignmentConfigs; import org.junit.Test; import java.util.ArrayList; @@ -31,8 +28,11 @@ import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; +import java.util.UUID; import static java.util.Arrays.asList; +import static java.util.Collections.singleton; +import static org.apache.kafka.common.utils.Utils.mkSet; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_0; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_1; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_2; @@ -54,12 +54,15 @@ import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_4; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_5; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_6; -import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.IsIterableContaining.hasItem; -import static org.hamcrest.core.IsIterableContaining.hasItems; -import static org.hamcrest.core.IsNot.not; -import static org.junit.Assert.assertTrue; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.not; public class StickyTaskAssignorTest { @@ -73,11 +76,11 @@ public void shouldAssignOneActiveTaskToEachProcessWhenTaskCountSameAsProcessCoun createClient(UUID_2, 1); createClient(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(followupRebalanceNeeded, is(false)); - for (final UUID processId : clients.keySet()) { - assertThat(clients.get(processId).activeTaskCount(), equalTo(1)); + for (final ClientState clientState : clients.values()) { + assertThat(clientState.activeTaskCount(), equalTo(1)); } } @@ -87,8 +90,9 @@ public void shouldAssignTopicGroupIdEvenlyAcrossClientsWithNoStandByTasks() { createClient(UUID_2, 2); createClient(UUID_3, 2); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_1_0, TASK_1_1, TASK_2_2, TASK_2_0, TASK_2_1, TASK_1_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_1_0, TASK_1_1, TASK_2_2, TASK_2_0, TASK_2_1, TASK_1_2); + assertThat(followupRebalanceNeeded, is(false)); + assertActiveTaskTopicGroupIdsEvenlyDistributed(); } @@ -98,8 +102,9 @@ public void shouldAssignTopicGroupIdEvenlyAcrossClientsWithStandByTasks() { createClient(UUID_2, 2); createClient(UUID_3, 2); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_2_0, TASK_1_1, TASK_1_2, TASK_1_0, TASK_2_1, TASK_2_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_2_0, TASK_1_1, TASK_1_2, TASK_1_0, TASK_2_1, TASK_2_2); + assertThat(followupRebalanceNeeded, is(false)); + assertActiveTaskTopicGroupIdsEvenlyDistributed(); } @@ -108,8 +113,7 @@ public void shouldNotMigrateActiveTaskToOtherProcess() { createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0); createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1); - final StickyTaskAssignor firstAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - firstAssignor.assign(); + assertThat(assign(TASK_0_0, TASK_0_1, TASK_0_2), is(false)); assertThat(clients.get(UUID_1).activeTasks(), hasItems(TASK_0_0)); assertThat(clients.get(UUID_2).activeTasks(), hasItems(TASK_0_1)); @@ -121,8 +125,7 @@ public void shouldNotMigrateActiveTaskToOtherProcess() { createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_1); createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_2); - final StickyTaskAssignor secondAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - secondAssignor.assign(); + assertThat(assign(TASK_0_0, TASK_0_1, TASK_0_2), is(false)); assertThat(clients.get(UUID_1).activeTasks(), hasItems(TASK_0_1)); assertThat(clients.get(UUID_2).activeTasks(), hasItems(TASK_0_2)); @@ -135,11 +138,10 @@ public void shouldMigrateActiveTasksToNewProcessWithoutChangingAllAssignments() createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1); createClient(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(clients.get(UUID_2).activeTasks(), equalTo(Collections.singleton(TASK_0_1))); + assertThat(followupRebalanceNeeded, is(false)); + assertThat(clients.get(UUID_2).activeTasks(), equalTo(singleton(TASK_0_1))); assertThat(clients.get(UUID_1).activeTasks().size(), equalTo(1)); assertThat(clients.get(UUID_3).activeTasks().size(), equalTo(1)); assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2))); @@ -149,9 +151,9 @@ public void shouldMigrateActiveTasksToNewProcessWithoutChangingAllAssignments() public void shouldAssignBasedOnCapacity() { createClient(UUID_1, 1); createClient(UUID_2, 2); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks().size(), equalTo(1)); assertThat(clients.get(UUID_2).activeTasks().size(), equalTo(2)); } @@ -162,31 +164,29 @@ public void shouldAssignTasksEvenlyWithUnequalTopicGroupSizes() { createClient(UUID_2, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_1_0, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5); + assertThat(assign(TASK_1_0, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5), is(false)); final Set expectedClientITasks = new HashSet<>(asList(TASK_0_0, TASK_0_1, TASK_1_0, TASK_0_5)); final Set expectedClientIITasks = new HashSet<>(asList(TASK_0_2, TASK_0_3, TASK_0_4)); - taskAssignor.assign(); assertThat(clients.get(UUID_1).activeTasks(), equalTo(expectedClientITasks)); assertThat(clients.get(UUID_2).activeTasks(), equalTo(expectedClientIITasks)); } @Test - public void shouldKeepActiveTaskStickynessWhenMoreClientThanActiveTasks() { + public void shouldKeepActiveTaskStickinessWhenMoreClientThanActiveTasks() { createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0); createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_2); createClientWithPreviousActiveTasks(UUID_3, 1, TASK_0_1); createClient(UUID_4, 1); createClient(UUID_5, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + assertThat(assign(TASK_0_0, TASK_0_1, TASK_0_2), is(false)); - assertThat(clients.get(UUID_1).activeTasks(), equalTo(Collections.singleton(TASK_0_0))); - assertThat(clients.get(UUID_2).activeTasks(), equalTo(Collections.singleton(TASK_0_2))); - assertThat(clients.get(UUID_3).activeTasks(), equalTo(Collections.singleton(TASK_0_1))); + assertThat(clients.get(UUID_1).activeTasks(), equalTo(singleton(TASK_0_0))); + assertThat(clients.get(UUID_2).activeTasks(), equalTo(singleton(TASK_0_2))); + assertThat(clients.get(UUID_3).activeTasks(), equalTo(singleton(TASK_0_1))); // change up the assignment and make sure it is still sticky clients.clear(); @@ -196,72 +196,72 @@ public void shouldKeepActiveTaskStickynessWhenMoreClientThanActiveTasks() { createClientWithPreviousActiveTasks(UUID_4, 1, TASK_0_2); createClientWithPreviousActiveTasks(UUID_5, 1, TASK_0_1); - final StickyTaskAssignor secondAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - secondAssignor.assign(); + assertThat(assign(TASK_0_0, TASK_0_1, TASK_0_2), is(false)); - assertThat(clients.get(UUID_2).activeTasks(), equalTo(Collections.singleton(TASK_0_0))); - assertThat(clients.get(UUID_4).activeTasks(), equalTo(Collections.singleton(TASK_0_2))); - assertThat(clients.get(UUID_5).activeTasks(), equalTo(Collections.singleton(TASK_0_1))); + assertThat(clients.get(UUID_2).activeTasks(), equalTo(singleton(TASK_0_0))); + assertThat(clients.get(UUID_4).activeTasks(), equalTo(singleton(TASK_0_2))); + assertThat(clients.get(UUID_5).activeTasks(), equalTo(singleton(TASK_0_1))); } @Test public void shouldAssignTasksToClientWithPreviousStandbyTasks() { final ClientState client1 = createClient(UUID_1, 1); - client1.addPreviousStandbyTasks(Utils.mkSet(TASK_0_2)); + client1.addPreviousStandbyTasks(mkSet(TASK_0_2)); final ClientState client2 = createClient(UUID_2, 1); - client2.addPreviousStandbyTasks(Utils.mkSet(TASK_0_1)); + client2.addPreviousStandbyTasks(mkSet(TASK_0_1)); final ClientState client3 = createClient(UUID_3, 1); - client3.addPreviousStandbyTasks(Utils.mkSet(TASK_0_0)); + client3.addPreviousStandbyTasks(mkSet(TASK_0_0)); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + assertThat(followupRebalanceNeeded, is(false)); - assertThat(clients.get(UUID_1).activeTasks(), equalTo(Collections.singleton(TASK_0_2))); - assertThat(clients.get(UUID_2).activeTasks(), equalTo(Collections.singleton(TASK_0_1))); - assertThat(clients.get(UUID_3).activeTasks(), equalTo(Collections.singleton(TASK_0_0))); + assertThat(clients.get(UUID_1).activeTasks(), equalTo(singleton(TASK_0_2))); + assertThat(clients.get(UUID_2).activeTasks(), equalTo(singleton(TASK_0_1))); + assertThat(clients.get(UUID_3).activeTasks(), equalTo(singleton(TASK_0_0))); } @Test public void shouldAssignBasedOnCapacityWhenMultipleClientHaveStandbyTasks() { final ClientState c1 = createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0); - c1.addPreviousStandbyTasks(Utils.mkSet(TASK_0_1)); + c1.addPreviousStandbyTasks(mkSet(TASK_0_1)); final ClientState c2 = createClientWithPreviousActiveTasks(UUID_2, 2, TASK_0_2); - c2.addPreviousStandbyTasks(Utils.mkSet(TASK_0_1)); + c2.addPreviousStandbyTasks(mkSet(TASK_0_1)); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + assertThat(followupRebalanceNeeded, is(false)); - assertThat(clients.get(UUID_1).activeTasks(), equalTo(Collections.singleton(TASK_0_0))); - assertThat(clients.get(UUID_2).activeTasks(), equalTo(Utils.mkSet(TASK_0_2, TASK_0_1))); + assertThat(clients.get(UUID_1).activeTasks(), equalTo(singleton(TASK_0_0))); + assertThat(clients.get(UUID_2).activeTasks(), equalTo(mkSet(TASK_0_2, TASK_0_1))); } @Test - public void shouldAssignStandbyTasksToDifferentClientThanCorrespondingActiveTaskIsAssingedTo() { + public void shouldAssignStandbyTasksToDifferentClientThanCorrespondingActiveTaskIsAssignedTo() { createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0); createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1); createClientWithPreviousActiveTasks(UUID_3, 1, TASK_0_2); createClientWithPreviousActiveTasks(UUID_4, 1, TASK_0_3); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); + assertThat(clients.get(UUID_1).standbyTasks(), not(hasItems(TASK_0_0))); - assertTrue(clients.get(UUID_1).standbyTasks().size() <= 2); + assertThat(clients.get(UUID_1).standbyTasks().size(), lessThanOrEqualTo(2)); assertThat(clients.get(UUID_2).standbyTasks(), not(hasItems(TASK_0_1))); - assertTrue(clients.get(UUID_2).standbyTasks().size() <= 2); + assertThat(clients.get(UUID_2).standbyTasks().size(), lessThanOrEqualTo(2)); assertThat(clients.get(UUID_3).standbyTasks(), not(hasItems(TASK_0_2))); - assertTrue(clients.get(UUID_3).standbyTasks().size() <= 2); + assertThat(clients.get(UUID_3).standbyTasks().size(), lessThanOrEqualTo(2)); assertThat(clients.get(UUID_4).standbyTasks(), not(hasItems(TASK_0_3))); - assertTrue(clients.get(UUID_4).standbyTasks().size() <= 2); + assertThat(clients.get(UUID_4).standbyTasks().size(), lessThanOrEqualTo(2)); int nonEmptyStandbyTaskCount = 0; - for (final UUID client : clients.keySet()) { - nonEmptyStandbyTaskCount += clients.get(client).standbyTasks().isEmpty() ? 0 : 1; + for (final ClientState clientState : clients.values()) { + nonEmptyStandbyTaskCount += clientState.standbyTasks().isEmpty() ? 0 : 1; } - assertTrue(nonEmptyStandbyTaskCount >= 3); + assertThat(nonEmptyStandbyTaskCount, greaterThanOrEqualTo(3)); assertThat(allStandbyTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3))); } @@ -271,19 +271,19 @@ public void shouldAssignMultipleReplicasOfStandbyTask() { createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1); createClientWithPreviousActiveTasks(UUID_3, 1, TASK_0_2); - final StickyTaskAssignor taskAssignor = createTaskAssignor(2, TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(2, TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(followupRebalanceNeeded, is(false)); - assertThat(clients.get(UUID_1).standbyTasks(), equalTo(Utils.mkSet(TASK_0_1, TASK_0_2))); - assertThat(clients.get(UUID_2).standbyTasks(), equalTo(Utils.mkSet(TASK_0_2, TASK_0_0))); - assertThat(clients.get(UUID_3).standbyTasks(), equalTo(Utils.mkSet(TASK_0_0, TASK_0_1))); + assertThat(clients.get(UUID_1).standbyTasks(), equalTo(mkSet(TASK_0_1, TASK_0_2))); + assertThat(clients.get(UUID_2).standbyTasks(), equalTo(mkSet(TASK_0_2, TASK_0_0))); + assertThat(clients.get(UUID_3).standbyTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1))); } @Test public void shouldNotAssignStandbyTaskReplicasWhenNoClientAvailableWithoutHavingTheTaskAssigned() { createClient(UUID_1, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_0_0); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_0_0); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).standbyTasks().size(), equalTo(0)); } @@ -293,8 +293,8 @@ public void shouldAssignActiveAndStandbyTasks() { createClient(UUID_2, 1); createClient(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(followupRebalanceNeeded, is(false)); assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2))); assertThat(allStandbyTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2))); @@ -306,8 +306,8 @@ public void shouldAssignAtLeastOneTaskToEachClientIfPossible() { createClient(UUID_2, 1); createClient(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_3).assignedTaskCount(), equalTo(1)); @@ -322,8 +322,8 @@ public void shouldAssignEachActiveTaskToOneClientWhenMoreClientsThanTasks() { createClient(UUID_5, 1); createClient(UUID_6, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(followupRebalanceNeeded, is(false)); assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2))); } @@ -337,8 +337,8 @@ public void shouldBalanceActiveAndStandbyTasksAcrossAvailableClients() { createClient(UUID_5, 1); createClient(UUID_6, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(followupRebalanceNeeded, is(false)); for (final ClientState clientState : clients.values()) { assertThat(clientState.assignedTaskCount(), equalTo(1)); @@ -350,20 +350,20 @@ public void shouldAssignMoreTasksToClientWithMoreCapacity() { createClient(UUID_2, 2); createClient(UUID_1, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, - TASK_0_1, - TASK_0_2, - new TaskId(1, 0), - new TaskId(1, 1), - new TaskId(1, 2), - new TaskId(2, 0), - new TaskId(2, 1), - new TaskId(2, 2), - new TaskId(3, 0), - new TaskId(3, 1), - new TaskId(3, 2)); - - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, + TASK_0_1, + TASK_0_2, + new TaskId(1, 0), + new TaskId(1, 1), + new TaskId(1, 2), + new TaskId(2, 0), + new TaskId(2, 1), + new TaskId(2, 2), + new TaskId(3, 0), + new TaskId(3, 1), + new TaskId(3, 2)); + + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(8)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(4)); } @@ -387,8 +387,8 @@ public void shouldEvenlyDistributeByTaskIdAndPartition() { Collections.shuffle(taskIds); taskIds.toArray(taskIdArray); - final StickyTaskAssignor taskAssignor = createTaskAssignor(taskIdArray); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(taskIdArray); + assertThat(followupRebalanceNeeded, is(false)); Collections.sort(taskIds); final Set expectedClientOneAssignment = getExpectedTaskIdAssignment(taskIds, 0, 4, 8, 12); @@ -412,8 +412,8 @@ public void shouldNotHaveSameAssignmentOnAnyTwoHosts() { createClient(UUID_3, 1); createClient(UUID_4, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); for (final UUID uuid : allUUIDs) { final Set taskIds = clients.get(uuid).assignedTasks(); @@ -435,8 +435,8 @@ public void shouldNotHaveSameAssignmentOnAnyTwoHostsWhenThereArePreviousActiveTa createClientWithPreviousActiveTasks(UUID_3, 1, TASK_0_0); createClient(UUID_4, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); for (final UUID uuid : allUUIDs) { final Set taskIds = clients.get(uuid).assignedTasks(); @@ -455,15 +455,15 @@ public void shouldNotHaveSameAssignmentOnAnyTwoHostsWhenThereArePreviousStandbyT final List allUUIDs = asList(UUID_1, UUID_2, UUID_3, UUID_4); final ClientState c1 = createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_1, TASK_0_2); - c1.addPreviousStandbyTasks(Utils.mkSet(TASK_0_3, TASK_0_0)); + c1.addPreviousStandbyTasks(mkSet(TASK_0_3, TASK_0_0)); final ClientState c2 = createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_3, TASK_0_0); - c2.addPreviousStandbyTasks(Utils.mkSet(TASK_0_1, TASK_0_2)); + c2.addPreviousStandbyTasks(mkSet(TASK_0_1, TASK_0_2)); createClient(UUID_3, 1); createClient(UUID_4, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); for (final UUID uuid : allUUIDs) { final Set taskIds = clients.get(uuid).assignedTasks(); @@ -484,8 +484,8 @@ public void shouldReBalanceTasksAcrossAllClientsWhenCapacityAndTaskCountTheSame( createClient(UUID_2, 1); createClient(UUID_4, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(1)); @@ -499,8 +499,8 @@ public void shouldReBalanceTasksAcrossClientsWhenCapacityLessThanTaskCount() { createClient(UUID_1, 1); createClient(UUID_2, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_3).assignedTaskCount(), equalTo(2)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(1)); @@ -511,23 +511,23 @@ public void shouldReBalanceTasksAcrossClientsWhenCapacityLessThanTaskCount() { public void shouldRebalanceTasksToClientsBasedOnCapacity() { createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_0, TASK_0_3, TASK_0_2); createClient(UUID_3, 2); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_2, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_3).assignedTaskCount(), equalTo(2)); } @Test public void shouldMoveMinimalNumberOfTasksWhenPreviouslyAboveCapacityAndNewClientAdded() { - final Set p1PrevTasks = Utils.mkSet(TASK_0_0, TASK_0_2); - final Set p2PrevTasks = Utils.mkSet(TASK_0_1, TASK_0_3); + final Set p1PrevTasks = mkSet(TASK_0_0, TASK_0_2); + final Set p2PrevTasks = mkSet(TASK_0_1, TASK_0_3); createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0, TASK_0_2); createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1, TASK_0_3); createClientWithPreviousActiveTasks(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(followupRebalanceNeeded, is(false)); final Set p3ActiveTasks = clients.get(UUID_3).activeTasks(); assertThat(p3ActiveTasks.size(), equalTo(1)); @@ -543,8 +543,8 @@ public void shouldNotMoveAnyTasksWhenNewTasksAdded() { createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0, TASK_0_1); createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_2, TASK_0_3); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks(), hasItems(TASK_0_0, TASK_0_1)); assertThat(clients.get(UUID_2).activeTasks(), hasItems(TASK_0_2, TASK_0_3)); @@ -557,8 +557,8 @@ public void shouldAssignNewTasksToNewClientWhenPreviousTasksAssignedToOldClients createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_0, TASK_0_3); createClient(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); + assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks(), hasItems(TASK_0_2, TASK_0_1)); assertThat(clients.get(UUID_2).activeTasks(), hasItems(TASK_0_0, TASK_0_3)); @@ -568,51 +568,51 @@ public void shouldAssignNewTasksToNewClientWhenPreviousTasksAssignedToOldClients @Test public void shouldAssignTasksNotPreviouslyActiveToNewClient() { final ClientState c1 = createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_1, TASK_1_2, TASK_1_3); - c1.addPreviousStandbyTasks(Utils.mkSet(TASK_0_0, TASK_1_1, TASK_2_0, TASK_2_1, TASK_2_3)); + c1.addPreviousStandbyTasks(mkSet(TASK_0_0, TASK_1_1, TASK_2_0, TASK_2_1, TASK_2_3)); final ClientState c2 = createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_0, TASK_1_1, TASK_2_2); - c2.addPreviousStandbyTasks(Utils.mkSet(TASK_0_1, TASK_1_0, TASK_0_2, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_3)); + c2.addPreviousStandbyTasks(mkSet(TASK_0_1, TASK_1_0, TASK_0_2, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_3)); final ClientState c3 = createClientWithPreviousActiveTasks(UUID_3, 1, TASK_2_0, TASK_2_1, TASK_2_3); - c3.addPreviousStandbyTasks(Utils.mkSet(TASK_0_2, TASK_1_2)); + c3.addPreviousStandbyTasks(mkSet(TASK_0_2, TASK_1_2)); final ClientState newClient = createClient(UUID_4, 1); - newClient.addPreviousStandbyTasks(Utils.mkSet(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3)); + newClient.addPreviousStandbyTasks(mkSet(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3)); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); + assertThat(followupRebalanceNeeded, is(false)); - assertThat(c1.activeTasks(), equalTo(Utils.mkSet(TASK_0_1, TASK_1_2, TASK_1_3))); - assertThat(c2.activeTasks(), equalTo(Utils.mkSet(TASK_0_0, TASK_1_1, TASK_2_2))); - assertThat(c3.activeTasks(), equalTo(Utils.mkSet(TASK_2_0, TASK_2_1, TASK_2_3))); - assertThat(newClient.activeTasks(), equalTo(Utils.mkSet(TASK_0_2, TASK_0_3, TASK_1_0))); + assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_1, TASK_1_2, TASK_1_3))); + assertThat(c2.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_1_1, TASK_2_2))); + assertThat(c3.activeTasks(), equalTo(mkSet(TASK_2_0, TASK_2_1, TASK_2_3))); + assertThat(newClient.activeTasks(), equalTo(mkSet(TASK_0_2, TASK_0_3, TASK_1_0))); } @Test public void shouldAssignTasksNotPreviouslyActiveToMultipleNewClients() { final ClientState c1 = createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_1, TASK_1_2, TASK_1_3); - c1.addPreviousStandbyTasks(Utils.mkSet(TASK_0_0, TASK_1_1, TASK_2_0, TASK_2_1, TASK_2_3)); + c1.addPreviousStandbyTasks(mkSet(TASK_0_0, TASK_1_1, TASK_2_0, TASK_2_1, TASK_2_3)); final ClientState c2 = createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_0, TASK_1_1, TASK_2_2); - c2.addPreviousStandbyTasks(Utils.mkSet(TASK_0_1, TASK_1_0, TASK_0_2, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_3)); + c2.addPreviousStandbyTasks(mkSet(TASK_0_1, TASK_1_0, TASK_0_2, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_3)); final ClientState bounce1 = createClient(UUID_3, 1); - bounce1.addPreviousStandbyTasks(Utils.mkSet(TASK_2_0, TASK_2_1, TASK_2_3)); + bounce1.addPreviousStandbyTasks(mkSet(TASK_2_0, TASK_2_1, TASK_2_3)); final ClientState bounce2 = createClient(UUID_4, 1); - bounce2.addPreviousStandbyTasks(Utils.mkSet(TASK_0_2, TASK_0_3, TASK_1_0)); + bounce2.addPreviousStandbyTasks(mkSet(TASK_0_2, TASK_0_3, TASK_1_0)); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); + assertThat(followupRebalanceNeeded, is(false)); - assertThat(c1.activeTasks(), equalTo(Utils.mkSet(TASK_0_1, TASK_1_2, TASK_1_3))); - assertThat(c2.activeTasks(), equalTo(Utils.mkSet(TASK_0_0, TASK_1_1, TASK_2_2))); - assertThat(bounce1.activeTasks(), equalTo(Utils.mkSet(TASK_2_0, TASK_2_1, TASK_2_3))); - assertThat(bounce2.activeTasks(), equalTo(Utils.mkSet(TASK_0_2, TASK_0_3, TASK_1_0))); + assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_1, TASK_1_2, TASK_1_3))); + assertThat(c2.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_1_1, TASK_2_2))); + assertThat(bounce1.activeTasks(), equalTo(mkSet(TASK_2_0, TASK_2_1, TASK_2_3))); + assertThat(bounce2.activeTasks(), equalTo(mkSet(TASK_0_2, TASK_0_3, TASK_1_0))); } @Test public void shouldAssignTasksToNewClient() { createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_1, TASK_0_2); createClient(UUID_2, 1); - createTaskAssignor(TASK_0_1, TASK_0_2).assign(); + assertThat(assign(TASK_0_1, TASK_0_2), is(false)); assertThat(clients.get(UUID_1).activeTaskCount(), equalTo(1)); } @@ -622,8 +622,8 @@ public void shouldAssignTasksToNewClientWithoutFlippingAssignmentBetweenExisting final ClientState c2 = createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_3, TASK_0_4, TASK_0_5); final ClientState newClient = createClient(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5); + assertThat(followupRebalanceNeeded, is(false)); assertThat(c1.activeTasks(), not(hasItem(TASK_0_3))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_4))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_5))); @@ -639,11 +639,11 @@ public void shouldAssignTasksToNewClientWithoutFlippingAssignmentBetweenExisting public void shouldAssignTasksToNewClientWithoutFlippingAssignmentBetweenExistingAndBouncedClients() { final ClientState c1 = createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_6); final ClientState c2 = createClient(UUID_2, 1); - c2.addPreviousStandbyTasks(Utils.mkSet(TASK_0_3, TASK_0_4, TASK_0_5)); + c2.addPreviousStandbyTasks(mkSet(TASK_0_3, TASK_0_4, TASK_0_5)); final ClientState newClient = createClient(UUID_3, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5, TASK_0_6); - taskAssignor.assign(); + final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5, TASK_0_6); + assertThat(followupRebalanceNeeded, is(false)); assertThat(c1.activeTasks(), not(hasItem(TASK_0_3))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_4))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_5))); @@ -660,32 +660,32 @@ public void shouldViolateBalanceToPreserveActiveTaskStickiness() { final ClientState c1 = createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0, TASK_0_1, TASK_0_2); final ClientState c2 = createClient(UUID_2, 1); - final StickyTaskAssignor taskAssignor = createTaskAssignor(0, true, TASK_0_0, TASK_0_1, TASK_0_2); - taskAssignor.assign(); + final List taskIds = asList(TASK_0_0, TASK_0_1, TASK_0_2); + Collections.shuffle(taskIds); + final boolean followupRebalanceNeeded = new StickyTaskAssignor(true).assign( + clients, + new HashSet<>(taskIds), + new HashSet<>(taskIds), + new AssignorConfiguration.AssignmentConfigs(0L, 0, 0, 0, 0L) + ); + assertThat(followupRebalanceNeeded, is(false)); - assertThat(c1.activeTasks(), equalTo(Utils.mkSet(TASK_0_0, TASK_0_1, TASK_0_2))); - assertTrue(c2.activeTasks().isEmpty()); + assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2))); + assertThat(c2.activeTasks(), empty()); } - private StickyTaskAssignor createTaskAssignor(final TaskId... tasks) { - return createTaskAssignor(0, false, tasks); - } - - private StickyTaskAssignor createTaskAssignor(final int numStandbys, final TaskId... tasks) { - return createTaskAssignor(numStandbys, false, tasks); + private boolean assign(final TaskId... tasks) { + return assign(0, tasks); } - private StickyTaskAssignor createTaskAssignor(final int numStandbys, - final boolean mustPreserveActiveTaskAssignment, - final TaskId... tasks) { + private boolean assign(final int numStandbys, final TaskId... tasks) { final List taskIds = asList(tasks); Collections.shuffle(taskIds); - return new StickyTaskAssignor( + return new StickyTaskAssignor().assign( clients, new HashSet<>(taskIds), new HashSet<>(taskIds), - new AssignmentConfigs(0L, 0, 0, numStandbys, 0L), - mustPreserveActiveTaskAssignment + new AssignorConfiguration.AssignmentConfigs(0L, 0, 0, numStandbys, 0L) ); } @@ -713,7 +713,7 @@ private ClientState createClient(final UUID processId, final int capacity) { private ClientState createClientWithPreviousActiveTasks(final UUID processId, final int capacity, final TaskId... taskIds) { final ClientState clientState = new ClientState(capacity); - clientState.addPreviousActiveTasks(Utils.mkSet(taskIds)); + clientState.addPreviousActiveTasks(mkSet(taskIds)); clients.put(processId, clientState); return clientState; } @@ -730,7 +730,7 @@ private void assertActiveTaskTopicGroupIdsEvenlyDistributed() { } } - private Map> sortClientAssignments(final Map clients) { + private static Map> sortClientAssignments(final Map clients) { final Map> sortedAssignments = new HashMap<>(); for (final Map.Entry entry : clients.entrySet()) { final Set sorted = new TreeSet<>(entry.getValue().activeTasks()); @@ -739,12 +739,11 @@ private Map> sortClientAssignments(final Map getExpectedTaskIdAssignment(final List tasks, final int... indices) { + private static Set getExpectedTaskIdAssignment(final List tasks, final int... indices) { final Set sortedAssignment = new TreeSet<>(); for (final int index : indices) { sortedAssignment.add(tasks.get(index)); } return sortedAssignment; } - } diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java index 7be6ee7719ec2..0944e6d194549 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java @@ -416,11 +416,10 @@ private static void testForConvergence(final Harness harness, iteration++; harness.prepareForNextRebalance(); harness.recordBefore(iteration); - rebalancePending = new HighAvailabilityTaskAssignor( - harness.clientStates, allTasks, - harness.statefulTaskEndOffsetSums.keySet(), - configs - ).assign(); + rebalancePending = new HighAvailabilityTaskAssignor().assign(harness.clientStates, + allTasks, + harness.statefulTaskEndOffsetSums.keySet(), + configs); harness.recordAfter(iteration, rebalancePending); } From fed2506de71a1a661b7ddaa07592dd8599e868e6 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Thu, 23 Apr 2020 21:41:05 -0500 Subject: [PATCH 02/12] restore LagFetchIntegrationTest --- .../integration/LagFetchIntegrationTest.java | 354 ++++++++++++++++++ 1 file changed, 354 insertions(+) create mode 100644 streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java diff --git a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java new file mode 100644 index 0000000000000..1a3f89e1f60d3 --- /dev/null +++ b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java @@ -0,0 +1,354 @@ +/* + * 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.integration; + +import static org.apache.kafka.common.utils.Utils.mkSet; +import static org.apache.kafka.streams.integration.utils.IntegrationTestUtils.startApplicationAndWaitUntilRunning; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import kafka.utils.MockTime; +import org.apache.kafka.clients.consumer.ConsumerConfig; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.serialization.LongDeserializer; +import org.apache.kafka.common.serialization.LongSerializer; +import org.apache.kafka.common.serialization.Serdes; +import org.apache.kafka.common.serialization.StringDeserializer; +import org.apache.kafka.common.serialization.StringSerializer; +import org.apache.kafka.streams.KafkaStreams; +import org.apache.kafka.streams.KafkaStreamsWrapper; +import org.apache.kafka.streams.KeyValue; +import org.apache.kafka.streams.LagInfo; +import org.apache.kafka.streams.StreamsBuilder; +import org.apache.kafka.streams.StreamsConfig; +import org.apache.kafka.streams.integration.utils.EmbeddedKafkaCluster; +import org.apache.kafka.streams.integration.utils.IntegrationTestUtils; +import org.apache.kafka.streams.kstream.KTable; +import org.apache.kafka.streams.kstream.Materialized; +import org.apache.kafka.streams.processor.StateRestoreListener; +import org.apache.kafka.streams.processor.internals.StreamThread; +import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration; +import org.apache.kafka.streams.processor.internals.assignment.PriorTaskAssignor; +import org.apache.kafka.test.IntegrationTest; +import org.apache.kafka.test.TestUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Category({IntegrationTest.class}) +public class LagFetchIntegrationTest { + + @ClassRule + public static final EmbeddedKafkaCluster CLUSTER = new EmbeddedKafkaCluster(1); + + private static final long WAIT_TIMEOUT_MS = 120000; + private static final Logger LOG = LoggerFactory.getLogger(LagFetchIntegrationTest.class); + + private final MockTime mockTime = CLUSTER.time; + private Properties streamsConfiguration; + private Properties consumerConfiguration; + private String inputTopicName; + private String outputTopicName; + private String stateStoreName; + + @Rule + public TestName name = new TestName(); + + @Before + public void before() { + inputTopicName = "input-topic-" + name.getMethodName(); + outputTopicName = "output-topic-" + name.getMethodName(); + stateStoreName = "lagfetch-test-store" + name.getMethodName(); + + streamsConfiguration = new Properties(); + streamsConfiguration.put(StreamsConfig.APPLICATION_ID_CONFIG, "lag-fetch-" + name.getMethodName()); + streamsConfiguration.put(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG, CLUSTER.bootstrapServers()); + streamsConfiguration.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); + streamsConfiguration.put(StreamsConfig.DEFAULT_KEY_SERDE_CLASS_CONFIG, Serdes.String().getClass()); + streamsConfiguration.put(StreamsConfig.DEFAULT_VALUE_SERDE_CLASS_CONFIG, Serdes.String().getClass()); + streamsConfiguration.put(StreamsConfig.COMMIT_INTERVAL_MS_CONFIG, 100); + + consumerConfiguration = new Properties(); + consumerConfiguration.setProperty(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, CLUSTER.bootstrapServers()); + consumerConfiguration.setProperty(ConsumerConfig.GROUP_ID_CONFIG, name.getMethodName() + "-consumer"); + consumerConfiguration.setProperty(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest"); + consumerConfiguration.setProperty(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, StringDeserializer.class.getName()); + consumerConfiguration.setProperty(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, LongDeserializer.class.getName()); + } + + @After + public void shutdown() throws Exception { + IntegrationTestUtils.purgeLocalStreamsState(streamsConfiguration); + } + + private Map> getFirstNonEmptyLagMap(final KafkaStreams streams) throws InterruptedException { + final Map> offsetLagInfoMap = new HashMap<>(); + TestUtils.waitForCondition(() -> { + final Map> lagMap = streams.allLocalStorePartitionLags(); + if (lagMap.size() > 0) { + offsetLagInfoMap.putAll(lagMap); + } + return lagMap.size() > 0; + }, WAIT_TIMEOUT_MS, "Should obtain non-empty lag information eventually"); + return offsetLagInfoMap; + } + + private void shouldFetchLagsDuringRebalancing(final String optimization) throws Exception { + final CountDownLatch latchTillActiveIsRunning = new CountDownLatch(1); + final CountDownLatch latchTillStandbyIsRunning = new CountDownLatch(1); + final CountDownLatch latchTillStandbyHasPartitionsAssigned = new CountDownLatch(1); + final CyclicBarrier lagCheckBarrier = new CyclicBarrier(2); + final List streamsList = new ArrayList<>(); + + IntegrationTestUtils.produceKeyValuesSynchronously( + inputTopicName, + mkSet(new KeyValue<>("k1", 1L), new KeyValue<>("k2", 2L), new KeyValue<>("k3", 3L), new KeyValue<>("k4", 4L), new KeyValue<>("k5", 5L)), + TestUtils.producerConfig( + CLUSTER.bootstrapServers(), + StringSerializer.class, + LongSerializer.class, + new Properties()), + mockTime); + + // create stream threads + for (int i = 0; i < 2; i++) { + final Properties props = (Properties) streamsConfiguration.clone(); + // this test relies on the second instance getting the standby, so we specify + // an assignor with this contract. + props.put(AssignorConfiguration.INTERNAL_TASK_ASSIGNOR_CLASS, PriorTaskAssignor.class.getName()); + props.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "localhost:" + i); + props.put(StreamsConfig.CLIENT_ID_CONFIG, "instance-" + i); + props.put(StreamsConfig.TOPOLOGY_OPTIMIZATION, optimization); + props.put(StreamsConfig.NUM_STANDBY_REPLICAS_CONFIG, 1); + props.put(StreamsConfig.STATE_DIR_CONFIG, TestUtils.tempDirectory(stateStoreName + i).getAbsolutePath()); + + final StreamsBuilder builder = new StreamsBuilder(); + final KTable t1 = builder.table(inputTopicName, Materialized.as(stateStoreName)); + t1.toStream().to(outputTopicName); + final KafkaStreamsWrapper streams = new KafkaStreamsWrapper(builder.build(props), props); + streamsList.add(streams); + } + + final KafkaStreamsWrapper activeStreams = streamsList.get(0); + final KafkaStreamsWrapper standbyStreams = streamsList.get(1); + activeStreams.setStreamThreadStateListener((thread, newState, oldState) -> { + if (newState == StreamThread.State.RUNNING) { + latchTillActiveIsRunning.countDown(); + } + }); + standbyStreams.setStreamThreadStateListener((thread, newState, oldState) -> { + if (oldState == StreamThread.State.PARTITIONS_ASSIGNED && newState == StreamThread.State.RUNNING) { + latchTillStandbyHasPartitionsAssigned.countDown(); + try { + lagCheckBarrier.await(60, TimeUnit.SECONDS); + } catch (final Exception e) { + throw new RuntimeException(e); + } + } else if (newState == StreamThread.State.RUNNING) { + latchTillStandbyIsRunning.countDown(); + } + }); + + try { + // First start up the active. + TestUtils.waitForCondition(() -> activeStreams.allLocalStorePartitionLags().size() == 0, + WAIT_TIMEOUT_MS, + "Should see empty lag map before streams is started."); + activeStreams.start(); + latchTillActiveIsRunning.await(60, TimeUnit.SECONDS); + + IntegrationTestUtils.waitUntilMinValuesRecordsReceived( + consumerConfiguration, + outputTopicName, + 5, + WAIT_TIMEOUT_MS); + // Check the active reports proper lag values. + Map> offsetLagInfoMap = getFirstNonEmptyLagMap(activeStreams); + assertThat(offsetLagInfoMap.size(), equalTo(1)); + assertThat(offsetLagInfoMap.keySet(), equalTo(mkSet(stateStoreName))); + assertThat(offsetLagInfoMap.get(stateStoreName).size(), equalTo(1)); + LagInfo lagInfo = offsetLagInfoMap.get(stateStoreName).get(0); + assertThat(lagInfo.currentOffsetPosition(), equalTo(5L)); + assertThat(lagInfo.endOffsetPosition(), equalTo(5L)); + assertThat(lagInfo.offsetLag(), equalTo(0L)); + + // start up the standby & make it pause right after it has partition assigned + standbyStreams.start(); + latchTillStandbyHasPartitionsAssigned.await(60, TimeUnit.SECONDS); + offsetLagInfoMap = getFirstNonEmptyLagMap(standbyStreams); + assertThat(offsetLagInfoMap.size(), equalTo(1)); + assertThat(offsetLagInfoMap.keySet(), equalTo(mkSet(stateStoreName))); + assertThat(offsetLagInfoMap.get(stateStoreName).size(), equalTo(1)); + lagInfo = offsetLagInfoMap.get(stateStoreName).get(0); + assertThat(lagInfo.currentOffsetPosition(), equalTo(0L)); + assertThat(lagInfo.endOffsetPosition(), equalTo(5L)); + assertThat(lagInfo.offsetLag(), equalTo(5L)); + // standby thread wont proceed to RUNNING before this barrier is crossed + lagCheckBarrier.await(60, TimeUnit.SECONDS); + + // wait till the lag goes down to 0, on the standby + TestUtils.waitForCondition(() -> standbyStreams.allLocalStorePartitionLags().get(stateStoreName).get(0).offsetLag() == 0, + WAIT_TIMEOUT_MS, + "Standby should eventually catchup and have zero lag."); + } finally { + for (final KafkaStreams streams : streamsList) { + streams.close(); + } + } + } + + @Test + public void shouldFetchLagsDuringRebalancingWithOptimization() throws Exception { + shouldFetchLagsDuringRebalancing(StreamsConfig.OPTIMIZE); + } + + @Test + public void shouldFetchLagsDuringRebalancingWithNoOptimization() throws Exception { + shouldFetchLagsDuringRebalancing(StreamsConfig.NO_OPTIMIZATION); + } + + @Test + public void shouldFetchLagsDuringRestoration() throws Exception { + IntegrationTestUtils.produceKeyValuesSynchronously( + inputTopicName, + mkSet(new KeyValue<>("k1", 1L), new KeyValue<>("k2", 2L), new KeyValue<>("k3", 3L), new KeyValue<>("k4", 4L), new KeyValue<>("k5", 5L)), + TestUtils.producerConfig( + CLUSTER.bootstrapServers(), + StringSerializer.class, + LongSerializer.class, + new Properties()), + mockTime); + + // create stream threads + final Properties props = (Properties) streamsConfiguration.clone(); + final File stateDir = TestUtils.tempDirectory(stateStoreName + "0"); + props.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "localhost:0"); + props.put(StreamsConfig.CLIENT_ID_CONFIG, "instance-0"); + props.put(StreamsConfig.STATE_DIR_CONFIG, stateDir.getAbsolutePath()); + + final StreamsBuilder builder = new StreamsBuilder(); + final KTable t1 = builder.table(inputTopicName, Materialized.as(stateStoreName)); + t1.toStream().to(outputTopicName); + final KafkaStreams streams = new KafkaStreams(builder.build(), props); + + try { + // First start up the active. + TestUtils.waitForCondition(() -> streams.allLocalStorePartitionLags().size() == 0, + WAIT_TIMEOUT_MS, + "Should see empty lag map before streams is started."); + + // Get the instance to fully catch up and reach RUNNING state + startApplicationAndWaitUntilRunning(Collections.singletonList(streams), Duration.ofSeconds(60)); + IntegrationTestUtils.waitUntilMinValuesRecordsReceived( + consumerConfiguration, + outputTopicName, + 5, + WAIT_TIMEOUT_MS); + + // check for proper lag values. + final AtomicReference zeroLagRef = new AtomicReference<>(); + TestUtils.waitForCondition(() -> { + final Map> offsetLagInfoMap = streams.allLocalStorePartitionLags(); + assertThat(offsetLagInfoMap.size(), equalTo(1)); + assertThat(offsetLagInfoMap.keySet(), equalTo(mkSet(stateStoreName))); + assertThat(offsetLagInfoMap.get(stateStoreName).size(), equalTo(1)); + + final LagInfo zeroLagInfo = offsetLagInfoMap.get(stateStoreName).get(0); + assertThat(zeroLagInfo.currentOffsetPosition(), equalTo(5L)); + assertThat(zeroLagInfo.endOffsetPosition(), equalTo(5L)); + assertThat(zeroLagInfo.offsetLag(), equalTo(0L)); + zeroLagRef.set(zeroLagInfo); + return true; + }, WAIT_TIMEOUT_MS, "Eventually should reach zero lag."); + + // Kill instance, delete state to force restoration. + assertThat("Streams instance did not close within timeout", streams.close(Duration.ofSeconds(60))); + IntegrationTestUtils.purgeLocalStreamsState(streamsConfiguration); + Files.walk(stateDir.toPath()).sorted(Comparator.reverseOrder()) + .map(Path::toFile) + .forEach(f -> assertTrue("Some state " + f + " could not be deleted", f.delete())); + + // wait till the lag goes down to 0 + final KafkaStreams restartedStreams = new KafkaStreams(builder.build(), props); + // set a state restoration listener to track progress of restoration + final CountDownLatch restorationEndLatch = new CountDownLatch(1); + final Map> restoreStartLagInfo = new HashMap<>(); + final Map> restoreEndLagInfo = new HashMap<>(); + restartedStreams.setGlobalStateRestoreListener(new StateRestoreListener() { + @Override + public void onRestoreStart(final TopicPartition topicPartition, final String storeName, final long startingOffset, final long endingOffset) { + try { + restoreStartLagInfo.putAll(getFirstNonEmptyLagMap(restartedStreams)); + } catch (final Exception e) { + LOG.error("Exception while trying to obtain lag map", e); + } + } + + @Override + public void onBatchRestored(final TopicPartition topicPartition, final String storeName, final long batchEndOffset, final long numRestored) { + } + + @Override + public void onRestoreEnd(final TopicPartition topicPartition, final String storeName, final long totalRestored) { + try { + restoreEndLagInfo.putAll(getFirstNonEmptyLagMap(restartedStreams)); + } catch (final Exception e) { + LOG.error("Exception while trying to obtain lag map", e); + } + restorationEndLatch.countDown(); + } + }); + + restartedStreams.start(); + restorationEndLatch.await(WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + TestUtils.waitForCondition(() -> restartedStreams.allLocalStorePartitionLags().get(stateStoreName).get(0).offsetLag() == 0, + WAIT_TIMEOUT_MS, + "Standby should eventually catchup and have zero lag."); + final LagInfo fullLagInfo = restoreStartLagInfo.get(stateStoreName).get(0); + assertThat(fullLagInfo.currentOffsetPosition(), equalTo(0L)); + assertThat(fullLagInfo.endOffsetPosition(), equalTo(5L)); + assertThat(fullLagInfo.offsetLag(), equalTo(5L)); + + assertThat(restoreEndLagInfo.get(stateStoreName).get(0), equalTo(zeroLagRef.get())); + } finally { + streams.close(); + streams.cleanUp(); + } + } +} From b7373de4dd29699868a6c7317b8e6ec71d2220ae Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 24 Apr 2020 10:25:47 -0500 Subject: [PATCH 03/12] cr feedback --- .../apache/kafka/streams/StreamsConfig.java | 5 +++ .../assignment/AssignorConfiguration.java | 4 +- .../integration/LagFetchIntegrationTest.java | 2 +- ...ilabilityStreamsPartitionAssignorTest.java | 2 +- .../StreamsPartitionAssignorTest.java | 42 +++++++++---------- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java b/streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java index c441deed69396..1df68390bb10b 100644 --- a/streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java +++ b/streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java @@ -868,6 +868,11 @@ public class StreamsConfig extends AbstractConfig { } public static class InternalConfig { + // This is settable in the main Streams config, but it's a private API for now + public static final String INTERNAL_TASK_ASSIGNOR_CLASS = "internal.task.assignor.class"; + + // These are not settable in the main Streams config; they are set by the StreamThread to pass internal + // state into the assignor. public static final String TASK_MANAGER_FOR_PARTITION_ASSIGNOR = "__task.manager.instance__"; public static final String STREAMS_METADATA_STATE_FOR_PARTITION_ASSIGNOR = "__streams.metadata.state.instance__"; public static final String STREAMS_ADMIN_CLIENT = "__streams.admin.client.instance__"; diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java index 3a4b2c9578fce..2a5d1d499e31d 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java @@ -16,7 +16,6 @@ */ package org.apache.kafka.streams.processor.internals.assignment; -import java.util.concurrent.atomic.AtomicLong; import org.apache.kafka.clients.CommonClientConfigs; import org.apache.kafka.clients.admin.Admin; import org.apache.kafka.clients.admin.AdminClientConfig; @@ -36,13 +35,14 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import static org.apache.kafka.common.utils.Utils.getHost; import static org.apache.kafka.common.utils.Utils.getPort; +import static org.apache.kafka.streams.StreamsConfig.InternalConfig.INTERNAL_TASK_ASSIGNOR_CLASS; import static org.apache.kafka.streams.processor.internals.assignment.StreamsAssignmentProtocolVersions.LATEST_SUPPORTED_VERSION; public final class AssignorConfiguration { - public static final String INTERNAL_TASK_ASSIGNOR_CLASS = "internal.task.assignor.class"; private final String taskAssignorClass; private final String logPrefix; diff --git a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java index 1a3f89e1f60d3..7ca6a8903eb86 100644 --- a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java @@ -151,7 +151,7 @@ private void shouldFetchLagsDuringRebalancing(final String optimization) throws final Properties props = (Properties) streamsConfiguration.clone(); // this test relies on the second instance getting the standby, so we specify // an assignor with this contract. - props.put(AssignorConfiguration.INTERNAL_TASK_ASSIGNOR_CLASS, PriorTaskAssignor.class.getName()); + props.put(StreamsConfig.InternalConfig.INTERNAL_TASK_ASSIGNOR_CLASS, PriorTaskAssignor.class.getName()); props.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "localhost:" + i); props.put(StreamsConfig.CLIENT_ID_CONFIG, "instance-" + i); props.put(StreamsConfig.TOPOLOGY_OPTIMIZATION, optimization); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java index 3f47b4868251e..4f497d02dd9f2 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java @@ -125,7 +125,7 @@ private Map configProps() { configurationMap.put(InternalConfig.ASSIGNMENT_ERROR_CODE, assignmentError); configurationMap.put(InternalConfig.NEXT_PROBING_REBALANCE_MS, nextProbingRebalanceMs); configurationMap.put(InternalConfig.TIME, time); - configurationMap.put(AssignorConfiguration.INTERNAL_TASK_ASSIGNOR_CLASS, HighAvailabilityTaskAssignor.class.getName()); + configurationMap.put(InternalConfig.INTERNAL_TASK_ASSIGNOR_CLASS, HighAvailabilityTaskAssignor.class.getName()); return configurationMap; } diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java index e2adf14a616db..3ce208f23317d 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java @@ -16,8 +16,6 @@ */ package org.apache.kafka.streams.processor.internals; -import java.util.Map.Entry; -import java.util.concurrent.atomic.AtomicLong; import org.apache.kafka.clients.admin.Admin; import org.apache.kafka.clients.admin.AdminClient; import org.apache.kafka.clients.admin.AdminClientConfig; @@ -64,6 +62,8 @@ import org.easymock.Capture; import org.easymock.EasyMock; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -73,12 +73,12 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; import static java.time.Duration.ofMillis; import static java.util.Arrays.asList; @@ -89,13 +89,8 @@ import static org.apache.kafka.common.utils.Utils.mkEntry; import static org.apache.kafka.common.utils.Utils.mkMap; import static org.apache.kafka.common.utils.Utils.mkSet; -import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.EMPTY_TASKS; -import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_2_1; -import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_2_2; -import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_2_3; -import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_1; -import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_2; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.EMPTY_CHANGELOG_END_OFFSETS; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.EMPTY_TASKS; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.EMPTY_TASK_OFFSET_SUMS; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_0; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_1; @@ -106,13 +101,18 @@ import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_1_2; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_1_3; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_2_0; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_2_1; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_2_2; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_2_3; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_1; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.UUID_2; import static org.apache.kafka.streams.processor.internals.assignment.StreamsAssignmentProtocolVersions.LATEST_SUPPORTED_VERSION; import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.expect; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.Matchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -207,13 +207,13 @@ private Map configProps() { final Map configurationMap = new HashMap<>(); configurationMap.put(StreamsConfig.APPLICATION_ID_CONFIG, APPLICATION_ID); configurationMap.put(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG, USER_END_POINT); - configurationMap.put(StreamsConfig.InternalConfig.TASK_MANAGER_FOR_PARTITION_ASSIGNOR, taskManager); - configurationMap.put(StreamsConfig.InternalConfig.STREAMS_METADATA_STATE_FOR_PARTITION_ASSIGNOR, streamsMetadataState); - configurationMap.put(StreamsConfig.InternalConfig.STREAMS_ADMIN_CLIENT, adminClient); - configurationMap.put(StreamsConfig.InternalConfig.ASSIGNMENT_ERROR_CODE, assignmentError); - configurationMap.put(StreamsConfig.InternalConfig.NEXT_PROBING_REBALANCE_MS, nextProbingRebalanceMs); + configurationMap.put(InternalConfig.TASK_MANAGER_FOR_PARTITION_ASSIGNOR, taskManager); + configurationMap.put(InternalConfig.STREAMS_METADATA_STATE_FOR_PARTITION_ASSIGNOR, streamsMetadataState); + configurationMap.put(InternalConfig.STREAMS_ADMIN_CLIENT, adminClient); + configurationMap.put(InternalConfig.ASSIGNMENT_ERROR_CODE, assignmentError); + configurationMap.put(InternalConfig.NEXT_PROBING_REBALANCE_MS, nextProbingRebalanceMs); configurationMap.put(InternalConfig.TIME, time); - configurationMap.put(AssignorConfiguration.INTERNAL_TASK_ASSIGNOR_CLASS, taskAssignor.getName()); + configurationMap.put(InternalConfig.INTERNAL_TASK_ASSIGNOR_CLASS, taskAssignor.getName()); return configurationMap; } @@ -1435,7 +1435,7 @@ public void shouldNotAddStandbyTaskPartitionsToPartitionsForHost() { @Test public void shouldThrowKafkaExceptionIfTaskMangerNotConfigured() { final Map config = configProps(); - config.remove(StreamsConfig.InternalConfig.TASK_MANAGER_FOR_PARTITION_ASSIGNOR); + config.remove(InternalConfig.TASK_MANAGER_FOR_PARTITION_ASSIGNOR); try { partitionAssignor.configure(config); @@ -1448,7 +1448,7 @@ public void shouldThrowKafkaExceptionIfTaskMangerNotConfigured() { @Test public void shouldThrowKafkaExceptionIfTaskMangerConfigIsNotTaskManagerInstance() { final Map config = configProps(); - config.put(StreamsConfig.InternalConfig.TASK_MANAGER_FOR_PARTITION_ASSIGNOR, "i am not a task manager"); + config.put(InternalConfig.TASK_MANAGER_FOR_PARTITION_ASSIGNOR, "i am not a task manager"); try { partitionAssignor.configure(config); @@ -1463,7 +1463,7 @@ public void shouldThrowKafkaExceptionIfTaskMangerConfigIsNotTaskManagerInstance( public void shouldThrowKafkaExceptionAssignmentErrorCodeNotConfigured() { createDefaultMockTaskManager(); final Map config = configProps(); - config.remove(StreamsConfig.InternalConfig.ASSIGNMENT_ERROR_CODE); + config.remove(InternalConfig.ASSIGNMENT_ERROR_CODE); try { partitionAssignor.configure(config); @@ -1477,7 +1477,7 @@ public void shouldThrowKafkaExceptionAssignmentErrorCodeNotConfigured() { public void shouldThrowKafkaExceptionIfVersionProbingFlagConfigIsNotAtomicInteger() { createDefaultMockTaskManager(); final Map config = configProps(); - config.put(StreamsConfig.InternalConfig.ASSIGNMENT_ERROR_CODE, "i am not an AtomicInteger"); + config.put(InternalConfig.ASSIGNMENT_ERROR_CODE, "i am not an AtomicInteger"); try { partitionAssignor.configure(config); From deefb582a3a8c3f3c15d1a403b064dc77fa5dc1b Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 24 Apr 2020 10:39:49 -0500 Subject: [PATCH 04/12] cr comments and fixing system tests in progress --- .../integration/LagFetchIntegrationTest.java | 44 +++++++++---------- ...ilabilityStreamsPartitionAssignorTest.java | 1 - tests/kafkatest/services/streams.py | 2 + .../streams_broker_down_resilience_test.py | 12 ++++- .../streams/streams_standby_replica_test.py | 11 +++-- 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java index 7ca6a8903eb86..b9a578e2a6ab9 100644 --- a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java @@ -16,27 +16,6 @@ */ package org.apache.kafka.streams.integration; -import static org.apache.kafka.common.utils.Utils.mkSet; -import static org.apache.kafka.streams.integration.utils.IntegrationTestUtils.startApplicationAndWaitUntilRunning; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.IsEqual.equalTo; -import static org.junit.Assert.assertTrue; - -import java.io.File; -import java.nio.file.Files; -import java.nio.file.Path; -import java.time.Duration; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.CyclicBarrier; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; import kafka.utils.MockTime; import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.common.TopicPartition; @@ -57,7 +36,6 @@ import org.apache.kafka.streams.kstream.Materialized; import org.apache.kafka.streams.processor.StateRestoreListener; import org.apache.kafka.streams.processor.internals.StreamThread; -import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration; import org.apache.kafka.streams.processor.internals.assignment.PriorTaskAssignor; import org.apache.kafka.test.IntegrationTest; import org.apache.kafka.test.TestUtils; @@ -71,6 +49,28 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static org.apache.kafka.common.utils.Utils.mkSet; +import static org.apache.kafka.streams.integration.utils.IntegrationTestUtils.startApplicationAndWaitUntilRunning; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.Assert.assertTrue; + @Category({IntegrationTest.class}) public class LagFetchIntegrationTest { diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java index 4f497d02dd9f2..dd34134359930 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java @@ -34,7 +34,6 @@ import org.apache.kafka.streams.errors.StreamsException; import org.apache.kafka.streams.processor.TaskId; import org.apache.kafka.streams.processor.internals.assignment.AssignmentInfo; -import org.apache.kafka.streams.processor.internals.assignment.AssignorConfiguration; import org.apache.kafka.streams.processor.internals.assignment.AssignorError; import org.apache.kafka.streams.processor.internals.assignment.HighAvailabilityTaskAssignor; import org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo; diff --git a/tests/kafkatest/services/streams.py b/tests/kafkatest/services/streams.py index e8788829e68bd..dd81dc0f282ea 100644 --- a/tests/kafkatest/services/streams.py +++ b/tests/kafkatest/services/streams.py @@ -562,6 +562,8 @@ def prop_file(self): consumer_property.SESSION_TIMEOUT_MS: 60000} properties['input.topic'] = self.INPUT_TOPIC + # TODO KIP-441: consider rewriting the test for HighAvailabilityTaskAssignor + properties['internal.task.assignor.class'] = "org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor" cfg = KafkaConfig(**properties) return cfg.render() diff --git a/tests/kafkatest/tests/streams/streams_broker_down_resilience_test.py b/tests/kafkatest/tests/streams/streams_broker_down_resilience_test.py index 58f3b1832b6d3..8fcf14a3fcc6d 100644 --- a/tests/kafkatest/tests/streams/streams_broker_down_resilience_test.py +++ b/tests/kafkatest/tests/streams/streams_broker_down_resilience_test.py @@ -144,7 +144,11 @@ def test_streams_runs_with_broker_down_initially(self): def test_streams_should_scale_in_while_brokers_down(self): self.kafka.start() - configs = self.get_configs(extra_configs=",application.id=shutdown_with_broker_down") + # TODO KIP-441: consider rewriting the test for HighAvailabilityTaskAssignor + configs = self.get_configs( + extra_configs=",application.id=shutdown_with_broker_down" + + ",internal.task.assignor.class=org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor" + ) processor = StreamsBrokerDownResilienceService(self.test_context, self.kafka, configs) processor.start() @@ -217,7 +221,11 @@ def test_streams_should_scale_in_while_brokers_down(self): def test_streams_should_failover_while_brokers_down(self): self.kafka.start() - configs = self.get_configs(extra_configs=",application.id=failover_with_broker_down") + # TODO KIP-441: consider rewriting the test for HighAvailabilityTaskAssignor + configs = self.get_configs( + extra_configs=",application.id=failover_with_broker_down" + + ",internal.task.assignor.class=org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor" + ) processor = StreamsBrokerDownResilienceService(self.test_context, self.kafka, configs) processor.start() diff --git a/tests/kafkatest/tests/streams/streams_standby_replica_test.py b/tests/kafkatest/tests/streams/streams_standby_replica_test.py index 310f8a5ba2b56..e847c3ebf9d90 100644 --- a/tests/kafkatest/tests/streams/streams_standby_replica_test.py +++ b/tests/kafkatest/tests/streams/streams_standby_replica_test.py @@ -44,9 +44,14 @@ def __init__(self, test_context): }) def test_standby_tasks_rebalance(self): - configs = self.get_configs(",sourceTopic=%s,sinkTopic1=%s,sinkTopic2=%s" % (self.streams_source_topic, - self.streams_sink_topic_1, - self.streams_sink_topic_2)) + # TODO KIP-441: consider rewriting the test for HighAvailabilityTaskAssignor + configs = self.get_configs( + ",sourceTopic=%s,sinkTopic1=%s,sinkTopic2=%s,internal.task.assignor.class=org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor" % ( + self.streams_source_topic, + self.streams_sink_topic_1, + self.streams_sink_topic_2 + ) + ) producer = self.get_producer(self.streams_source_topic, self.num_messages, throughput=15000, repeating_keys=6) producer.start() From 77fbb4193766c72eaf2a8b2ab34c96a4482de14f Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 24 Apr 2020 14:41:10 -0500 Subject: [PATCH 05/12] fix system tests --- tests/kafkatest/services/streams.py | 13 +++++++++---- .../tests/streams/streams_broker_bounce_test.py | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/kafkatest/services/streams.py b/tests/kafkatest/services/streams.py index dd81dc0f282ea..9ccbec8432cbc 100644 --- a/tests/kafkatest/services/streams.py +++ b/tests/kafkatest/services/streams.py @@ -309,12 +309,17 @@ def __init__(self, test_context, kafka, command, processing_guarantee = 'at_leas command) self.NUM_THREADS = num_threads self.PROCESSING_GUARANTEE = processing_guarantee + self.extra_properties = {} + + def set_config(self, key, value): + self.extra_properties[key] = value def prop_file(self): - properties = {streams_property.STATE_DIR: self.PERSISTENT_ROOT, - streams_property.KAFKA_SERVERS: self.kafka.bootstrap_servers(), - streams_property.PROCESSING_GUARANTEE: self.PROCESSING_GUARANTEE, - streams_property.NUM_THREADS: self.NUM_THREADS} + properties = self.extra_properties.copy() + properties[streams_property.STATE_DIR] = self.PERSISTENT_ROOT + properties[streams_property.KAFKA_SERVERS] = self.kafka.bootstrap_servers() + properties[streams_property.PROCESSING_GUARANTEE] = self.PROCESSING_GUARANTEE + properties[streams_property.NUM_THREADS] = self.NUM_THREADS cfg = KafkaConfig(**properties) return cfg.render() diff --git a/tests/kafkatest/tests/streams/streams_broker_bounce_test.py b/tests/kafkatest/tests/streams/streams_broker_bounce_test.py index a20eb02f9ad44..d23cdceb796a2 100644 --- a/tests/kafkatest/tests/streams/streams_broker_bounce_test.py +++ b/tests/kafkatest/tests/streams/streams_broker_bounce_test.py @@ -165,6 +165,8 @@ def setup_system(self, start_processor=True, num_threads=3): # Start test harness self.driver = StreamsSmokeTestDriverService(self.test_context, self.kafka) self.processor1 = StreamsSmokeTestJobRunnerService(self.test_context, self.kafka, num_threads) + # TODO KIP-441: consider rewriting the test for HighAvailabilityTaskAssignor + self.processor1.set_config("internal.task.assignor.class", "org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor") self.driver.start() From 9015be8cb3ed6cbd3865758660d4023da350a702 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 24 Apr 2020 16:25:37 -0500 Subject: [PATCH 06/12] fix tests --- tests/kafkatest/services/streams.py | 10 ++++++++-- tests/kafkatest/tests/streams/streams_upgrade_test.py | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/kafkatest/services/streams.py b/tests/kafkatest/services/streams.py index 9ccbec8432cbc..18065df901808 100644 --- a/tests/kafkatest/services/streams.py +++ b/tests/kafkatest/services/streams.py @@ -482,6 +482,10 @@ def __init__(self, test_context, kafka): "") self.UPGRADE_FROM = None self.UPGRADE_TO = None + self.extra_properties = {} + + def set_config(self, key, value): + self.extra_properties[key] = value def set_version(self, kafka_streams_version): self.KAFKA_STREAMS_VERSION = kafka_streams_version @@ -493,8 +497,10 @@ def set_upgrade_to(self, upgrade_to): self.UPGRADE_TO = upgrade_to def prop_file(self): - properties = {streams_property.STATE_DIR: self.PERSISTENT_ROOT, - streams_property.KAFKA_SERVERS: self.kafka.bootstrap_servers()} + properties = self.extra_properties.copy() + properties[streams_property.STATE_DIR] = self.PERSISTENT_ROOT + properties[streams_property.KAFKA_SERVERS] = self.kafka.bootstrap_servers() + if self.UPGRADE_FROM is not None: properties['upgrade.from'] = self.UPGRADE_FROM if self.UPGRADE_TO == "future_version": diff --git a/tests/kafkatest/tests/streams/streams_upgrade_test.py b/tests/kafkatest/tests/streams/streams_upgrade_test.py index 662f141ce9b51..de665d14dc071 100644 --- a/tests/kafkatest/tests/streams/streams_upgrade_test.py +++ b/tests/kafkatest/tests/streams/streams_upgrade_test.py @@ -303,9 +303,13 @@ def test_version_probing_upgrade(self): self.driver = StreamsSmokeTestDriverService(self.test_context, self.kafka) self.driver.disable_auto_terminate() + # TODO KIP-441: consider rewriting the test for HighAvailabilityTaskAssignor self.processor1 = StreamsUpgradeTestJobRunnerService(self.test_context, self.kafka) + self.processor1.set_config("internal.task.assignor.class", "org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor") self.processor2 = StreamsUpgradeTestJobRunnerService(self.test_context, self.kafka) + self.processor2.set_config("internal.task.assignor.class", "org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor") self.processor3 = StreamsUpgradeTestJobRunnerService(self.test_context, self.kafka) + self.processor3.set_config("internal.task.assignor.class", "org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor") self.driver.start() self.start_all_nodes_with("") # run with TRUNK From 126afd1f2249cb70d7f23c57965d1fdf01a4d957 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Fri, 24 Apr 2020 17:27:58 -0500 Subject: [PATCH 07/12] revert unnecessary change --- tests/kafkatest/services/streams.py | 13 ++++--------- .../tests/streams/streams_broker_bounce_test.py | 4 +--- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/tests/kafkatest/services/streams.py b/tests/kafkatest/services/streams.py index 18065df901808..b5e2feb0c5452 100644 --- a/tests/kafkatest/services/streams.py +++ b/tests/kafkatest/services/streams.py @@ -309,17 +309,12 @@ def __init__(self, test_context, kafka, command, processing_guarantee = 'at_leas command) self.NUM_THREADS = num_threads self.PROCESSING_GUARANTEE = processing_guarantee - self.extra_properties = {} - - def set_config(self, key, value): - self.extra_properties[key] = value def prop_file(self): - properties = self.extra_properties.copy() - properties[streams_property.STATE_DIR] = self.PERSISTENT_ROOT - properties[streams_property.KAFKA_SERVERS] = self.kafka.bootstrap_servers() - properties[streams_property.PROCESSING_GUARANTEE] = self.PROCESSING_GUARANTEE - properties[streams_property.NUM_THREADS] = self.NUM_THREADS + properties = {streams_property.STATE_DIR: self.PERSISTENT_ROOT, + streams_property.KAFKA_SERVERS: self.kafka.bootstrap_servers(), + streams_property.PROCESSING_GUARANTEE: self.PROCESSING_GUARANTEE, + streams_property.NUM_THREADS: self.NUM_THREADS} cfg = KafkaConfig(**properties) return cfg.render() diff --git a/tests/kafkatest/tests/streams/streams_broker_bounce_test.py b/tests/kafkatest/tests/streams/streams_broker_bounce_test.py index d23cdceb796a2..69f72d2a07e3b 100644 --- a/tests/kafkatest/tests/streams/streams_broker_bounce_test.py +++ b/tests/kafkatest/tests/streams/streams_broker_bounce_test.py @@ -164,9 +164,7 @@ def setup_system(self, start_processor=True, num_threads=3): # Start test harness self.driver = StreamsSmokeTestDriverService(self.test_context, self.kafka) - self.processor1 = StreamsSmokeTestJobRunnerService(self.test_context, self.kafka, num_threads) - # TODO KIP-441: consider rewriting the test for HighAvailabilityTaskAssignor - self.processor1.set_config("internal.task.assignor.class", "org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor") + self.processor1 = StreamsSmokeTestJobRunnerService(self.test_context, self.kafka, "at_least_once", num_threads) self.driver.start() From 98e317e93a9b961f32a8c6187dc2324748ab0749 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Mon, 27 Apr 2020 16:26:19 -0500 Subject: [PATCH 08/12] Update streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java Co-Authored-By: Bruno Cadonna --- .../assignment/StickyTaskAssignorTest.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java index 3e4730c03173b..f6a7d14d0c0b7 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java @@ -350,18 +350,20 @@ public void shouldAssignMoreTasksToClientWithMoreCapacity() { createClient(UUID_2, 2); createClient(UUID_1, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, - TASK_0_1, - TASK_0_2, - new TaskId(1, 0), - new TaskId(1, 1), - new TaskId(1, 2), - new TaskId(2, 0), - new TaskId(2, 1), - new TaskId(2, 2), - new TaskId(3, 0), - new TaskId(3, 1), - new TaskId(3, 2)); + final boolean followupRebalanceNeeded = assign( + TASK_0_0, + TASK_0_1, + TASK_0_2, + new TaskId(1, 0), + new TaskId(1, 1), + new TaskId(1, 2), + new TaskId(2, 0), + new TaskId(2, 1), + new TaskId(2, 2), + new TaskId(3, 0), + new TaskId(3, 1), + new TaskId(3, 2) + ); assertThat(followupRebalanceNeeded, is(false)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(8)); From e3bf615ecd9842630013745174bafb98a9fa8a3e Mon Sep 17 00:00:00 2001 From: John Roesler Date: Mon, 27 Apr 2020 16:26:43 -0500 Subject: [PATCH 09/12] Update streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java Co-Authored-By: Bruno Cadonna --- .../assignment/TaskAssignorConvergenceTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java index 0944e6d194549..9517400315400 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignorConvergenceTest.java @@ -416,10 +416,12 @@ private static void testForConvergence(final Harness harness, iteration++; harness.prepareForNextRebalance(); harness.recordBefore(iteration); - rebalancePending = new HighAvailabilityTaskAssignor().assign(harness.clientStates, - allTasks, - harness.statefulTaskEndOffsetSums.keySet(), - configs); + rebalancePending = new HighAvailabilityTaskAssignor().assign( + harness.clientStates, + allTasks, + harness.statefulTaskEndOffsetSums.keySet(), + configs + ); harness.recordAfter(iteration, rebalancePending); } From e077be2947f06b9a8dc4797c01687aaa4620ddf6 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Mon, 27 Apr 2020 18:24:36 -0500 Subject: [PATCH 10/12] cr feedback --- .../apache/kafka/common/utils/UtilsTest.java | 15 +++++++++ .../internals/StreamsPartitionAssignor.java | 30 ++++++++++------- ...or.java => FallbackPriorTaskAssignor.java} | 15 +++++++-- .../integration/LagFetchIntegrationTest.java | 4 +-- .../StreamsPartitionAssignorTest.java | 4 +-- .../internals/assignment/ClientStateTest.java | 33 ++++++++++++++++++- ...ava => FallbackPriorTaskAssignorTest.java} | 6 ++-- 7 files changed, 84 insertions(+), 23 deletions(-) rename streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/{PriorTaskAssignor.java => FallbackPriorTaskAssignor.java} (74%) rename streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/{PriorTaskAssignorTest.java => FallbackPriorTaskAssignorTest.java} (94%) diff --git a/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java b/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java index c0e5fc8f454f7..0744f77ac365e 100755 --- a/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java +++ b/clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java @@ -37,6 +37,7 @@ import java.util.Properties; import java.util.Random; import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -47,7 +48,11 @@ import static org.apache.kafka.common.utils.Utils.getPort; import static org.apache.kafka.common.utils.Utils.mkSet; import static org.apache.kafka.common.utils.Utils.murmur2; +import static org.apache.kafka.common.utils.Utils.union; import static org.apache.kafka.common.utils.Utils.validHostPattern; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -582,4 +587,14 @@ public void testConvertTo32BitField() { } catch (IllegalArgumentException e) { } } + + @Test + public void testUnion() { + final Set oneSet = mkSet("a", "b", "c"); + final Set anotherSet = mkSet("c", "d", "e"); + final Set union = union(TreeSet::new, oneSet, anotherSet); + + assertThat(union, is(mkSet("a", "b", "c", "d", "e"))); + assertThat(union.getClass(), equalTo(TreeSet.class)); + } } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java index 2bf489e570da0..fd52f08c4521c 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java @@ -39,7 +39,7 @@ import org.apache.kafka.streams.processor.internals.assignment.AssignorError; import org.apache.kafka.streams.processor.internals.assignment.ClientState; import org.apache.kafka.streams.processor.internals.assignment.CopartitionedTopicsEnforcer; -import org.apache.kafka.streams.processor.internals.assignment.PriorTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.FallbackPriorTaskAssignor; import org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo; import org.apache.kafka.streams.processor.internals.assignment.TaskAssignor; import org.apache.kafka.streams.state.HostInfo; @@ -171,7 +171,7 @@ public String toString() { private CopartitionedTopicsEnforcer copartitionedTopicsEnforcer; private RebalanceProtocol rebalanceProtocol; - private Supplier taskAssignor; + private Supplier taskAssignorSupplier; /** * We need to have the PartitionAssignor and its StreamThread to be mutually accessible since the former needs @@ -201,7 +201,7 @@ public void configure(final Map configs) { internalTopicManager = assignorConfiguration.getInternalTopicManager(); copartitionedTopicsEnforcer = assignorConfiguration.getCopartitionedTopicsEnforcer(); rebalanceProtocol = assignorConfiguration.rebalanceProtocol(); - taskAssignor = assignorConfiguration::getTaskAssignor; + taskAssignorSupplier = assignorConfiguration::getTaskAssignor; } @Override @@ -712,15 +712,8 @@ private boolean assignTasksToClients(final Set allSourceTopics, log.debug("Assigning tasks {} to clients {} with number of replicas {}", allTasks, clientStates, numStandbyReplicas()); - final TaskAssignor taskAssignor; - if (!lagComputationSuccessful) { - log.info("Failed to fetch end offsets for changelogs, will return previous assignment to clients and " - + "trigger another rebalance to retry."); - setAssignmentErrorCode(AssignorError.REBALANCE_NEEDED.code()); - taskAssignor = new PriorTaskAssignor(); - } else { - taskAssignor = this.taskAssignor.get(); - } + final TaskAssignor taskAssignor = createTaskAssignor(lagComputationSuccessful); + final boolean followupRebalanceNeeded = taskAssignor.assign(clientStates, allTasks, statefulTasks, @@ -732,6 +725,19 @@ private boolean assignTasksToClients(final Set allSourceTopics, return followupRebalanceNeeded; } + private TaskAssignor createTaskAssignor(final boolean lagComputationSuccessful) { + final TaskAssignor taskAssignor; + if (lagComputationSuccessful) { + taskAssignor = taskAssignorSupplier.get(); + } else { + log.info("Failed to fetch end offsets for changelogs, will return previous assignment to clients and " + + "trigger another rebalance to retry."); + setAssignmentErrorCode(AssignorError.REBALANCE_NEEDED.code()); + taskAssignor = new FallbackPriorTaskAssignor(); + } + return taskAssignor; + } + /** * Builds a map from client to state, and readies each ClientState for assignment by adding any missing prev tasks * and computing the per-task overall lag based on the fetched end offsets for each changelog. diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignor.java similarity index 74% rename from streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignor.java rename to streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignor.java index 581637f583311..b17b25c2b7801 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignor.java @@ -23,10 +23,18 @@ import java.util.Set; import java.util.UUID; -public class PriorTaskAssignor implements TaskAssignor { +/** + * A special task assignor implementation to be used as a fallback in case the + * configured assignor couldn't be invoked. + * + * Specifically, this assignor must: + * 1. ignore the task lags in the ClientState map + * 2. always return true, indicating that a follow-up rebalance is needed + */ +public class FallbackPriorTaskAssignor implements TaskAssignor { private final StickyTaskAssignor delegate; - public PriorTaskAssignor() { + public FallbackPriorTaskAssignor() { delegate = new StickyTaskAssignor(true); } @@ -35,6 +43,7 @@ public boolean assign(final Map clients, final Set allTaskIds, final Set standbyTaskIds, final AssignmentConfigs configs) { - return delegate.assign(clients, allTaskIds, standbyTaskIds, configs); + delegate.assign(clients, allTaskIds, standbyTaskIds, configs); + return true; } } diff --git a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java index b9a578e2a6ab9..f5143e1df95b1 100644 --- a/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/integration/LagFetchIntegrationTest.java @@ -36,7 +36,7 @@ import org.apache.kafka.streams.kstream.Materialized; import org.apache.kafka.streams.processor.StateRestoreListener; import org.apache.kafka.streams.processor.internals.StreamThread; -import org.apache.kafka.streams.processor.internals.assignment.PriorTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.FallbackPriorTaskAssignor; import org.apache.kafka.test.IntegrationTest; import org.apache.kafka.test.TestUtils; import org.junit.After; @@ -151,7 +151,7 @@ private void shouldFetchLagsDuringRebalancing(final String optimization) throws final Properties props = (Properties) streamsConfiguration.clone(); // this test relies on the second instance getting the standby, so we specify // an assignor with this contract. - props.put(StreamsConfig.InternalConfig.INTERNAL_TASK_ASSIGNOR_CLASS, PriorTaskAssignor.class.getName()); + props.put(StreamsConfig.InternalConfig.INTERNAL_TASK_ASSIGNOR_CLASS, FallbackPriorTaskAssignor.class.getName()); props.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "localhost:" + i); props.put(StreamsConfig.CLIENT_ID_CONFIG, "instance-" + i); props.put(StreamsConfig.TOPOLOGY_OPTIMIZATION, optimization); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java index 3ce208f23317d..814bbcfe7bdfa 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignorTest.java @@ -50,7 +50,7 @@ import org.apache.kafka.streams.processor.internals.assignment.AssignorError; import org.apache.kafka.streams.processor.internals.assignment.ClientState; import org.apache.kafka.streams.processor.internals.assignment.HighAvailabilityTaskAssignor; -import org.apache.kafka.streams.processor.internals.assignment.PriorTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.FallbackPriorTaskAssignor; import org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor; import org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo; import org.apache.kafka.streams.processor.internals.assignment.TaskAssignor; @@ -293,7 +293,7 @@ public static Collection parameters() { return asList( new Object[]{HighAvailabilityTaskAssignor.class}, new Object[]{StickyTaskAssignor.class}, - new Object[]{PriorTaskAssignor.class} + new Object[]{FallbackPriorTaskAssignor.class} ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java index cb32155690c9f..ac9dafe8b1363 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/ClientStateTest.java @@ -16,22 +16,26 @@ */ package org.apache.kafka.streams.processor.internals.assignment; -import java.util.Map; import org.apache.kafka.common.utils.Utils; import org.apache.kafka.streams.processor.TaskId; import org.apache.kafka.streams.processor.internals.Task; import org.junit.Test; import java.util.Collections; +import java.util.Map; import static org.apache.kafka.common.utils.Utils.mkEntry; import static org.apache.kafka.common.utils.Utils.mkMap; import static org.apache.kafka.common.utils.Utils.mkSet; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_0; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_1; import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_2; +import static org.apache.kafka.streams.processor.internals.assignment.AssignmentTestUtils.TASK_0_3; import static org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo.UNKNOWN_OFFSET_SUM; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -41,6 +45,33 @@ public class ClientStateTest { private final ClientState client = new ClientState(1); private final ClientState zeroCapacityClient = new ClientState(0); + @Test + public void previousStateConstructorShouldCreateAValidObject() { + final ClientState clientState = new ClientState( + mkSet(TASK_0_0, TASK_0_1), + mkSet(TASK_0_2, TASK_0_3), + mkMap(mkEntry(TASK_0_0, 5L), mkEntry(TASK_0_2, -1L)), + 4 + ); + + // all the "next assignment" fields should be empty + assertThat(clientState.activeTaskCount(), is(0)); + assertThat(clientState.activeTaskLoad(), is(0.0)); + assertThat(clientState.activeTasks(), is(empty())); + assertThat(clientState.standbyTaskCount(), is(0)); + assertThat(clientState.standbyTasks(), is(empty())); + assertThat(clientState.assignedTaskCount(), is(0)); + assertThat(clientState.assignedTasks(), is(empty())); + + // and the "previous assignment" fields should match the constructor args + assertThat(clientState.prevActiveTasks(), is(mkSet(TASK_0_0, TASK_0_1))); + assertThat(clientState.prevStandbyTasks(), is(mkSet(TASK_0_2, TASK_0_3))); + assertThat(clientState.previousAssignedTasks(), is(mkSet(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3))); + assertThat(clientState.capacity(), is(4)); + assertThat(clientState.lagFor(TASK_0_0), is(5L)); + assertThat(clientState.lagFor(TASK_0_2), is(-1L)); + } + @Test public void shouldHaveNotReachedCapacityWhenAssignedTasksLessThanCapacity() { assertFalse(client.reachedCapacity()); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignorTest.java similarity index 94% rename from streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignorTest.java rename to streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignorTest.java index 5d256bbd8d7b6..4139817b02893 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/PriorTaskAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignorTest.java @@ -38,7 +38,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -public class PriorTaskAssignorTest { +public class FallbackPriorTaskAssignorTest { private final Map clients = new TreeMap<>(); @@ -49,13 +49,13 @@ public void shouldViolateBalanceToPreserveActiveTaskStickiness() { final List taskIds = asList(TASK_0_0, TASK_0_1, TASK_0_2); Collections.shuffle(taskIds); - final boolean followupRebalanceNeeded = new PriorTaskAssignor().assign( + final boolean followupRebalanceNeeded = new FallbackPriorTaskAssignor().assign( clients, new HashSet<>(taskIds), new HashSet<>(taskIds), new AssignorConfiguration.AssignmentConfigs(0L, 0, 0, 0, 0L) ); - assertThat(followupRebalanceNeeded, is(false)); + assertThat(followupRebalanceNeeded, is(true)); assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2))); assertThat(c2.activeTasks(), empty()); From d22de81d1e873c13b208ddedbb8eec1bd44628a9 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Mon, 27 Apr 2020 21:58:29 -0500 Subject: [PATCH 11/12] cr feedback --- .../internals/StreamsPartitionAssignor.java | 15 +++++--- ...ilabilityStreamsPartitionAssignorTest.java | 37 +++++++++++-------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java index fd52f08c4521c..6d3add8c28172 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java @@ -40,6 +40,7 @@ import org.apache.kafka.streams.processor.internals.assignment.ClientState; import org.apache.kafka.streams.processor.internals.assignment.CopartitionedTopicsEnforcer; import org.apache.kafka.streams.processor.internals.assignment.FallbackPriorTaskAssignor; +import org.apache.kafka.streams.processor.internals.assignment.StickyTaskAssignor; import org.apache.kafka.streams.processor.internals.assignment.SubscriptionInfo; import org.apache.kafka.streams.processor.internals.assignment.TaskAssignor; import org.apache.kafka.streams.state.HostInfo; @@ -726,16 +727,18 @@ private boolean assignTasksToClients(final Set allSourceTopics, } private TaskAssignor createTaskAssignor(final boolean lagComputationSuccessful) { - final TaskAssignor taskAssignor; - if (lagComputationSuccessful) { - taskAssignor = taskAssignorSupplier.get(); + final TaskAssignor taskAssignor = taskAssignorSupplier.get(); + if (taskAssignor instanceof StickyTaskAssignor) { + // special case: to preserve pre-existing behavior, we invoke the StickyTaskAssignor + // whether or not lag computation failed. + return taskAssignor; + } else if (lagComputationSuccessful) { + return taskAssignor; } else { log.info("Failed to fetch end offsets for changelogs, will return previous assignment to clients and " + "trigger another rebalance to retry."); - setAssignmentErrorCode(AssignorError.REBALANCE_NEEDED.code()); - taskAssignor = new FallbackPriorTaskAssignor(); + return new FallbackPriorTaskAssignor(); } - return taskAssignor; } /** diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java index dd34134359930..8cd57ba73dd44 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java @@ -57,7 +57,6 @@ import java.util.stream.Collectors; import static java.util.Arrays.asList; -import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; @@ -74,7 +73,9 @@ import static org.easymock.EasyMock.expect; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertTrue; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; public class HighAvailabilityStreamsPartitionAssignorTest { @@ -128,11 +129,6 @@ private Map configProps() { return configurationMap; } - // Make sure to complete setting up any mocks (such as TaskManager or AdminClient) before configuring the assignor - private void configureDefaultPartitionAssignor() { - configurePartitionAssignorWith(emptyMap()); - } - // Make sure to complete setting up any mocks (such as TaskManager or AdminClient) before configuring the assignor private void configurePartitionAssignorWith(final Map props) { final Map configMap = configProps(); @@ -195,6 +191,8 @@ public void setUp() { @Test public void shouldReturnAllActiveTasksToPreviousOwnerRegardlessOfBalanceAndTriggerRebalanceIfEndOffsetFetchFailsAndHighAvailabilityEnabled() { + final long rebalanceInterval = 5 * 60 * 1000L; + builder.addSource(null, "source1", null, null, null, "topic1"); builder.addProcessor("processor1", new MockProcessorSupplier<>(), "source1"); builder.addStateStore(new MockKeyValueStoreBuilder("store1", false), "processor1"); @@ -203,7 +201,7 @@ public void shouldReturnAllActiveTasksToPreviousOwnerRegardlessOfBalanceAndTrigg createMockTaskManager(allTasks); adminClient = EasyMock.createMock(AdminClient.class); expect(adminClient.listOffsets(anyObject())).andThrow(new StreamsException("Should be handled")); - configureDefaultPartitionAssignor(); + configurePartitionAssignorWith(singletonMap(StreamsConfig.PROBING_REBALANCE_INTERVAL_MS_CONFIG, rebalanceInterval)); final String firstConsumer = "consumer1"; final String newConsumer = "consumer2"; @@ -223,14 +221,23 @@ public void shouldReturnAllActiveTasksToPreviousOwnerRegardlessOfBalanceAndTrigg .assign(metadata, new GroupSubscription(subscriptions)) .groupAssignment(); - final List firstConsumerActiveTasks = - AssignmentInfo.decode(assignments.get(firstConsumer).userData()).activeTasks(); - final List newConsumerActiveTasks = - AssignmentInfo.decode(assignments.get(newConsumer).userData()).activeTasks(); + final AssignmentInfo firstConsumerUserData = AssignmentInfo.decode(assignments.get(firstConsumer).userData()); + final List firstConsumerActiveTasks = firstConsumerUserData.activeTasks(); + final AssignmentInfo newConsumerUserData = AssignmentInfo.decode(assignments.get(newConsumer).userData()); + final List newConsumerActiveTasks = newConsumerUserData.activeTasks(); + // The tasks were returned to their prior owner assertThat(firstConsumerActiveTasks, equalTo(new ArrayList<>(allTasks))); - assertTrue(newConsumerActiveTasks.isEmpty()); - assertThat(assignmentError.get(), equalTo(AssignorError.REBALANCE_NEEDED.code())); + assertThat(newConsumerActiveTasks, empty()); + + // There is a rebalance scheduled + assertThat( + time.milliseconds() + rebalanceInterval, + anyOf( + is(firstConsumerUserData.nextRebalanceMs()), + is(newConsumerUserData.nextRebalanceMs()) + ) + ); } @Test @@ -272,7 +279,7 @@ public void shouldScheduleProbingRebalanceOnThisClientIfWarmupTasksRequired() { AssignmentInfo.decode(assignments.get(newConsumer).userData()).activeTasks(); assertThat(firstConsumerActiveTasks, equalTo(new ArrayList<>(allTasks))); - assertTrue(newConsumerActiveTasks.isEmpty()); + assertThat(newConsumerActiveTasks, empty()); assertThat(assignmentError.get(), equalTo(AssignorError.NONE.code())); From d58f62dc73dc3f4832cb89b5be6a8c8ce2f32e60 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Mon, 27 Apr 2020 22:21:14 -0500 Subject: [PATCH 12/12] rename 'followup' to 'probing' --- .../internals/StreamsPartitionAssignor.java | 24 ++-- .../HighAvailabilityTaskAssignor.java | 8 +- .../internals/assignment/TaskAssignor.java | 2 +- ...ilabilityStreamsPartitionAssignorTest.java | 2 +- .../FallbackPriorTaskAssignorTest.java | 4 +- .../assignment/StickyTaskAssignorTest.java | 120 +++++++++--------- 6 files changed, 80 insertions(+), 80 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java index 6d3add8c28172..666da21877b85 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java @@ -362,7 +362,7 @@ public GroupAssignment assign(final Cluster metadata, final GroupSubscription gr final Map> partitionsForTask = partitionGrouper.partitionGroups(sourceTopicsByGroup, fullMetadata); - final boolean followupRebalanceNeeded = + final boolean probingRebalanceNeeded = assignTasksToClients(allSourceTopics, partitionsForTask, topicGroups, clientMetadataMap, fullMetadata); // ---------------- Step Three ---------------- // @@ -400,7 +400,7 @@ public GroupAssignment assign(final Cluster metadata, final GroupSubscription gr allOwnedPartitions, minReceivedMetadataVersion, minSupportedMetadataVersion, - followupRebalanceNeeded + probingRebalanceNeeded ); } @@ -689,7 +689,7 @@ private Map> prepareChangelogTopics(final Map allSourceTopics, final Map> partitionsForTask, @@ -715,15 +715,15 @@ private boolean assignTasksToClients(final Set allSourceTopics, final TaskAssignor taskAssignor = createTaskAssignor(lagComputationSuccessful); - final boolean followupRebalanceNeeded = taskAssignor.assign(clientStates, - allTasks, - statefulTasks, - assignmentConfigs); + final boolean probingRebalanceNeeded = taskAssignor.assign(clientStates, + allTasks, + statefulTasks, + assignmentConfigs); log.info("Assigned tasks to clients as {}{}.", Utils.NL, clientStates.entrySet().stream().map(Map.Entry::toString).collect(Collectors.joining(Utils.NL))); - return followupRebalanceNeeded; + return probingRebalanceNeeded; } private TaskAssignor createTaskAssignor(final boolean lagComputationSuccessful) { @@ -972,9 +972,9 @@ private void addClientAssignments(final Map assignment, final int minUserMetadataVersion, final int minSupportedMetadataVersion, final boolean versionProbing, - final boolean followupRebalanceNeeded) { - boolean encodeNextRebalanceTime = followupRebalanceNeeded; - boolean stableAssignment = !followupRebalanceNeeded && !versionProbing; + final boolean probingRebalanceNeeded) { + boolean encodeNextRebalanceTime = probingRebalanceNeeded; + boolean stableAssignment = !probingRebalanceNeeded && !versionProbing; // Loop through the consumers and build their assignment for (final String consumer : clientMetadata.consumers) { @@ -1029,7 +1029,7 @@ private void addClientAssignments(final Map assignment, if (stableAssignment) { log.info("Finished stable assignment of tasks, no followup rebalances required."); } else { - log.info("Finished unstable assignment of tasks, a followup rebalance will be triggered."); + log.info("Finished unstable assignment of tasks, a followup probing rebalance will be triggered."); } } diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java index 860e33ef08ee9..3253c2680dd93 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignor.java @@ -88,7 +88,7 @@ public boolean assign(final Map clientStates, final Map tasksToRemainingStandbys = statefulTasks.stream().collect(Collectors.toMap(task -> task, t -> configs.numStandbyReplicas)); - final boolean followupRebalanceNeeded = assignStatefulActiveTasks(tasksToRemainingStandbys); + final boolean probingRebalanceNeeded = assignStatefulActiveTasks(tasksToRemainingStandbys); assignStandbyReplicaTasks(tasksToRemainingStandbys); @@ -97,9 +97,9 @@ public boolean assign(final Map clientStates, log.info("Decided on assignment: " + clientStates + " with " + - (followupRebalanceNeeded ? "" : "no") + - " followup rebalance."); - return followupRebalanceNeeded; + (probingRebalanceNeeded ? "" : "no") + + " followup probing rebalance."); + return probingRebalanceNeeded; } private boolean assignStatefulActiveTasks(final Map tasksToRemainingStandbys) { diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java index 64522872f967e..485bd81634f9e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/TaskAssignor.java @@ -24,7 +24,7 @@ public interface TaskAssignor { /** - * @return whether the generated assignment requires a followup rebalance to satisfy all conditions + * @return whether the generated assignment requires a followup probing rebalance to satisfy all conditions */ boolean assign(Map clients, Set allTaskIds, diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java index 8cd57ba73dd44..2c27d11bddec6 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/HighAvailabilityStreamsPartitionAssignorTest.java @@ -229,7 +229,7 @@ public void shouldReturnAllActiveTasksToPreviousOwnerRegardlessOfBalanceAndTrigg // The tasks were returned to their prior owner assertThat(firstConsumerActiveTasks, equalTo(new ArrayList<>(allTasks))); assertThat(newConsumerActiveTasks, empty()); - + // There is a rebalance scheduled assertThat( time.milliseconds() + rebalanceInterval, diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignorTest.java index 4139817b02893..687e5b6df54ee 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/FallbackPriorTaskAssignorTest.java @@ -49,13 +49,13 @@ public void shouldViolateBalanceToPreserveActiveTaskStickiness() { final List taskIds = asList(TASK_0_0, TASK_0_1, TASK_0_2); Collections.shuffle(taskIds); - final boolean followupRebalanceNeeded = new FallbackPriorTaskAssignor().assign( + final boolean probingRebalanceNeeded = new FallbackPriorTaskAssignor().assign( clients, new HashSet<>(taskIds), new HashSet<>(taskIds), new AssignorConfiguration.AssignmentConfigs(0L, 0, 0, 0, 0L) ); - assertThat(followupRebalanceNeeded, is(true)); + assertThat(probingRebalanceNeeded, is(true)); assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2))); assertThat(c2.activeTasks(), empty()); diff --git a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java index f6a7d14d0c0b7..5203832f28a28 100644 --- a/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/StickyTaskAssignorTest.java @@ -76,8 +76,8 @@ public void shouldAssignOneActiveTaskToEachProcessWhenTaskCountSameAsProcessCoun createClient(UUID_2, 1); createClient(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(probingRebalanceNeeded, is(false)); for (final ClientState clientState : clients.values()) { assertThat(clientState.activeTaskCount(), equalTo(1)); @@ -90,8 +90,8 @@ public void shouldAssignTopicGroupIdEvenlyAcrossClientsWithNoStandByTasks() { createClient(UUID_2, 2); createClient(UUID_3, 2); - final boolean followupRebalanceNeeded = assign(TASK_1_0, TASK_1_1, TASK_2_2, TASK_2_0, TASK_2_1, TASK_1_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_1_0, TASK_1_1, TASK_2_2, TASK_2_0, TASK_2_1, TASK_1_2); + assertThat(probingRebalanceNeeded, is(false)); assertActiveTaskTopicGroupIdsEvenlyDistributed(); } @@ -102,8 +102,8 @@ public void shouldAssignTopicGroupIdEvenlyAcrossClientsWithStandByTasks() { createClient(UUID_2, 2); createClient(UUID_3, 2); - final boolean followupRebalanceNeeded = assign(1, TASK_2_0, TASK_1_1, TASK_1_2, TASK_1_0, TASK_2_1, TASK_2_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_2_0, TASK_1_1, TASK_1_2, TASK_1_0, TASK_2_1, TASK_2_2); + assertThat(probingRebalanceNeeded, is(false)); assertActiveTaskTopicGroupIdsEvenlyDistributed(); } @@ -138,9 +138,9 @@ public void shouldMigrateActiveTasksToNewProcessWithoutChangingAllAssignments() createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1); createClient(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_2).activeTasks(), equalTo(singleton(TASK_0_1))); assertThat(clients.get(UUID_1).activeTasks().size(), equalTo(1)); assertThat(clients.get(UUID_3).activeTasks().size(), equalTo(1)); @@ -151,9 +151,9 @@ public void shouldMigrateActiveTasksToNewProcessWithoutChangingAllAssignments() public void shouldAssignBasedOnCapacity() { createClient(UUID_1, 1); createClient(UUID_2, 2); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks().size(), equalTo(1)); assertThat(clients.get(UUID_2).activeTasks().size(), equalTo(2)); } @@ -212,9 +212,9 @@ public void shouldAssignTasksToClientWithPreviousStandbyTasks() { final ClientState client3 = createClient(UUID_3, 1); client3.addPreviousStandbyTasks(mkSet(TASK_0_0)); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks(), equalTo(singleton(TASK_0_2))); assertThat(clients.get(UUID_2).activeTasks(), equalTo(singleton(TASK_0_1))); @@ -228,9 +228,9 @@ public void shouldAssignBasedOnCapacityWhenMultipleClientHaveStandbyTasks() { final ClientState c2 = createClientWithPreviousActiveTasks(UUID_2, 2, TASK_0_2); c2.addPreviousStandbyTasks(mkSet(TASK_0_1)); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks(), equalTo(singleton(TASK_0_0))); assertThat(clients.get(UUID_2).activeTasks(), equalTo(mkSet(TASK_0_2, TASK_0_1))); @@ -243,8 +243,8 @@ public void shouldAssignStandbyTasksToDifferentClientThanCorrespondingActiveTask createClientWithPreviousActiveTasks(UUID_3, 1, TASK_0_2); createClientWithPreviousActiveTasks(UUID_4, 1, TASK_0_3); - final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).standbyTasks(), not(hasItems(TASK_0_0))); @@ -271,8 +271,8 @@ public void shouldAssignMultipleReplicasOfStandbyTask() { createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1); createClientWithPreviousActiveTasks(UUID_3, 1, TASK_0_2); - final boolean followupRebalanceNeeded = assign(2, TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(2, TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).standbyTasks(), equalTo(mkSet(TASK_0_1, TASK_0_2))); assertThat(clients.get(UUID_2).standbyTasks(), equalTo(mkSet(TASK_0_2, TASK_0_0))); @@ -282,8 +282,8 @@ public void shouldAssignMultipleReplicasOfStandbyTask() { @Test public void shouldNotAssignStandbyTaskReplicasWhenNoClientAvailableWithoutHavingTheTaskAssigned() { createClient(UUID_1, 1); - final boolean followupRebalanceNeeded = assign(1, TASK_0_0); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_0_0); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).standbyTasks().size(), equalTo(0)); } @@ -293,8 +293,8 @@ public void shouldAssignActiveAndStandbyTasks() { createClient(UUID_2, 1); createClient(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(probingRebalanceNeeded, is(false)); assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2))); assertThat(allStandbyTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2))); @@ -306,8 +306,8 @@ public void shouldAssignAtLeastOneTaskToEachClientIfPossible() { createClient(UUID_2, 1); createClient(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_3).assignedTaskCount(), equalTo(1)); @@ -322,8 +322,8 @@ public void shouldAssignEachActiveTaskToOneClientWhenMoreClientsThanTasks() { createClient(UUID_5, 1); createClient(UUID_6, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(probingRebalanceNeeded, is(false)); assertThat(allActiveTasks(), equalTo(asList(TASK_0_0, TASK_0_1, TASK_0_2))); } @@ -337,8 +337,8 @@ public void shouldBalanceActiveAndStandbyTasksAcrossAvailableClients() { createClient(UUID_5, 1); createClient(UUID_6, 1); - final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_0_0, TASK_0_1, TASK_0_2); + assertThat(probingRebalanceNeeded, is(false)); for (final ClientState clientState : clients.values()) { assertThat(clientState.assignedTaskCount(), equalTo(1)); @@ -350,7 +350,7 @@ public void shouldAssignMoreTasksToClientWithMoreCapacity() { createClient(UUID_2, 2); createClient(UUID_1, 1); - final boolean followupRebalanceNeeded = assign( + final boolean probingRebalanceNeeded = assign( TASK_0_0, TASK_0_1, TASK_0_2, @@ -365,7 +365,7 @@ public void shouldAssignMoreTasksToClientWithMoreCapacity() { new TaskId(3, 2) ); - assertThat(followupRebalanceNeeded, is(false)); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(8)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(4)); } @@ -389,8 +389,8 @@ public void shouldEvenlyDistributeByTaskIdAndPartition() { Collections.shuffle(taskIds); taskIds.toArray(taskIdArray); - final boolean followupRebalanceNeeded = assign(taskIdArray); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(taskIdArray); + assertThat(probingRebalanceNeeded, is(false)); Collections.sort(taskIds); final Set expectedClientOneAssignment = getExpectedTaskIdAssignment(taskIds, 0, 4, 8, 12); @@ -414,8 +414,8 @@ public void shouldNotHaveSameAssignmentOnAnyTwoHosts() { createClient(UUID_3, 1); createClient(UUID_4, 1); - final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); for (final UUID uuid : allUUIDs) { final Set taskIds = clients.get(uuid).assignedTasks(); @@ -437,8 +437,8 @@ public void shouldNotHaveSameAssignmentOnAnyTwoHostsWhenThereArePreviousActiveTa createClientWithPreviousActiveTasks(UUID_3, 1, TASK_0_0); createClient(UUID_4, 1); - final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); for (final UUID uuid : allUUIDs) { final Set taskIds = clients.get(uuid).assignedTasks(); @@ -464,8 +464,8 @@ public void shouldNotHaveSameAssignmentOnAnyTwoHostsWhenThereArePreviousStandbyT createClient(UUID_3, 1); createClient(UUID_4, 1); - final boolean followupRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(1, TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); for (final UUID uuid : allUUIDs) { final Set taskIds = clients.get(uuid).assignedTasks(); @@ -486,8 +486,8 @@ public void shouldReBalanceTasksAcrossAllClientsWhenCapacityAndTaskCountTheSame( createClient(UUID_2, 1); createClient(UUID_4, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(1)); @@ -501,8 +501,8 @@ public void shouldReBalanceTasksAcrossClientsWhenCapacityLessThanTaskCount() { createClient(UUID_1, 1); createClient(UUID_2, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_3).assignedTaskCount(), equalTo(2)); assertThat(clients.get(UUID_1).assignedTaskCount(), equalTo(1)); @@ -513,8 +513,8 @@ public void shouldReBalanceTasksAcrossClientsWhenCapacityLessThanTaskCount() { public void shouldRebalanceTasksToClientsBasedOnCapacity() { createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_0, TASK_0_3, TASK_0_2); createClient(UUID_3, 2); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_2).assignedTaskCount(), equalTo(1)); assertThat(clients.get(UUID_3).assignedTaskCount(), equalTo(2)); } @@ -528,8 +528,8 @@ public void shouldMoveMinimalNumberOfTasksWhenPreviouslyAboveCapacityAndNewClien createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_1, TASK_0_3); createClientWithPreviousActiveTasks(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_2, TASK_0_1, TASK_0_3); + assertThat(probingRebalanceNeeded, is(false)); final Set p3ActiveTasks = clients.get(UUID_3).activeTasks(); assertThat(p3ActiveTasks.size(), equalTo(1)); @@ -545,8 +545,8 @@ public void shouldNotMoveAnyTasksWhenNewTasksAdded() { createClientWithPreviousActiveTasks(UUID_1, 1, TASK_0_0, TASK_0_1); createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_2, TASK_0_3); - final boolean followupRebalanceNeeded = assign(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks(), hasItems(TASK_0_0, TASK_0_1)); assertThat(clients.get(UUID_2).activeTasks(), hasItems(TASK_0_2, TASK_0_3)); @@ -559,8 +559,8 @@ public void shouldAssignNewTasksToNewClientWhenPreviousTasksAssignedToOldClients createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_0, TASK_0_3); createClient(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_3, TASK_0_1, TASK_0_4, TASK_0_2, TASK_0_0, TASK_0_5); + assertThat(probingRebalanceNeeded, is(false)); assertThat(clients.get(UUID_1).activeTasks(), hasItems(TASK_0_2, TASK_0_1)); assertThat(clients.get(UUID_2).activeTasks(), hasItems(TASK_0_0, TASK_0_3)); @@ -579,8 +579,8 @@ public void shouldAssignTasksNotPreviouslyActiveToNewClient() { final ClientState newClient = createClient(UUID_4, 1); newClient.addPreviousStandbyTasks(mkSet(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3)); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); + assertThat(probingRebalanceNeeded, is(false)); assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_1, TASK_1_2, TASK_1_3))); assertThat(c2.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_1_1, TASK_2_2))); @@ -601,8 +601,8 @@ public void shouldAssignTasksNotPreviouslyActiveToMultipleNewClients() { final ClientState bounce2 = createClient(UUID_4, 1); bounce2.addPreviousStandbyTasks(mkSet(TASK_0_2, TASK_0_3, TASK_1_0)); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_1_0, TASK_0_1, TASK_0_2, TASK_1_1, TASK_2_0, TASK_0_3, TASK_1_2, TASK_2_1, TASK_1_3, TASK_2_2, TASK_2_3); + assertThat(probingRebalanceNeeded, is(false)); assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_1, TASK_1_2, TASK_1_3))); assertThat(c2.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_1_1, TASK_2_2))); @@ -624,8 +624,8 @@ public void shouldAssignTasksToNewClientWithoutFlippingAssignmentBetweenExisting final ClientState c2 = createClientWithPreviousActiveTasks(UUID_2, 1, TASK_0_3, TASK_0_4, TASK_0_5); final ClientState newClient = createClient(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5); + assertThat(probingRebalanceNeeded, is(false)); assertThat(c1.activeTasks(), not(hasItem(TASK_0_3))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_4))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_5))); @@ -644,8 +644,8 @@ public void shouldAssignTasksToNewClientWithoutFlippingAssignmentBetweenExisting c2.addPreviousStandbyTasks(mkSet(TASK_0_3, TASK_0_4, TASK_0_5)); final ClientState newClient = createClient(UUID_3, 1); - final boolean followupRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5, TASK_0_6); - assertThat(followupRebalanceNeeded, is(false)); + final boolean probingRebalanceNeeded = assign(TASK_0_0, TASK_0_1, TASK_0_2, TASK_0_3, TASK_0_4, TASK_0_5, TASK_0_6); + assertThat(probingRebalanceNeeded, is(false)); assertThat(c1.activeTasks(), not(hasItem(TASK_0_3))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_4))); assertThat(c1.activeTasks(), not(hasItem(TASK_0_5))); @@ -664,13 +664,13 @@ public void shouldViolateBalanceToPreserveActiveTaskStickiness() { final List taskIds = asList(TASK_0_0, TASK_0_1, TASK_0_2); Collections.shuffle(taskIds); - final boolean followupRebalanceNeeded = new StickyTaskAssignor(true).assign( + final boolean probingRebalanceNeeded = new StickyTaskAssignor(true).assign( clients, new HashSet<>(taskIds), new HashSet<>(taskIds), new AssignorConfiguration.AssignmentConfigs(0L, 0, 0, 0, 0L) ); - assertThat(followupRebalanceNeeded, is(false)); + assertThat(probingRebalanceNeeded, is(false)); assertThat(c1.activeTasks(), equalTo(mkSet(TASK_0_0, TASK_0_1, TASK_0_2))); assertThat(c2.activeTasks(), empty());