diff --git a/README.md b/README.md index 8b5fe4c332ed2..06c0e3921ebc1 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ Follow instructions in https://kafka.apache.org/quickstart ### Running a particular unit/integration test ### ./gradlew clients:test --tests RequestResponseTest + ./gradlew streams:integration-tests:test --tests RestoreIntegrationTest ### Repeatedly running a particular unit/integration test with specific times by setting N ### N=500; I=0; while [ $I -lt $N ] && ./gradlew clients:test --tests RequestResponseTest --rerun --fail-fast; do (( I=$I+1 )); echo "Completed run: $I"; sleep 1; done @@ -59,6 +60,7 @@ Follow instructions in https://kafka.apache.org/quickstart ### Running a particular test method within a unit/integration test ### ./gradlew core:test --tests kafka.api.ProducerFailureHandlingTest.testCannotSendToInternalTopic ./gradlew clients:test --tests org.apache.kafka.clients.MetadataTest.testTimeToNextUpdate + ./gradlew streams:integration-tests:test --tests org.apache.kafka.streams.integration.RestoreIntegrationTest.shouldRestoreNullRecord ### Running a particular unit/integration test with log4j output ### By default, there will be only small number of logs output while testing. You can adjust it by changing the `log4j2.yaml` file in the module's `src/test/resources` directory. diff --git a/clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/admin/DescribeProducersWithBrokerIdTest.java b/clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/admin/DescribeProducersWithBrokerIdTest.java new file mode 100644 index 0000000000000..529c59e5e51d9 --- /dev/null +++ b/clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/admin/DescribeProducersWithBrokerIdTest.java @@ -0,0 +1,161 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.clients.admin; + +import org.apache.kafka.clients.producer.ProducerConfig; +import org.apache.kafka.clients.producer.ProducerRecord; +import org.apache.kafka.clients.producer.Producer; +import org.apache.kafka.common.Node; +import org.apache.kafka.common.TopicPartition; +import org.apache.kafka.common.errors.NotLeaderOrFollowerException; +import org.apache.kafka.common.serialization.StringSerializer; +import org.apache.kafka.common.test.ClusterInstance; +import org.apache.kafka.common.test.api.ClusterConfigProperty; +import org.apache.kafka.common.test.api.ClusterTest; +import org.apache.kafka.common.test.api.ClusterTestDefaults; +import org.apache.kafka.test.TestUtils; + +import java.util.List; +import java.util.Map; + +import static org.apache.kafka.coordinator.group.GroupCoordinatorConfig.GROUP_INITIAL_REBALANCE_DELAY_MS_CONFIG; +import static org.apache.kafka.coordinator.group.GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG; +import static org.apache.kafka.server.config.ServerLogConfigs.AUTO_CREATE_TOPICS_ENABLE_CONFIG; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + + +@ClusterTestDefaults( + brokers = 4, + serverProperties = { + @ClusterConfigProperty(key = AUTO_CREATE_TOPICS_ENABLE_CONFIG, value = "false"), + @ClusterConfigProperty(key = OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"), + @ClusterConfigProperty(key = GROUP_INITIAL_REBALANCE_DELAY_MS_CONFIG, value = "0") + } +) +class DescribeProducersWithBrokerIdTest { + private static final String TOPIC_NAME = "test-topic"; + private static final int NUM_PARTITIONS = 3; + private static final short REPLICATION_FACTOR = 3; + + private final ClusterInstance clusterInstance; + public DescribeProducersWithBrokerIdTest(ClusterInstance clusterInstance) { + this.clusterInstance = clusterInstance; + } + + private static void sendTestRecords(Producer producer) { + for (int partition = 0; partition < NUM_PARTITIONS; partition++) { + producer.send(new ProducerRecord<>(TOPIC_NAME, partition, "key-" + partition, "value-" + partition)); + } + producer.flush(); + } + + @ClusterTest + void testDescribeProducersDefaultRoutesToLeader() throws Exception { + clusterInstance.createTopic(TOPIC_NAME, NUM_PARTITIONS, REPLICATION_FACTOR); + + try (var producer = clusterInstance.producer(Map.of( + ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class, + ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class)); + var admin = clusterInstance.admin()) { + + sendTestRecords(producer); + + var topicPartition = new TopicPartition(TOPIC_NAME, 0); + var leaderBrokerId = clusterInstance.getLeaderBrokerId(topicPartition); + + var stateWithExplicitLeader = admin.describeProducers(List.of(topicPartition), + new DescribeProducersOptions().brokerId(leaderBrokerId)) + .partitionResult(topicPartition).get(); + + var stateWithDefaultRouting = admin.describeProducers(List.of(topicPartition)) + .partitionResult(topicPartition).get(); + + assertNotNull(stateWithDefaultRouting); + assertFalse(stateWithDefaultRouting.activeProducers().isEmpty()); + assertEquals(stateWithExplicitLeader.activeProducers(), stateWithDefaultRouting.activeProducers()); + } + } + + @ClusterTest + void testDescribeProducersFromFollower() throws Exception { + clusterInstance.createTopic(TOPIC_NAME, NUM_PARTITIONS, REPLICATION_FACTOR); + + try (var producer = clusterInstance.producer(Map.of( + ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class, + ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class)); + var admin = clusterInstance.admin()) { + + sendTestRecords(producer); + + var topicPartition = new TopicPartition(TOPIC_NAME, 0); + var topicDescription = admin.describeTopics(List.of(TOPIC_NAME)).allTopicNames().get().get(TOPIC_NAME); + var replicaBrokerIds = topicDescription.partitions().get(0).replicas().stream() + .map(Node::id) + .toList(); + + var leaderBrokerId = clusterInstance.getLeaderBrokerId(topicPartition); + var followerBrokerId = replicaBrokerIds.stream() + .filter(id -> !id.equals(leaderBrokerId)) + .findFirst() + .orElseThrow(() -> new IllegalStateException("No follower found")); + + var followerState = admin.describeProducers(List.of(topicPartition), + new DescribeProducersOptions().brokerId(followerBrokerId)) + .partitionResult(topicPartition).get(); + var leaderState = admin.describeProducers(List.of(topicPartition)) + .partitionResult(topicPartition).get(); + + assertNotNull(followerState); + assertFalse(followerState.activeProducers().isEmpty()); + + var followerProducerId = followerState.activeProducers().iterator().next().producerId(); + var leaderProducerId = leaderState.activeProducers().iterator().next().producerId(); + assertEquals(leaderProducerId, followerProducerId); + } + } + + @ClusterTest + void testDescribeProducersWithInvalidBrokerId() throws Exception { + clusterInstance.createTopic(TOPIC_NAME, NUM_PARTITIONS, REPLICATION_FACTOR); + + try (var producer = clusterInstance.producer(Map.of( + ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class, + ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class)); + var admin = clusterInstance.admin()) { + + sendTestRecords(producer); + + var topicPartition = new TopicPartition(TOPIC_NAME, 0); + var topicDescription = admin.describeTopics(List.of(TOPIC_NAME)).allTopicNames().get().get(TOPIC_NAME); + var replicaBrokerIds = topicDescription.partitions().get(0).replicas().stream() + .map(Node::id) + .toList(); + + var nonReplicaBrokerId = clusterInstance.brokerIds().stream() + .filter(id -> !replicaBrokerIds.contains(id)) + .findFirst() + .orElseThrow(() -> new IllegalStateException("No non-replica broker found")); + + TestUtils.assertFutureThrows(NotLeaderOrFollowerException.class, + admin.describeProducers(List.of(topicPartition), + new DescribeProducersOptions().brokerId(nonReplicaBrokerId)) + .partitionResult(topicPartition)); + } + } +} \ No newline at end of file diff --git a/clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/consumer/PlaintextConsumerSubscriptionTest.java b/clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/consumer/PlaintextConsumerSubscriptionTest.java index 065e6600a542e..ff840ebf2ea7a 100644 --- a/clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/consumer/PlaintextConsumerSubscriptionTest.java +++ b/clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/consumer/PlaintextConsumerSubscriptionTest.java @@ -576,7 +576,7 @@ public void testClassicConsumerSubscribeInvalidTopicCanUnsubscribe() throws Inte } @ClusterTest - public void testAsyncConsumerClassicConsumerSubscribeInvalidTopicCanUnsubscribe() throws InterruptedException { + public void testAsyncConsumerSubscribeInvalidTopicCanUnsubscribe() throws InterruptedException { testSubscribeInvalidTopicCanUnsubscribe(GroupProtocol.CONSUMER); } diff --git a/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java b/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java index 1100479021fc5..692847a8b1553 100644 --- a/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java @@ -1051,9 +1051,9 @@ private void handleApiVersionsResponse(List responses, apiVersionsResponse.data().finalizedFeaturesEpoch()); apiVersions.update(node, nodeVersionInfo); this.connectionStates.ready(node); - log.debug("Node {} has finalized features epoch: {}, finalized features: {}, supported features: {}, ZK migration ready: {}, API versions: {}.", + log.debug("Node {} has finalized features epoch: {}, finalized features: {}, supported features: {}, API versions: {}.", node, apiVersionsResponse.data().finalizedFeaturesEpoch(), apiVersionsResponse.data().finalizedFeatures(), - apiVersionsResponse.data().supportedFeatures(), apiVersionsResponse.data().zkMigrationReady(), nodeVersionInfo); + apiVersionsResponse.data().supportedFeatures(), nodeVersionInfo); } /** diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/AlterConfigOp.java b/clients/src/main/java/org/apache/kafka/clients/admin/AlterConfigOp.java index 48d5646764d42..789c9f64a93aa 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/AlterConfigOp.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/AlterConfigOp.java @@ -26,6 +26,20 @@ /** * A class representing an alter configuration entry containing name, value and operation type. + *

+ * Note for Broker Logger Configuration:
+ * When altering broker logger levels (using {@link org.apache.kafka.common.config.ConfigResource.Type#BROKER_LOGGER}), + * it is strongly recommended to use log level constants from {@link org.apache.kafka.common.config.LogLevelConfig} instead of string literals. + * This ensures compatibility with Kafka's log level validation and avoids potential configuration errors. + *

+ * Example: + * 

+ * Recommended approach:
+ * new AlterConfigOp(new ConfigEntry(loggerName, LogLevelConfig.DEBUG_LOG_LEVEL), OpType.SET)
+ *
+ * Avoid this:
+ * new AlterConfigOp(new ConfigEntry(loggerName, "DEBUG"), OpType.SET)
+ *
*/ public class AlterConfigOp { diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java index ac2d67d2022a1..90f83eac935e7 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java @@ -2334,7 +2334,7 @@ private Map> handleDescribeTopicsByNamesWi } // First, we need to retrieve the node info. - DescribeClusterResult clusterResult = describeCluster(); + DescribeClusterResult clusterResult = describeCluster(new DescribeClusterOptions().timeoutMs(options.timeoutMs())); clusterResult.nodes().whenComplete( (nodes, exception) -> { if (exception != null) { @@ -5154,6 +5154,8 @@ private static long getOffsetFromSpec(OffsetSpec offsetSpec) { return ListOffsetsRequest.EARLIEST_LOCAL_TIMESTAMP; } else if (offsetSpec instanceof OffsetSpec.LatestTieredSpec) { return ListOffsetsRequest.LATEST_TIERED_TIMESTAMP; + } else if (offsetSpec instanceof OffsetSpec.EarliestPendingUploadSpec) { + return ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP; } return ListOffsetsRequest.LATEST_TIMESTAMP; } diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/OffsetSpec.java b/clients/src/main/java/org/apache/kafka/clients/admin/OffsetSpec.java index 68f94cc493e5a..ad73c8d51f086 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/OffsetSpec.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/OffsetSpec.java @@ -28,6 +28,7 @@ public static class LatestSpec extends OffsetSpec { } public static class MaxTimestampSpec extends OffsetSpec { } public static class EarliestLocalSpec extends OffsetSpec { } public static class LatestTieredSpec extends OffsetSpec { } + public static class EarliestPendingUploadSpec extends OffsetSpec { } public static class TimestampSpec extends OffsetSpec { private final long timestamp; @@ -91,4 +92,13 @@ public static OffsetSpec earliestLocal() { public static OffsetSpec latestTiered() { return new LatestTieredSpec(); } + + /** + * Used to retrieve the earliest offset of records that are pending upload to remote storage. + *
+ * Note: When tiered storage is not enabled, we will return unknown offset. + */ + public static OffsetSpec earliestPendingUpload() { + return new EarliestPendingUploadSpec(); + } } diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/internals/ListOffsetsHandler.java b/clients/src/main/java/org/apache/kafka/clients/admin/internals/ListOffsetsHandler.java index f7c495d7fd8aa..a46d6f24a7b93 100644 --- a/clients/src/main/java/org/apache/kafka/clients/admin/internals/ListOffsetsHandler.java +++ b/clients/src/main/java/org/apache/kafka/clients/admin/internals/ListOffsetsHandler.java @@ -103,12 +103,17 @@ ListOffsetsRequest.Builder buildBatchedRequest(int brokerId, Set .stream() .anyMatch(key -> offsetTimestampsByPartition.get(key) == ListOffsetsRequest.LATEST_TIERED_TIMESTAMP); + boolean requireEarliestPendingUploadTimestamp = keys + .stream() + .anyMatch(key -> offsetTimestampsByPartition.get(key) == ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP); + int timeoutMs = options.timeoutMs() != null ? options.timeoutMs() : defaultApiTimeoutMs; return ListOffsetsRequest.Builder.forConsumer(true, options.isolationLevel(), supportsMaxTimestamp, requireEarliestLocalTimestamp, - requireTieredStorageTimestamp) + requireTieredStorageTimestamp, + requireEarliestPendingUploadTimestamp) .setTargetTimes(new ArrayList<>(topicsByName.values())) .setTimeoutMs(timeoutMs); } diff --git a/clients/src/main/java/org/apache/kafka/common/requests/ListOffsetsRequest.java b/clients/src/main/java/org/apache/kafka/common/requests/ListOffsetsRequest.java index 7415412d0509c..5862ebdfafc67 100644 --- a/clients/src/main/java/org/apache/kafka/common/requests/ListOffsetsRequest.java +++ b/clients/src/main/java/org/apache/kafka/common/requests/ListOffsetsRequest.java @@ -47,6 +47,8 @@ public class ListOffsetsRequest extends AbstractRequest { public static final long LATEST_TIERED_TIMESTAMP = -5L; + public static final long EARLIEST_PENDING_UPLOAD_TIMESTAMP = -6L; + public static final int CONSUMER_REPLICA_ID = -1; public static final int DEBUGGING_REPLICA_ID = -2; @@ -58,16 +60,19 @@ public static class Builder extends AbstractRequest.Builder public static Builder forConsumer(boolean requireTimestamp, IsolationLevel isolationLevel) { - return forConsumer(requireTimestamp, isolationLevel, false, false, false); + return forConsumer(requireTimestamp, isolationLevel, false, false, false, false); } public static Builder forConsumer(boolean requireTimestamp, IsolationLevel isolationLevel, boolean requireMaxTimestamp, boolean requireEarliestLocalTimestamp, - boolean requireTieredStorageTimestamp) { + boolean requireTieredStorageTimestamp, + boolean requireEarliestPendingUploadTimestamp) { short minVersion = ApiKeys.LIST_OFFSETS.oldestVersion(); - if (requireTieredStorageTimestamp) + if (requireEarliestPendingUploadTimestamp) + minVersion = 11; + else if (requireTieredStorageTimestamp) minVersion = 9; else if (requireEarliestLocalTimestamp) minVersion = 8; diff --git a/clients/src/main/resources/common/message/ListOffsetsRequest.json b/clients/src/main/resources/common/message/ListOffsetsRequest.json index 6f8ff7d6cf935..1a2de6ca30a2f 100644 --- a/clients/src/main/resources/common/message/ListOffsetsRequest.json +++ b/clients/src/main/resources/common/message/ListOffsetsRequest.json @@ -40,7 +40,9 @@ // Version 9 enables listing offsets by last tiered offset (KIP-1005). // // Version 10 enables async remote list offsets support (KIP-1075) - "validVersions": "1-10", + // + // Version 11 enables listing offsets by earliest pending upload offset (KIP-1023) + "validVersions": "1-11", "flexibleVersions": "6+", "latestVersionUnstable": false, "fields": [ diff --git a/clients/src/main/resources/common/message/ListOffsetsResponse.json b/clients/src/main/resources/common/message/ListOffsetsResponse.json index 7f9588847b9a0..1407273bf4d8c 100644 --- a/clients/src/main/resources/common/message/ListOffsetsResponse.json +++ b/clients/src/main/resources/common/message/ListOffsetsResponse.json @@ -40,7 +40,9 @@ // Version 9 enables listing offsets by last tiered offset (KIP-1005). // // Version 10 enables async remote list offsets support (KIP-1075) - "validVersions": "1-10", + // + // Version 11 enables listing offsets by earliest pending upload offset (KIP-1023) + "validVersions": "1-11", "flexibleVersions": "6+", "fields": [ { "name": "ThrottleTimeMs", "type": "int32", "versions": "2+", "ignorable": true, diff --git a/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java b/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java index 1098078b582fc..e7fa11177d3ed 100644 --- a/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java +++ b/clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java @@ -551,7 +551,8 @@ public void testCloseAdminClient() { * Test if admin client can be closed in the callback invoked when * an api call completes. If calling {@link Admin#close()} in callback, AdminClient thread hangs */ - @Test @Timeout(10) + @Test + @Timeout(10) public void testCloseAdminClientInCallback() throws InterruptedException { MockTime time = new MockTime(); AdminClientUnitTestEnv env = new AdminClientUnitTestEnv(time, mockCluster(3, 0)); @@ -8729,6 +8730,34 @@ public void testListOffsetsLatestTierSpecSpecMinVersion() throws Exception { } } + @Test + public void testListOffsetsEarliestPendingUploadSpecSpecMinVersion() throws Exception { + Node node = new Node(0, "localhost", 8120); + List nodes = Collections.singletonList(node); + List pInfos = new ArrayList<>(); + pInfos.add(new PartitionInfo("foo", 0, node, new Node[]{node}, new Node[]{node})); + final Cluster cluster = new Cluster( + "mockClusterId", + nodes, + pInfos, + Collections.emptySet(), + Collections.emptySet(), + node); + final TopicPartition tp0 = new TopicPartition("foo", 0); + try (AdminClientUnitTestEnv env = new AdminClientUnitTestEnv(cluster, + AdminClientConfig.RETRIES_CONFIG, "2")) { + + env.kafkaClient().setNodeApiVersions(NodeApiVersions.create()); + env.kafkaClient().prepareResponse(prepareMetadataResponse(env.cluster(), Errors.NONE)); + + env.adminClient().listOffsets(Collections.singletonMap(tp0, OffsetSpec.earliestPendingUpload())); + + TestUtils.waitForCondition(() -> env.kafkaClient().requests().stream().anyMatch(request -> + request.requestBuilder().apiKey().messageType == ApiMessageType.LIST_OFFSETS && request.requestBuilder().oldestAllowedVersion() == 11 + ), "no listOffsets request has the expected oldestAllowedVersion"); + } + } + private Map makeTestFeatureUpdates() { return Utils.mkMap( Utils.mkEntry("test_feature_1", new FeatureUpdate((short) 2, FeatureUpdate.UpgradeType.UPGRADE)), @@ -11668,4 +11697,27 @@ private static StreamsGroupDescribeResponseData makeFullStreamsGroupDescribeResp .setAssignmentEpoch(1)); return data; } + + @Test + @Timeout(30) + public void testDescribeTopicsTimeoutWhenNoBrokerResponds() throws Exception { + try (AdminClientUnitTestEnv env = new AdminClientUnitTestEnv( + mockCluster(1, 0), + AdminClientConfig.RETRIES_CONFIG, "0", + AdminClientConfig.REQUEST_TIMEOUT_MS_CONFIG, "30000")) { + env.kafkaClient().setNodeApiVersions(NodeApiVersions.create()); + + // Not using prepareResponse is equivalent to "no brokers respond". + long start = System.currentTimeMillis(); + DescribeTopicsResult result = env.adminClient().describeTopics(List.of("test-topic"), new DescribeTopicsOptions().timeoutMs(200)); + Map> topicDescriptionMap = result.topicNameValues(); + KafkaFuture topicDescription = topicDescriptionMap.get("test-topic"); + ExecutionException exception = assertThrows(ExecutionException.class, topicDescription::get); + // Duration should be greater than or equal to 200 ms but less than 30000 ms. + long duration = System.currentTimeMillis() - start; + + assertInstanceOf(TimeoutException.class, exception.getCause()); + assertTrue(duration >= 150L && duration < 30000); + } + } } diff --git a/clients/src/test/java/org/apache/kafka/common/requests/ListOffsetsRequestTest.java b/clients/src/test/java/org/apache/kafka/common/requests/ListOffsetsRequestTest.java index 2cf4cbc00c969..48542c1a2fd7d 100644 --- a/clients/src/test/java/org/apache/kafka/common/requests/ListOffsetsRequestTest.java +++ b/clients/src/test/java/org/apache/kafka/common/requests/ListOffsetsRequestTest.java @@ -127,13 +127,16 @@ public void testListOffsetsRequestOldestVersion() { .forConsumer(false, IsolationLevel.READ_COMMITTED); ListOffsetsRequest.Builder maxTimestampRequestBuilder = ListOffsetsRequest.Builder - .forConsumer(false, IsolationLevel.READ_UNCOMMITTED, true, false, false); + .forConsumer(false, IsolationLevel.READ_UNCOMMITTED, true, false, false, false); ListOffsetsRequest.Builder requireEarliestLocalTimestampRequestBuilder = ListOffsetsRequest.Builder - .forConsumer(false, IsolationLevel.READ_UNCOMMITTED, false, true, false); + .forConsumer(false, IsolationLevel.READ_UNCOMMITTED, false, true, false, false); ListOffsetsRequest.Builder requireTieredStorageTimestampRequestBuilder = ListOffsetsRequest.Builder - .forConsumer(false, IsolationLevel.READ_UNCOMMITTED, false, false, true); + .forConsumer(false, IsolationLevel.READ_UNCOMMITTED, false, false, true, false); + + ListOffsetsRequest.Builder requireEarliestPendingUploadTimestampRequestBuilder = ListOffsetsRequest.Builder + .forConsumer(false, IsolationLevel.READ_UNCOMMITTED, false, false, false, true); assertEquals((short) 1, consumerRequestBuilder.oldestAllowedVersion()); assertEquals((short) 1, requireTimestampRequestBuilder.oldestAllowedVersion()); @@ -141,5 +144,6 @@ public void testListOffsetsRequestOldestVersion() { assertEquals((short) 7, maxTimestampRequestBuilder.oldestAllowedVersion()); assertEquals((short) 8, requireEarliestLocalTimestampRequestBuilder.oldestAllowedVersion()); assertEquals((short) 9, requireTieredStorageTimestampRequestBuilder.oldestAllowedVersion()); + assertEquals((short) 11, requireEarliestPendingUploadTimestampRequestBuilder.oldestAllowedVersion()); } } diff --git a/core/src/main/java/kafka/server/share/SharePartition.java b/core/src/main/java/kafka/server/share/SharePartition.java index a07f9a12fbb36..8ed094b85f1b0 100644 --- a/core/src/main/java/kafka/server/share/SharePartition.java +++ b/core/src/main/java/kafka/server/share/SharePartition.java @@ -260,10 +260,10 @@ enum SharePartitionState { private long endOffset; /** - * The initial read gap offset tracks if there are any gaps in the in-flight batch during initial - * read of the share partition state from the persister. + * The persister read result gap window tracks if there are any gaps in the in-flight batch during + * initial read of the share partition state from the persister. */ - private InitialReadGapOffset initialReadGapOffset; + private GapWindow persisterReadResultGapWindow; /** * We maintain the latest fetch offset and its metadata to estimate the minBytes requirement more efficiently. @@ -475,9 +475,9 @@ public CompletableFuture maybeInitialize() { // in the cached state are not missed updateFindNextFetchOffset(true); endOffset = cachedState.lastEntry().getValue().lastOffset(); - // initialReadGapOffset is not required, if there are no gaps in the read state response + // gapWindow is not required, if there are no gaps in the read state response if (gapStartOffset != -1) { - initialReadGapOffset = new InitialReadGapOffset(endOffset, gapStartOffset); + persisterReadResultGapWindow = new GapWindow(endOffset, gapStartOffset); } // In case the persister read state RPC result contains no AVAILABLE records, we can update cached state // and start/end offsets. @@ -561,18 +561,27 @@ public long nextFetchOffset() { } long nextFetchOffset = -1; - long gapStartOffset = isInitialReadGapOffsetWindowActive() ? initialReadGapOffset.gapStartOffset() : -1; + long gapStartOffset = isPersisterReadGapWindowActive() ? persisterReadResultGapWindow.gapStartOffset() : -1; for (Map.Entry entry : cachedState.entrySet()) { // Check if there exists any gap in the in-flight batch which needs to be fetched. If - // initialReadGapOffset's endOffset is equal to the share partition's endOffset, then + // gapWindow's endOffset is equal to the share partition's endOffset, then // only the initial gaps should be considered. Once share partition's endOffset is past // initial read end offset then all gaps are anyway fetched. - if (isInitialReadGapOffsetWindowActive()) { + if (isPersisterReadGapWindowActive()) { if (entry.getKey() > gapStartOffset) { nextFetchOffset = gapStartOffset; break; } - gapStartOffset = entry.getValue().lastOffset() + 1; + // If the gapStartOffset is already past the last offset of the in-flight batch, + // then do not consider this batch for finding the next fetch offset. For example, + // consider during initialization, the gapWindow is set to 5 and the + // first cached batch is 15-18. First read will happen at offset 5 and say the data + // fetched is [5-6], now next fetch offset should be 7. This works fine but say + // subsequent read returns batch 8-11, and the gapStartOffset will be 12. Without + // the max check, the next fetch offset returned will be 7 which is incorrect. + // The natural gaps for which no data is available shall be considered hence + // take the max of the gapStartOffset and the last offset of the in-flight batch. + gapStartOffset = Math.max(entry.getValue().lastOffset() + 1, gapStartOffset); } // Check if the state is maintained per offset or batch. If the offsetState @@ -699,16 +708,33 @@ public ShareAcquiredRecords acquire( // Find the floor batch record for the request batch. The request batch could be // for a subset of the in-flight batch i.e. cached batch of offset 10-14 and request batch - // of 12-13. Hence, floor entry is fetched to find the sub-map. + // of 12-13. Hence, floor entry is fetched to find the sub-map. Secondly, when the share + // partition is initialized with persisted state, the start offset might be moved to a later + // offset. In such case, the first batch base offset might be less than the start offset. Map.Entry floorEntry = cachedState.floorEntry(baseOffset); - // We might find a batch with floor entry but not necessarily that batch has an overlap, - // if the request batch base offset is ahead of last offset from floor entry i.e. cached - // batch of 10-14 and request batch of 15-18, though floor entry is found but no overlap. - // Such scenario will be handled in the next step when considering the subMap. However, - // if the floor entry is found and the request batch base offset is within the floor entry - // then adjust the base offset to the floor entry so that acquire method can still work on - // previously cached batch boundaries. - if (floorEntry != null && floorEntry.getValue().lastOffset() >= baseOffset) { + if (floorEntry == null) { + // The initialize method check that there couldn't be any batches prior to the start offset. + // And once share partition starts fetching records, it will always fetch records, at least, + // from the start offset, but there could be cases where the batch base offset is prior + // to the start offset. This can happen when the share partition is initialized with + // partial persisted state and moved start offset i.e. start offset is not the batch's + // first offset. In such case, we need to adjust the base offset to the start offset. + // It's safe to adjust the base offset to the start offset when there isn't any floor + // i.e. no cached batches available prior to the request batch base offset. Hence, + // check for the floor entry and adjust the base offset accordingly. + if (baseOffset < startOffset) { + log.info("Adjusting base offset for the fetch as it's prior to start offset: {}-{}" + + "from {} to {}", groupId, topicIdPartition, baseOffset, startOffset); + baseOffset = startOffset; + } + } else if (floorEntry.getValue().lastOffset() >= baseOffset) { + // We might find a batch with floor entry but not necessarily that batch has an overlap, + // if the request batch base offset is ahead of last offset from floor entry i.e. cached + // batch of 10-14 and request batch of 15-18, though floor entry is found but no overlap. + // Such scenario will be handled in the next step when considering the subMap. However, + // if the floor entry is found and the request batch base offset is within the floor entry + // then adjust the base offset to the floor entry so that acquire method can still work on + // previously cached batch boundaries. baseOffset = floorEntry.getKey(); } // Validate if the fetch records are already part of existing batches and if available. @@ -743,10 +769,10 @@ public ShareAcquiredRecords acquire( } InFlightBatch inFlightBatch = entry.getValue(); - // If the initialReadGapOffset window is active, we need to treat the gaps in between the window as + // If the gapWindow window is active, we need to treat the gaps in between the window as // acquirable. Once the window is inactive (when we have acquired all the gaps inside the window), // the remaining gaps are natural (data does not exist at those offsets) and we need not acquire them. - if (isInitialReadGapOffsetWindowActive()) { + if (isPersisterReadGapWindowActive()) { // If nextBatchStartOffset is less than the key of the entry, this means the fetch happened for a gap in the cachedState. // Thus, a new batch needs to be acquired for the gap. if (maybeGapStartOffset < entry.getKey()) { @@ -755,7 +781,8 @@ public ShareAcquiredRecords acquire( result.addAll(shareAcquiredRecords.acquiredRecords()); acquiredCount += shareAcquiredRecords.count(); } - // Set nextBatchStartOffset as the last offset of the current in-flight batch + 1 + // Set nextBatchStartOffset as the last offset of the current in-flight batch + 1. + // Hence, after the loop iteration the next gap can be considered. maybeGapStartOffset = inFlightBatch.lastOffset() + 1; // If the acquired count is equal to the max fetch records then break the loop. if (acquiredCount >= maxRecordsToAcquire) { @@ -831,7 +858,7 @@ public ShareAcquiredRecords acquire( acquiredCount += shareAcquiredRecords.count(); } if (!result.isEmpty()) { - maybeUpdateReadGapFetchOffset(result.get(result.size() - 1).lastOffset() + 1); + maybeUpdatePersisterGapWindowStartOffset(result.get(result.size() - 1).lastOffset() + 1); return maybeFilterAbortedTransactionalAcquiredRecords(fetchPartitionData, isolationLevel, new ShareAcquiredRecords(result, acquiredCount)); } return new ShareAcquiredRecords(result, acquiredCount); @@ -1057,10 +1084,24 @@ private Optional releaseAcquiredRecordsForCompleteBatch(String member /** * Updates the cached state, start and end offsets of the share partition as per the new log * start offset. The method is called when the log start offset is moved for the share partition. + *

+ * This method only archives the available records in the cached state that are before the new log + * start offset. It does not persist the archived state batches to the persister, rather it + * updates the cached state and offsets to reflect the new log start offset. The state in persister + * will be updated lazily during the acknowledge/release records API calls or acquisition lock timeout. + *

+ * The AVAILABLE state records can either have ongoing state transition or not. Hence, the archive + * records method will update the state of the records to ARCHIVED and set the terminal state flag + * hence if the transition is rolled back then the state will not be AVAILABLE again. However, + * the ACQUIRED state records will not be archived as they are still in-flight and acknowledge + * method also do not allow the state update for any offsets post the log start offset, hence those + * records will only be archived once acquisition lock timeout occurs. * * @param logStartOffset The new log start offset. */ void updateCacheAndOffsets(long logStartOffset) { + log.debug("Updating cached states for share partition: {}-{} with new log start offset: {}", + groupId, topicIdPartition, logStartOffset); lock.writeLock().lock(); try { if (logStartOffset <= startOffset) { @@ -1428,16 +1469,20 @@ private boolean initializedOrThrowException() { } // Method to reduce the window that tracks gaps in the cachedState - private void maybeUpdateReadGapFetchOffset(long offset) { + private void maybeUpdatePersisterGapWindowStartOffset(long offset) { lock.writeLock().lock(); try { - if (initialReadGapOffset != null) { - if (initialReadGapOffset.endOffset() == endOffset) { - initialReadGapOffset.gapStartOffset(offset); + if (persisterReadResultGapWindow != null) { + // When last cached batch for persister's read gap window is acquired, then endOffset is + // same as the gapWindow's endOffset, but the gap offset to update in the method call + // is endOffset + 1. Hence, do not update the gap start offset if the request offset + // is ahead of the endOffset. + if (persisterReadResultGapWindow.endOffset() == endOffset && offset <= persisterReadResultGapWindow.endOffset()) { + persisterReadResultGapWindow.gapStartOffset(offset); } else { - // The initial read gap offset is not valid anymore as the end offset has moved - // beyond the initial read gap offset. Hence, reset the initial read gap offset. - initialReadGapOffset = null; + // The persister's read gap window is not valid anymore as the end offset has moved + // beyond the read gap window's endOffset. Hence, set the gap window to null. + persisterReadResultGapWindow = null; } } } finally { @@ -1445,6 +1490,15 @@ private void maybeUpdateReadGapFetchOffset(long offset) { } } + /** + * The method calculates the last offset and maximum records to acquire. The adjustment is needed + * to ensure that the records acquired do not exceed the maximum in-flight messages limit. + * + * @param fetchOffset The offset from which the records are fetched. + * @param maxFetchRecords The maximum number of records to acquire. + * @param lastOffset The last offset to acquire records to, which is the last offset of the fetched batch. + * @return LastOffsetAndMaxRecords object, containing the last offset to acquire and the maximum records to acquire. + */ private LastOffsetAndMaxRecords lastOffsetAndMaxRecordsToAcquire(long fetchOffset, int maxFetchRecords, long lastOffset) { // There can always be records fetched exceeding the max in-flight messages limit. Hence, // we need to check if the share partition has reached the max in-flight messages limit @@ -1512,6 +1566,20 @@ private ShareAcquiredRecords acquireNewBatchRecords( // which falls under the max messages limit. As the max fetch records is the soft // limit, the last offset can be higher than the max messages. lastAcquiredOffset = lastOffsetFromBatchWithRequestOffset(batches, firstAcquiredOffset + maxFetchRecords - 1); + // If the initial read gap offset window is active then it's not guaranteed that the + // batches align on batch boundaries. Hence, reset to last offset itself if the batch's + // last offset is greater than the last offset for acquisition, else there could be + // a situation where the batch overlaps with the initial read gap offset window batch. + // For example, if the initial read gap offset window is 10-30 i.e. gapWindow's + // startOffset is 10 and endOffset is 30, and the first persister's read batch is 15-30. + // Say first fetched batch from log is 10-30 and maxFetchRecords is 1, then the lastOffset + // in this method call would be 14. As the maxFetchRecords is lesser than the batch, + // hence last batch offset for request offset is fetched. In this example it will + // be 30, hence check if the initial read gap offset window is active and the last acquired + // offset should be adjusted to 14 instead of 30. + if (isPersisterReadGapWindowActive() && lastAcquiredOffset > lastOffset) { + lastAcquiredOffset = lastOffset; + } } // Create batches of acquired records. @@ -1528,7 +1596,7 @@ private ShareAcquiredRecords acquireNewBatchRecords( if (lastAcquiredOffset > endOffset) { endOffset = lastAcquiredOffset; } - maybeUpdateReadGapFetchOffset(lastAcquiredOffset + 1); + maybeUpdatePersisterGapWindowStartOffset(lastAcquiredOffset + 1); return new ShareAcquiredRecords(acquiredRecords, (int) (lastAcquiredOffset - firstAcquiredOffset + 1)); } finally { lock.writeLock().unlock(); @@ -2135,15 +2203,15 @@ be removed once all the messages (0-99) are acknowledged (ACCEPT or REJECT). // If the lastOffsetAcknowledged is equal to the last offset of entry, then the entire batch can potentially be removed. if (lastOffsetAcknowledged == entry.getValue().lastOffset()) { startOffset = cachedState.higherKey(lastOffsetAcknowledged); - if (isInitialReadGapOffsetWindowActive()) { + if (isPersisterReadGapWindowActive()) { // This case will arise if we have a situation where there is an acquirable gap after the lastOffsetAcknowledged. // Ex, the cachedState has following state batches -> {(0, 10), (11, 20), (31,40)} and all these batches are acked. - // There is a gap from 21 to 30. Let the initialReadGapOffset.gapStartOffset be 21. In this case, + // There is a gap from 21 to 30. Let the gapWindow's gapStartOffset be 21. In this case, // lastOffsetAcknowledged will be 20, but we cannot simply move the start offset to the first offset // of next cachedState batch (next cachedState batch is 31 to 40). There is an acquirable gap in between (21 to 30) - // and The startOffset should be at 21. Hence, we set startOffset to the minimum of initialReadGapOffset.gapStartOffset + // and The startOffset should be at 21. Hence, we set startOffset to the minimum of gapWindow.gapStartOffset // and higher key of lastOffsetAcknowledged - startOffset = Math.min(initialReadGapOffset.gapStartOffset(), startOffset); + startOffset = Math.min(persisterReadResultGapWindow.gapStartOffset(), startOffset); } lastKeyToRemove = entry.getKey(); } else { @@ -2208,8 +2276,8 @@ boolean canMoveStartOffset() { return isRecordStateAcknowledged(startOffsetState); } - private boolean isInitialReadGapOffsetWindowActive() { - return initialReadGapOffset != null && initialReadGapOffset.endOffset() == endOffset; + private boolean isPersisterReadGapWindowActive() { + return persisterReadResultGapWindow != null && persisterReadResultGapWindow.endOffset() == endOffset; } /** @@ -2232,7 +2300,7 @@ long findLastOffsetAcknowledged() { for (NavigableMap.Entry entry : cachedState.entrySet()) { InFlightBatch inFlightBatch = entry.getValue(); - if (isInitialReadGapOffsetWindowActive() && inFlightBatch.lastOffset() >= initialReadGapOffset.gapStartOffset()) { + if (isPersisterReadGapWindowActive() && inFlightBatch.lastOffset() >= persisterReadResultGapWindow.gapStartOffset()) { return lastOffsetAcknowledged; } @@ -2797,8 +2865,8 @@ Timer timer() { } // Visible for testing - InitialReadGapOffset initialReadGapOffset() { - return initialReadGapOffset; + GapWindow persisterReadResultGapWindow() { + return persisterReadResultGapWindow; } // Visible for testing. @@ -2807,17 +2875,17 @@ Uuid fetchLock() { } /** - * The InitialReadGapOffset class is used to record the gap start and end offset of the probable gaps + * The GapWindow class is used to record the gap start and end offset of the probable gaps * of available records which are neither known to Persister nor to SharePartition. Share Partition * will use this information to determine the next fetch offset and should try to fetch the records * in the gap. */ // Visible for Testing - static class InitialReadGapOffset { + static class GapWindow { private final long endOffset; private long gapStartOffset; - InitialReadGapOffset(long endOffset, long gapStartOffset) { + GapWindow(long endOffset, long gapStartOffset) { this.endOffset = endOffset; this.gapStartOffset = gapStartOffset; } diff --git a/core/src/main/scala/kafka/coordinator/transaction/TransactionLog.scala b/core/src/main/scala/kafka/coordinator/transaction/TransactionLog.scala index 75baa98da1517..a3e9eacb66f25 100644 --- a/core/src/main/scala/kafka/coordinator/transaction/TransactionLog.scala +++ b/core/src/main/scala/kafka/coordinator/transaction/TransactionLog.scala @@ -115,7 +115,13 @@ object TransactionLog { val version = buffer.getShort if (version >= TransactionLogValue.LOWEST_SUPPORTED_VERSION && version <= TransactionLogValue.HIGHEST_SUPPORTED_VERSION) { val value = new TransactionLogValue(new ByteBufferAccessor(buffer), version) - val transactionMetadata = new TransactionMetadata( + val state = TransactionState.fromId(value.transactionStatus) + val tps: util.Set[TopicPartition] = new util.HashSet[TopicPartition]() + if (!state.equals(TransactionState.EMPTY)) + value.transactionPartitions.forEach(partitionsSchema => { + partitionsSchema.partitionIds.forEach(partitionId => tps.add(new TopicPartition(partitionsSchema.topic, partitionId.intValue()))) + }) + Some(new TransactionMetadata( transactionalId, value.producerId, value.previousProducerId, @@ -123,20 +129,11 @@ object TransactionLog { value.producerEpoch, RecordBatch.NO_PRODUCER_EPOCH, value.transactionTimeoutMs, - TransactionState.fromId(value.transactionStatus), - util.Set.of(), + state, + tps, value.transactionStartTimestampMs, value.transactionLastUpdateTimestampMs, - TransactionVersion.fromFeatureLevel(value.clientTransactionVersion)) - - if (!transactionMetadata.state.equals(TransactionState.EMPTY)) - value.transactionPartitions.forEach(partitionsSchema => { - transactionMetadata.addPartitions(partitionsSchema.partitionIds - .stream - .map(partitionId => new TopicPartition(partitionsSchema.topic, partitionId.intValue())) - .toList) - }) - Some(transactionMetadata) + TransactionVersion.fromFeatureLevel(value.clientTransactionVersion))) } else throw new IllegalStateException(s"Unknown version $version from the transaction log message value") } } diff --git a/core/src/main/scala/kafka/server/DynamicBrokerConfig.scala b/core/src/main/scala/kafka/server/DynamicBrokerConfig.scala index c64218246b396..124a4c7b78f4c 100755 --- a/core/src/main/scala/kafka/server/DynamicBrokerConfig.scala +++ b/core/src/main/scala/kafka/server/DynamicBrokerConfig.scala @@ -239,7 +239,7 @@ object DynamicBrokerConfig { } } val configHandler = new BrokerConfigHandler(config, quotaManagers) - configHandler.processConfigChanges("", dynamicPerBrokerConfigs) + configHandler.processConfigChanges("", dynamicDefaultConfigs) configHandler.processConfigChanges(config.brokerId.toString, dynamicPerBrokerConfigs) } } diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 496e50208db89..070b3e544a6e0 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -172,7 +172,8 @@ object ReplicaManager { ListOffsetsRequest.LATEST_TIMESTAMP -> 1.toShort, ListOffsetsRequest.MAX_TIMESTAMP -> 7.toShort, ListOffsetsRequest.EARLIEST_LOCAL_TIMESTAMP -> 8.toShort, - ListOffsetsRequest.LATEST_TIERED_TIMESTAMP -> 9.toShort + ListOffsetsRequest.LATEST_TIERED_TIMESTAMP -> 9.toShort, + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP -> 11.toShort ) def createLogReadResult(highWatermark: Long, @@ -788,7 +789,11 @@ class ReplicaManager(val config: KafkaConfig, hasCustomErrorMessage = customException.isDefined ) } - val entriesWithoutErrorsPerPartition = entriesPerPartition.filter { case (key, _) => !errorResults.contains(key) } + // In non-transaction paths, errorResults is typically empty, so we can + // directly use entriesPerPartition instead of creating a new filtered collection + val entriesWithoutErrorsPerPartition = + if (errorResults.nonEmpty) entriesPerPartition.filter { case (key, _) => !errorResults.contains(key) } + else entriesPerPartition val preAppendPartitionResponses = buildProducePartitionStatus(errorResults).map { case (k, status) => k -> status.responseStatus } diff --git a/core/src/test/java/kafka/server/share/SharePartitionTest.java b/core/src/test/java/kafka/server/share/SharePartitionTest.java index ba24f3b259579..47e214a716fe6 100644 --- a/core/src/test/java/kafka/server/share/SharePartitionTest.java +++ b/core/src/test/java/kafka/server/share/SharePartitionTest.java @@ -17,6 +17,7 @@ package kafka.server.share; import kafka.server.ReplicaManager; +import kafka.server.share.SharePartition.GapWindow; import kafka.server.share.SharePartition.SharePartitionState; import kafka.server.share.SharePartitionManager.SharePartitionListener; @@ -965,11 +966,11 @@ public void testMaybeInitializeStateBatchesWithGapAtBeginning() { assertEquals(3, sharePartition.cachedState().get(21L).batchDeliveryCount()); assertNull(sharePartition.cachedState().get(21L).offsetState()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(10, initialReadGapOffset.gapStartOffset()); - assertEquals(30, initialReadGapOffset.endOffset()); + assertEquals(10, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(30, persisterReadResultGapWindow.endOffset()); } @Test @@ -1010,11 +1011,11 @@ public void testMaybeInitializeStateBatchesWithMultipleGaps() { assertEquals(3, sharePartition.cachedState().get(30L).batchDeliveryCount()); assertNull(sharePartition.cachedState().get(30L).offsetState()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(10, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + assertEquals(10, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -1051,11 +1052,11 @@ public void testMaybeInitializeStateBatchesWithGapNotAtBeginning() { assertEquals(3, sharePartition.cachedState().get(30L).batchDeliveryCount()); assertNull(sharePartition.cachedState().get(30L).offsetState()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(21, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + assertEquals(21, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -1082,10 +1083,676 @@ public void testMaybeInitializeStateBatchesWithoutGaps() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(31, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); - // Since there are no gaps present in the readState response, initialReadGapOffset should be null - assertNull(initialReadGapOffset); + // Since there are no gaps present in the readState response, persisterReadResultGapWindow should be null + assertNull(persisterReadResultGapWindow); + } + + @Test + public void testMaybeInitializeAndAcquire() { + Persister persister = Mockito.mock(Persister.class); + ReadShareGroupStateResult readShareGroupStateResult = Mockito.mock(ReadShareGroupStateResult.class); + Mockito.when(readShareGroupStateResult.topicsData()).thenReturn(List.of( + new TopicData<>(TOPIC_ID_PARTITION.topicId(), List.of( + PartitionFactory.newPartitionAllData(0, 3, 10L, Errors.NONE.code(), Errors.NONE.message(), + List.of( + new PersisterStateBatch(15L, 18L, RecordState.AVAILABLE.id, (short) 2), + new PersisterStateBatch(20L, 22L, RecordState.ARCHIVED.id, (short) 2), + new PersisterStateBatch(26L, 30L, RecordState.AVAILABLE.id, (short) 3))))))); + Mockito.when(persister.readState(Mockito.any())).thenReturn(CompletableFuture.completedFuture(readShareGroupStateResult)); + SharePartition sharePartition = SharePartitionBuilder.builder().withPersister(persister).build(); + + CompletableFuture result = sharePartition.maybeInitialize(); + assertTrue(result.isDone()); + assertFalse(result.isCompletedExceptionally()); + + assertEquals(SharePartitionState.ACTIVE, sharePartition.partitionState()); + assertEquals(3, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.startOffset()); + assertEquals(30, sharePartition.endOffset()); + assertEquals(10, sharePartition.nextFetchOffset()); + + assertEquals(18, sharePartition.cachedState().get(15L).lastOffset()); + assertEquals(22, sharePartition.cachedState().get(20L).lastOffset()); + assertEquals(30, sharePartition.cachedState().get(26L).lastOffset()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(10L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + assertEquals(30L, sharePartition.persisterReadResultGapWindow().endOffset()); + + // Create a single batch record that covers the entire range from 10 to 30 of initial read gap. + // The records in the batch are from 10 to 49. + MemoryRecords records = memoryRecords(40, 10); + // Set max fetch records to 1, records will be acquired till the first gap is encountered. + List acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 1, + 10, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 5); + + assertArrayEquals(expectedAcquiredRecord(10, 14, 1).toArray(), acquiredRecordsList.toArray()); + assertEquals(15, sharePartition.nextFetchOffset()); + assertEquals(4, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.cachedState().get(10L).firstOffset()); + assertEquals(14, sharePartition.cachedState().get(10L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(10L).batchState()); + assertEquals(1, sharePartition.cachedState().get(10L).batchDeliveryCount()); + assertNull(sharePartition.cachedState().get(10L).offsetState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(15L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + + // Send the same batch again to acquire the next set of records. + acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 10, + 15, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 13); + + List expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(15, 18, 3)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(19, 19, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(23, 25, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(26, 30, 4)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(31, sharePartition.nextFetchOffset()); + assertEquals(6, sharePartition.cachedState().size()); + assertEquals(19, sharePartition.cachedState().get(19L).firstOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(19L).batchState()); + assertEquals(1, sharePartition.cachedState().get(19L).batchDeliveryCount()); + assertNull(sharePartition.cachedState().get(19L).offsetState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(23, sharePartition.cachedState().get(23L).firstOffset()); + assertEquals(25, sharePartition.cachedState().get(23L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(23L).batchState()); + assertEquals(1, sharePartition.cachedState().get(23L).batchDeliveryCount()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(26L).batchState()); + assertEquals(30L, sharePartition.endOffset()); + // As all the gaps are now filled, the persisterReadResultGapWindow should be null. + assertNull(sharePartition.persisterReadResultGapWindow()); + + // Now initial read gap is filled, so the complete batch can be acquired despite max fetch records being 1. + acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 1, + 31, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 19); + + assertArrayEquals(expectedAcquiredRecord(31, 49, 1).toArray(), acquiredRecordsList.toArray()); + assertEquals(50, sharePartition.nextFetchOffset()); + assertEquals(7, sharePartition.cachedState().size()); + assertEquals(31, sharePartition.cachedState().get(31L).firstOffset()); + assertEquals(49, sharePartition.cachedState().get(31L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(31L).batchState()); + assertEquals(1, sharePartition.cachedState().get(31L).batchDeliveryCount()); + assertNull(sharePartition.cachedState().get(31L).offsetState()); + assertEquals(49L, sharePartition.endOffset()); + } + + @Test + public void testMaybeInitializeAndAcquireWithHigherMaxFetchRecords() { + Persister persister = Mockito.mock(Persister.class); + ReadShareGroupStateResult readShareGroupStateResult = Mockito.mock(ReadShareGroupStateResult.class); + Mockito.when(readShareGroupStateResult.topicsData()).thenReturn(List.of( + new TopicData<>(TOPIC_ID_PARTITION.topicId(), List.of( + PartitionFactory.newPartitionAllData(0, 3, 10L, Errors.NONE.code(), Errors.NONE.message(), + List.of( + new PersisterStateBatch(15L, 18L, RecordState.AVAILABLE.id, (short) 2), + new PersisterStateBatch(20L, 22L, RecordState.ARCHIVED.id, (short) 2), + new PersisterStateBatch(26L, 30L, RecordState.AVAILABLE.id, (short) 3))))))); + Mockito.when(persister.readState(Mockito.any())).thenReturn(CompletableFuture.completedFuture(readShareGroupStateResult)); + SharePartition sharePartition = SharePartitionBuilder.builder().withPersister(persister).build(); + + CompletableFuture result = sharePartition.maybeInitialize(); + assertTrue(result.isDone()); + assertFalse(result.isCompletedExceptionally()); + + assertEquals(SharePartitionState.ACTIVE, sharePartition.partitionState()); + assertEquals(3, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.startOffset()); + assertEquals(30, sharePartition.endOffset()); + assertEquals(10, sharePartition.nextFetchOffset()); + + assertEquals(18, sharePartition.cachedState().get(15L).lastOffset()); + assertEquals(22, sharePartition.cachedState().get(20L).lastOffset()); + assertEquals(30, sharePartition.cachedState().get(26L).lastOffset()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(10L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + assertEquals(30L, sharePartition.persisterReadResultGapWindow().endOffset()); + + // Create a single batch record that covers the entire range from 10 to 30 of initial read gap. + // The records in the batch are from 10 to 49. + MemoryRecords records = memoryRecords(40, 10); + // Set max fetch records to 500, all records should be acquired. + List acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 500, + 10, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 37); + + List expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(10, 14, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(15, 18, 3)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(19, 19, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(23, 25, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(26, 30, 4)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(31, 49, 1)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(50, sharePartition.nextFetchOffset()); + assertEquals(7, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.cachedState().get(10L).firstOffset()); + assertEquals(14, sharePartition.cachedState().get(10L).lastOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).firstOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).lastOffset()); + assertEquals(23, sharePartition.cachedState().get(23L).firstOffset()); + assertEquals(25, sharePartition.cachedState().get(23L).lastOffset()); + assertEquals(31, sharePartition.cachedState().get(31L).firstOffset()); + assertEquals(49, sharePartition.cachedState().get(31L).lastOffset()); + + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(10L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(19L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(23L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(26L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(31L).batchState()); + assertEquals(49L, sharePartition.endOffset()); + // As all the gaps are now filled, the persisterReadResultGapWindow should be null. + assertNull(sharePartition.persisterReadResultGapWindow()); + } + + @Test + public void testMaybeInitializeAndAcquireWithFetchBatchLastOffsetWithinCachedBatch() { + Persister persister = Mockito.mock(Persister.class); + ReadShareGroupStateResult readShareGroupStateResult = Mockito.mock(ReadShareGroupStateResult.class); + Mockito.when(readShareGroupStateResult.topicsData()).thenReturn(List.of( + new TopicData<>(TOPIC_ID_PARTITION.topicId(), List.of( + PartitionFactory.newPartitionAllData(0, 3, 10L, Errors.NONE.code(), Errors.NONE.message(), + List.of( + new PersisterStateBatch(15L, 18L, RecordState.AVAILABLE.id, (short) 2), + new PersisterStateBatch(20L, 22L, RecordState.ARCHIVED.id, (short) 2), + new PersisterStateBatch(26L, 30L, RecordState.AVAILABLE.id, (short) 3))))))); + Mockito.when(persister.readState(Mockito.any())).thenReturn(CompletableFuture.completedFuture(readShareGroupStateResult)); + SharePartition sharePartition = SharePartitionBuilder.builder().withPersister(persister).build(); + + CompletableFuture result = sharePartition.maybeInitialize(); + assertTrue(result.isDone()); + assertFalse(result.isCompletedExceptionally()); + + assertEquals(SharePartitionState.ACTIVE, sharePartition.partitionState()); + assertEquals(3, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.startOffset()); + assertEquals(30, sharePartition.endOffset()); + assertEquals(10, sharePartition.nextFetchOffset()); + + assertEquals(18, sharePartition.cachedState().get(15L).lastOffset()); + assertEquals(22, sharePartition.cachedState().get(20L).lastOffset()); + assertEquals(30, sharePartition.cachedState().get(26L).lastOffset()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(10L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + assertEquals(30L, sharePartition.persisterReadResultGapWindow().endOffset()); + + // Create a single batch record that ends in between the cached batch and the fetch offset is + // post startOffset. + MemoryRecords records = memoryRecords(16, 12); + // Set max fetch records to 500, records should be acquired till the last offset of the fetched batch. + List acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 500, + 10, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 13); + + List expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(12, 14, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(15, 18, 3)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(19, 19, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(23, 25, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecords(26, 27, 4)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(28, sharePartition.nextFetchOffset()); + assertEquals(6, sharePartition.cachedState().size()); + assertEquals(12, sharePartition.cachedState().get(12L).firstOffset()); + assertEquals(14, sharePartition.cachedState().get(12L).lastOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).firstOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).lastOffset()); + assertEquals(23, sharePartition.cachedState().get(23L).firstOffset()); + assertEquals(25, sharePartition.cachedState().get(23L).lastOffset()); + + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(12L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(19L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(23L).batchState()); + assertThrows(IllegalStateException.class, () -> sharePartition.cachedState().get(26L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(26L).offsetState().get(26L).state()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(26L).offsetState().get(27L).state()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).offsetState().get(28L).state()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).offsetState().get(29L).state()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).offsetState().get(30L).state()); + assertEquals(30L, sharePartition.endOffset()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(28L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + } + + @Test + public void testMaybeInitializeAndAcquireWithFetchBatchPriorStartOffset() { + Persister persister = Mockito.mock(Persister.class); + ReadShareGroupStateResult readShareGroupStateResult = Mockito.mock(ReadShareGroupStateResult.class); + Mockito.when(readShareGroupStateResult.topicsData()).thenReturn(List.of( + new TopicData<>(TOPIC_ID_PARTITION.topicId(), List.of( + PartitionFactory.newPartitionAllData(0, 3, 10L, Errors.NONE.code(), Errors.NONE.message(), + List.of( + new PersisterStateBatch(15L, 18L, RecordState.AVAILABLE.id, (short) 2), + new PersisterStateBatch(20L, 22L, RecordState.ARCHIVED.id, (short) 2), + new PersisterStateBatch(26L, 30L, RecordState.AVAILABLE.id, (short) 3))))))); + Mockito.when(persister.readState(Mockito.any())).thenReturn(CompletableFuture.completedFuture(readShareGroupStateResult)); + SharePartition sharePartition = SharePartitionBuilder.builder().withPersister(persister).build(); + + CompletableFuture result = sharePartition.maybeInitialize(); + assertTrue(result.isDone()); + assertFalse(result.isCompletedExceptionally()); + + assertEquals(SharePartitionState.ACTIVE, sharePartition.partitionState()); + assertEquals(3, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.startOffset()); + assertEquals(30, sharePartition.endOffset()); + assertEquals(10, sharePartition.nextFetchOffset()); + + assertEquals(18, sharePartition.cachedState().get(15L).lastOffset()); + assertEquals(22, sharePartition.cachedState().get(20L).lastOffset()); + assertEquals(30, sharePartition.cachedState().get(26L).lastOffset()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(10L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + assertEquals(30L, sharePartition.persisterReadResultGapWindow().endOffset()); + + // Create a single batch record where first offset is prior startOffset. + MemoryRecords records = memoryRecords(16, 6); + // Set max fetch records to 500, records should be acquired till the last offset of the fetched batch. + List acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 500, + 10, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 10); + + List expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(10, 14, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(15, 18, 3)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(19, 19, 1)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(23, sharePartition.nextFetchOffset()); + assertEquals(5, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.cachedState().get(10L).firstOffset()); + assertEquals(14, sharePartition.cachedState().get(10L).lastOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).firstOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).lastOffset()); + + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(10L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(19L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertEquals(30L, sharePartition.endOffset()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(20L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + } + + @Test + public void testMaybeInitializeAndAcquireWithMultipleBatches() { + Persister persister = Mockito.mock(Persister.class); + ReadShareGroupStateResult readShareGroupStateResult = Mockito.mock(ReadShareGroupStateResult.class); + Mockito.when(readShareGroupStateResult.topicsData()).thenReturn(List.of( + new TopicData<>(TOPIC_ID_PARTITION.topicId(), List.of( + PartitionFactory.newPartitionAllData(0, 3, 5L, Errors.NONE.code(), Errors.NONE.message(), + List.of( + new PersisterStateBatch(15L, 18L, RecordState.AVAILABLE.id, (short) 2), + new PersisterStateBatch(20L, 22L, RecordState.ARCHIVED.id, (short) 2), + new PersisterStateBatch(26L, 30L, RecordState.AVAILABLE.id, (short) 3))))))); + Mockito.when(persister.readState(Mockito.any())).thenReturn(CompletableFuture.completedFuture(readShareGroupStateResult)); + SharePartition sharePartition = SharePartitionBuilder.builder().withPersister(persister).build(); + + CompletableFuture result = sharePartition.maybeInitialize(); + assertTrue(result.isDone()); + assertFalse(result.isCompletedExceptionally()); + + assertEquals(SharePartitionState.ACTIVE, sharePartition.partitionState()); + assertEquals(3, sharePartition.cachedState().size()); + assertEquals(5, sharePartition.startOffset()); + assertEquals(30, sharePartition.endOffset()); + assertEquals(5, sharePartition.nextFetchOffset()); + + assertEquals(18, sharePartition.cachedState().get(15L).lastOffset()); + assertEquals(22, sharePartition.cachedState().get(20L).lastOffset()); + assertEquals(30, sharePartition.cachedState().get(26L).lastOffset()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(5L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + assertEquals(30L, sharePartition.persisterReadResultGapWindow().endOffset()); + + // Create multiple batch records that covers the entire range from 5 to 30 of initial read gap. + // The records in the batch are from 5 to 49. + ByteBuffer buffer = ByteBuffer.allocate(4096); + memoryRecordsBuilder(buffer, 2, 5).close(); + memoryRecordsBuilder(buffer, 1, 8).close(); + memoryRecordsBuilder(buffer, 2, 10).close(); + memoryRecordsBuilder(buffer, 6, 13).close(); + memoryRecordsBuilder(buffer, 3, 19).close(); + memoryRecordsBuilder(buffer, 9, 22).close(); + memoryRecordsBuilder(buffer, 19, 31).close(); + buffer.flip(); + MemoryRecords records = MemoryRecords.readableRecords(buffer); + // Set max fetch records to 1, records will be acquired till the first gap is encountered. + List acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 1, + 5L, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 2); + + assertArrayEquals(expectedAcquiredRecord(5, 6, 1).toArray(), acquiredRecordsList.toArray()); + assertEquals(7, sharePartition.nextFetchOffset()); + assertEquals(4, sharePartition.cachedState().size()); + assertEquals(5, sharePartition.cachedState().get(5L).firstOffset()); + assertEquals(6, sharePartition.cachedState().get(5L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(5L).batchState()); + assertEquals(1, sharePartition.cachedState().get(5L).batchDeliveryCount()); + assertNull(sharePartition.cachedState().get(5L).offsetState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(7L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + + // Remove first batch from the records as the fetch offset has moved forward to 7 offset. + List batch = TestUtils.toList(records.batches()); + records = records.slice(batch.get(0).sizeInBytes(), records.sizeInBytes() - batch.get(0).sizeInBytes()); + // Send the batch again to acquire the next set of records. + acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 3, + 7L, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 4); + + assertArrayEquals(expectedAcquiredRecord(8, 11, 1).toArray(), acquiredRecordsList.toArray()); + assertEquals(12, sharePartition.nextFetchOffset()); + assertEquals(5, sharePartition.cachedState().size()); + assertEquals(8, sharePartition.cachedState().get(8L).firstOffset()); + assertEquals(11, sharePartition.cachedState().get(8L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(8L).batchState()); + assertEquals(1, sharePartition.cachedState().get(8L).batchDeliveryCount()); + assertNull(sharePartition.cachedState().get(8L).offsetState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertEquals(30L, sharePartition.endOffset()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(12L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + + // Remove the next 2 batches from the records as the fetch offset has moved forward to 12 offset. + int size = batch.get(1).sizeInBytes() + batch.get(2).sizeInBytes(); + records = records.slice(size, records.sizeInBytes() - size); + // Send the records with 8 as max fetch records to acquire new and existing cached batches. + acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 8, + 12, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 10); + + List expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(13, 14, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(15, 18, 3)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(19, 19, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(23, 25, 1)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(26, sharePartition.nextFetchOffset()); + assertEquals(8, sharePartition.cachedState().size()); + assertEquals(13, sharePartition.cachedState().get(13L).firstOffset()); + assertEquals(14, sharePartition.cachedState().get(13L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(13L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(15L).batchState()); + assertEquals(19, sharePartition.cachedState().get(19L).firstOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(19L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(23, sharePartition.cachedState().get(23L).firstOffset()); + assertEquals(25, sharePartition.cachedState().get(23L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(23L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertEquals(30L, sharePartition.endOffset()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(26L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + + // Remove the next 2 batches from the records as the fetch offset has moved forward to 26 offset. + // Do not remove the 5th batch as it's only partially acquired. + size = batch.get(3).sizeInBytes() + batch.get(4).sizeInBytes(); + records = records.slice(size, records.sizeInBytes() - size); + // Send the records with 10 as max fetch records to acquire the existing and till end of the + // fetched data. + acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 10, + 26, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 24); + + expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(26, 30, 4)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(31, 49, 1)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(50, sharePartition.nextFetchOffset()); + assertEquals(9, sharePartition.cachedState().size()); + assertEquals(31, sharePartition.cachedState().get(31L).firstOffset()); + assertEquals(49, sharePartition.cachedState().get(31L).lastOffset()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(31L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(26L).batchState()); + assertEquals(49L, sharePartition.endOffset()); + // As all the gaps are now filled, the persisterReadResultGapWindow should be null. + assertNull(sharePartition.persisterReadResultGapWindow()); + } + + @Test + public void testMaybeInitializeAndAcquireWithMultipleBatchesAndLastOffsetWithinCachedBatch() { + Persister persister = Mockito.mock(Persister.class); + ReadShareGroupStateResult readShareGroupStateResult = Mockito.mock(ReadShareGroupStateResult.class); + Mockito.when(readShareGroupStateResult.topicsData()).thenReturn(List.of( + new TopicData<>(TOPIC_ID_PARTITION.topicId(), List.of( + PartitionFactory.newPartitionAllData(0, 3, 5L, Errors.NONE.code(), Errors.NONE.message(), + List.of( + new PersisterStateBatch(15L, 18L, RecordState.AVAILABLE.id, (short) 2), + new PersisterStateBatch(20L, 22L, RecordState.ARCHIVED.id, (short) 2), + new PersisterStateBatch(26L, 30L, RecordState.AVAILABLE.id, (short) 3))))))); + Mockito.when(persister.readState(Mockito.any())).thenReturn(CompletableFuture.completedFuture(readShareGroupStateResult)); + SharePartition sharePartition = SharePartitionBuilder.builder().withPersister(persister).build(); + + CompletableFuture result = sharePartition.maybeInitialize(); + assertTrue(result.isDone()); + assertFalse(result.isCompletedExceptionally()); + + assertEquals(SharePartitionState.ACTIVE, sharePartition.partitionState()); + assertEquals(3, sharePartition.cachedState().size()); + assertEquals(5, sharePartition.startOffset()); + assertEquals(30, sharePartition.endOffset()); + assertEquals(5, sharePartition.nextFetchOffset()); + + assertEquals(18, sharePartition.cachedState().get(15L).lastOffset()); + assertEquals(22, sharePartition.cachedState().get(20L).lastOffset()); + assertEquals(30, sharePartition.cachedState().get(26L).lastOffset()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(5L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + assertEquals(30L, sharePartition.persisterReadResultGapWindow().endOffset()); + + // Create multiple batch records that ends in between the cached batch and the fetch offset is + // post startOffset. + ByteBuffer buffer = ByteBuffer.allocate(4096); + memoryRecordsBuilder(buffer, 2, 7).close(); + memoryRecordsBuilder(buffer, 2, 10).close(); + memoryRecordsBuilder(buffer, 6, 13).close(); + // Though 19 offset is a gap but still be acquired. + memoryRecordsBuilder(buffer, 8, 20).close(); + buffer.flip(); + MemoryRecords records = MemoryRecords.readableRecords(buffer); + // Set max fetch records to 500, records should be acquired till the last offset of the fetched batch. + List acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 500, + 5, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 18); + + List expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(7, 14, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(15, 18, 3)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(19, 19, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(23, 25, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecords(26, 27, 4)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(28, sharePartition.nextFetchOffset()); + assertEquals(6, sharePartition.cachedState().size()); + assertEquals(7, sharePartition.cachedState().get(7L).firstOffset()); + assertEquals(14, sharePartition.cachedState().get(7L).lastOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).firstOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).lastOffset()); + assertEquals(23, sharePartition.cachedState().get(23L).firstOffset()); + assertEquals(25, sharePartition.cachedState().get(23L).lastOffset()); + + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(7L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(19L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(23L).batchState()); + assertThrows(IllegalStateException.class, () -> sharePartition.cachedState().get(26L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(26L).offsetState().get(26L).state()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(26L).offsetState().get(27L).state()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).offsetState().get(28L).state()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).offsetState().get(29L).state()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).offsetState().get(30L).state()); + assertEquals(30L, sharePartition.endOffset()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(28L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + } + + @Test + public void testMaybeInitializeAndAcquireWithMultipleBatchesPriorStartOffset() { + Persister persister = Mockito.mock(Persister.class); + ReadShareGroupStateResult readShareGroupStateResult = Mockito.mock(ReadShareGroupStateResult.class); + Mockito.when(readShareGroupStateResult.topicsData()).thenReturn(List.of( + new TopicData<>(TOPIC_ID_PARTITION.topicId(), List.of( + PartitionFactory.newPartitionAllData(0, 3, 10L, Errors.NONE.code(), Errors.NONE.message(), + List.of( + new PersisterStateBatch(15L, 18L, RecordState.AVAILABLE.id, (short) 2), + new PersisterStateBatch(20L, 22L, RecordState.ARCHIVED.id, (short) 2), + new PersisterStateBatch(26L, 30L, RecordState.AVAILABLE.id, (short) 3))))))); + Mockito.when(persister.readState(Mockito.any())).thenReturn(CompletableFuture.completedFuture(readShareGroupStateResult)); + SharePartition sharePartition = SharePartitionBuilder.builder().withPersister(persister).build(); + + CompletableFuture result = sharePartition.maybeInitialize(); + assertTrue(result.isDone()); + assertFalse(result.isCompletedExceptionally()); + + assertEquals(SharePartitionState.ACTIVE, sharePartition.partitionState()); + assertEquals(3, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.startOffset()); + assertEquals(30, sharePartition.endOffset()); + assertEquals(10, sharePartition.nextFetchOffset()); + + assertEquals(18, sharePartition.cachedState().get(15L).lastOffset()); + assertEquals(22, sharePartition.cachedState().get(20L).lastOffset()); + assertEquals(30, sharePartition.cachedState().get(26L).lastOffset()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(10L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); + assertEquals(30L, sharePartition.persisterReadResultGapWindow().endOffset()); + + // Create multiple batch records where multiple batches base offsets are prior startOffset. + ByteBuffer buffer = ByteBuffer.allocate(4096); + memoryRecordsBuilder(buffer, 2, 3).close(); + memoryRecordsBuilder(buffer, 1, 6).close(); + memoryRecordsBuilder(buffer, 4, 8).close(); + memoryRecordsBuilder(buffer, 10, 13).close(); + buffer.flip(); + MemoryRecords records = MemoryRecords.readableRecords(buffer); + // Set max fetch records to 500, records should be acquired till the last offset of the fetched batch. + List acquiredRecordsList = fetchAcquiredRecords(sharePartition.acquire( + MEMBER_ID, + BATCH_SIZE, + 500, + 10, + fetchPartitionData(records), + FETCH_ISOLATION_HWM), + 10); + + List expectedAcquiredRecords = new ArrayList<>(expectedAcquiredRecord(10, 14, 1)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(15, 18, 3)); + expectedAcquiredRecords.addAll(expectedAcquiredRecord(19, 19, 1)); + + assertArrayEquals(expectedAcquiredRecords.toArray(), acquiredRecordsList.toArray()); + assertEquals(23, sharePartition.nextFetchOffset()); + assertEquals(5, sharePartition.cachedState().size()); + assertEquals(10, sharePartition.cachedState().get(10L).firstOffset()); + assertEquals(14, sharePartition.cachedState().get(10L).lastOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).firstOffset()); + assertEquals(19, sharePartition.cachedState().get(19L).lastOffset()); + + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(10L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(15L).batchState()); + assertEquals(RecordState.ACQUIRED, sharePartition.cachedState().get(19L).batchState()); + assertEquals(RecordState.ARCHIVED, sharePartition.cachedState().get(20L).batchState()); + assertEquals(RecordState.AVAILABLE, sharePartition.cachedState().get(26L).batchState()); + assertEquals(30L, sharePartition.endOffset()); + assertNotNull(sharePartition.persisterReadResultGapWindow()); + assertEquals(20L, sharePartition.persisterReadResultGapWindow().gapStartOffset()); } @Test @@ -2368,12 +3035,12 @@ public void testAcquireGapAtBeginningAndRecordsFetchedFromGap() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(16, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // After records are acquired, the initialReadGapOffset should be updated - assertEquals(16, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + // After records are acquired, the persisterReadResultGapWindow should be updated + assertEquals(16, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -2407,12 +3074,12 @@ public void testAcquireGapAtBeginningAndFetchedRecordsOverlapInFlightBatches() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(41, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // After records are acquired, the initialReadGapOffset should be updated - assertEquals(21, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + // After records are acquired, the persisterReadResultGapWindow should be updated + assertEquals(21, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -2460,12 +3127,12 @@ public void testAcquireGapAtBeginningAndFetchedRecordsOverlapInFlightAvailableBa assertEquals(3, sharePartition.stateEpoch()); assertEquals(26, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // After records are acquired, the initialReadGapOffset should be updated - assertEquals(26, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + // After records are acquired, the persisterReadResultGapWindow should be updated + assertEquals(26, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -2504,12 +3171,12 @@ public void testAcquireWhenCachedStateContainsGapsAndRecordsFetchedFromNonGapOff assertEquals(3, sharePartition.stateEpoch()); assertEquals(26, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // After records are acquired, the initialReadGapOffset should be updated - assertEquals(26, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + // After records are acquired, the persisterReadResultGapWindow should be updated + assertEquals(26, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -2561,12 +3228,12 @@ public void testAcquireGapAtBeginningAndFetchedRecordsOverlapMultipleInFlightBat assertEquals(3, sharePartition.stateEpoch()); assertEquals(86, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // After records are acquired, the initialReadGapOffset should be updated - assertEquals(86, initialReadGapOffset.gapStartOffset()); - assertEquals(90, initialReadGapOffset.endOffset()); + // After records are acquired, the persisterReadResultGapWindow should be updated + assertEquals(86, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(90, persisterReadResultGapWindow.endOffset()); } @Test @@ -2605,12 +3272,12 @@ public void testAcquireGapAtBeginningAndFetchedRecordsEndJustBeforeGap() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(31, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // After records are acquired, the initialReadGapOffset should be updated - assertEquals(31, initialReadGapOffset.gapStartOffset()); - assertEquals(70, initialReadGapOffset.endOffset()); + // After records are acquired, the persisterReadResultGapWindow should be updated + assertEquals(31, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(70, persisterReadResultGapWindow.endOffset()); } @Test @@ -2656,12 +3323,12 @@ public void testAcquireGapAtBeginningAndFetchedRecordsIncludeGapOffsetsAtEnd() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(76, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // After records are acquired, the initialReadGapOffset should be updated - assertEquals(76, initialReadGapOffset.gapStartOffset()); - assertEquals(90, initialReadGapOffset.endOffset()); + // After records are acquired, the persisterReadResultGapWindow should be updated + assertEquals(76, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(90, persisterReadResultGapWindow.endOffset()); } @@ -2709,11 +3376,11 @@ public void testAcquireWhenRecordsFetchedFromGapAndMaxFetchRecordsIsExceeded() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(27, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(27, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + assertEquals(27, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -2758,11 +3425,11 @@ public void testAcquireMaxFetchRecordsExceededAfterAcquiringGaps() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(21, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(21, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + assertEquals(21, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -2807,11 +3474,11 @@ public void testAcquireMaxFetchRecordsExceededBeforeAcquiringGaps() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(21, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(21, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + assertEquals(21, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -2859,8 +3526,8 @@ public void testAcquireWhenRecordsFetchedFromGapAndPartitionContainsNaturalGaps( assertEquals(3, sharePartition.stateEpoch()); assertEquals(51, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNull(persisterReadResultGapWindow); } @Test @@ -2903,8 +3570,8 @@ public void testAcquireCachedStateInitialGapMatchesWithActualPartitionGap() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(61, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNull(persisterReadResultGapWindow); } @Test @@ -2949,8 +3616,8 @@ public void testAcquireCachedStateInitialGapOverlapsWithActualPartitionGap() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(61, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNull(persisterReadResultGapWindow); } @Test @@ -2998,8 +3665,8 @@ public void testAcquireCachedStateGapInBetweenOverlapsWithActualPartitionGap() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(61, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNull(persisterReadResultGapWindow); } @Test @@ -3039,11 +3706,11 @@ public void testAcquireWhenRecordsFetchedAfterGapsAreFetched() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(41, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(31, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + assertEquals(31, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); // Fetching from the nextFetchOffset so that endOffset moves ahead records = memoryRecords(15, 41); @@ -3059,9 +3726,9 @@ public void testAcquireWhenRecordsFetchedAfterGapsAreFetched() { assertEquals(3, sharePartition.stateEpoch()); assertEquals(56, sharePartition.nextFetchOffset()); - // Since the endOffset is now moved ahead, the initialReadGapOffset should be empty - initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNull(initialReadGapOffset); + // Since the endOffset is now moved ahead, the persisterReadResultGapWindow should be empty + persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNull(persisterReadResultGapWindow); } @Test @@ -6116,11 +6783,11 @@ public void testMaybeUpdateCachedStateGapAfterLastOffsetAcknowledged() { assertEquals(40, sharePartition.endOffset()); assertEquals(21, sharePartition.nextFetchOffset()); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - assertEquals(21, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + assertEquals(21, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); } @Test @@ -6690,16 +7357,16 @@ public void testFindLastOffsetAcknowledgedWhenGapAtBeginning() { sharePartition.maybeInitialize(); - SharePartition.InitialReadGapOffset initialReadGapOffset = sharePartition.initialReadGapOffset(); - assertNotNull(initialReadGapOffset); + GapWindow persisterReadResultGapWindow = sharePartition.persisterReadResultGapWindow(); + assertNotNull(persisterReadResultGapWindow); - // Since there is a gap in the beginning, the initialReadGapOffset window is same as the cachedState - assertEquals(11, initialReadGapOffset.gapStartOffset()); - assertEquals(40, initialReadGapOffset.endOffset()); + // Since there is a gap in the beginning, the persisterReadResultGapWindow window is same as the cachedState + assertEquals(11, persisterReadResultGapWindow.gapStartOffset()); + assertEquals(40, persisterReadResultGapWindow.endOffset()); long lastOffsetAcknowledged = sharePartition.findLastOffsetAcknowledged(); - // Since the initialReadGapOffset window begins at startOffset, we cannot count any of the offsets as acknowledged. + // Since the persisterReadResultGapWindow window begins at startOffset, we cannot count any of the offsets as acknowledged. // Thus, lastOffsetAcknowledged should be -1 assertEquals(-1, lastOffsetAcknowledged); } diff --git a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala index 75447e04df33d..170ee3679f47b 100644 --- a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala +++ b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala @@ -1100,9 +1100,13 @@ class DynamicBrokerReconfigurationTest extends QuorumTestHarness with SaslSetup @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedGroupProtocolNames) @MethodSource(Array("getTestGroupProtocolParametersAll")) def testServersCanStartWithInvalidStaticConfigsAndValidDynamicConfigs(groupProtocol: String): Unit = { + TestNumReplicaFetcherMetricsReporter.testReporters.clear() + // modify snapshot interval config to explicitly take snapshot on a broker with valid dynamic configs val props = defaultStaticConfig(numServers) props.put(MetadataLogConfig.METADATA_SNAPSHOT_MAX_INTERVAL_MS_CONFIG, "10000") + props.put(MetricConfigs.METRIC_REPORTER_CLASSES_CONFIG, classOf[TestNumReplicaFetcherMetricsReporter].getName) + props.put(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG, "1") val kafkaConfig = KafkaConfig.fromProps(props) val newBroker = createBroker(kafkaConfig).asInstanceOf[BrokerServer] @@ -1110,6 +1114,15 @@ class DynamicBrokerReconfigurationTest extends QuorumTestHarness with SaslSetup alterSslKeystoreUsingConfigCommand(sslProperties1, listenerPrefix(SecureExternal)) + // Add num.replica.fetchers to the cluster-level config. + val clusterLevelProps = new Properties + clusterLevelProps.put(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG, "2") + reconfigureServers(clusterLevelProps, perBrokerConfig = false, (ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG, "2")) + + // Wait for the metrics reporter to be configured + val initialReporter = TestNumReplicaFetcherMetricsReporter.waitForReporters(1).head + initialReporter.verifyState(reconfigureCount = 1, numFetcher = 2) + TestUtils.ensureConsistentKRaftMetadata(servers, controllerServer) TestUtils.waitUntilTrue( @@ -1122,11 +1135,19 @@ class DynamicBrokerReconfigurationTest extends QuorumTestHarness with SaslSetup newBroker.shutdown() newBroker.awaitShutdown() + // Clean up the test reporter + TestNumReplicaFetcherMetricsReporter.testReporters.clear() + val invalidStaticConfigs = defaultStaticConfig(newBroker.config.brokerId) invalidStaticConfigs.putAll(securityProps(invalidSslConfigs, KEYSTORE_PROPS, listenerPrefix(SecureExternal))) newBroker.config.updateCurrentConfig(KafkaConfig.fromProps(invalidStaticConfigs)) newBroker.startup() + + // Verify that the custom MetricsReporter is not reconfigured after restart. + // If readDynamicBrokerConfigsFromSnapshot works correctly, the reporter should maintain its state. + val reporterAfterRestart = TestNumReplicaFetcherMetricsReporter.waitForReporters(1).head + reporterAfterRestart.verifyState(reconfigureCount = 0, numFetcher = 2) } private def awaitInitialPositions(consumer: Consumer[_, _]): Unit = { @@ -1635,6 +1656,64 @@ class TestMetricsReporter extends MetricsReporter with Reconfigurable with Close } } +object TestNumReplicaFetcherMetricsReporter { + val testReporters = new ConcurrentLinkedQueue[TestNumReplicaFetcherMetricsReporter]() + + def waitForReporters(count: Int): List[TestNumReplicaFetcherMetricsReporter] = { + TestUtils.waitUntilTrue(() => testReporters.size == count, msg = "Metrics reporters size not matched. Expected: " + count + ", actual: " + testReporters.size()) + + val reporters = testReporters.asScala.toList + TestUtils.waitUntilTrue(() => reporters.forall(_.configureCount == 1), msg = "Metrics reporters not configured") + reporters + } +} + + +class TestNumReplicaFetcherMetricsReporter extends MetricsReporter { + import TestNumReplicaFetcherMetricsReporter._ + @volatile var configureCount = 0 + @volatile var reconfigureCount = 0 + @volatile var numFetchers: Int = 1 + testReporters.add(this) + + override def init(metrics: util.List[KafkaMetric]): Unit = { + } + + override def configure(configs: util.Map[String, _]): Unit = { + configureCount += 1 + numFetchers = configs.get(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG).toString.toInt + } + + override def metricChange(metric: KafkaMetric): Unit = { + } + + override def metricRemoval(metric: KafkaMetric): Unit = { + } + + override def reconfigurableConfigs(): util.Set[String] = { + util.Set.of(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG) + } + + override def validateReconfiguration(configs: util.Map[String, _]): Unit = { + val numFetchers = configs.get(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG).toString.toInt + if (numFetchers <= 0) + throw new ConfigException(s"Invalid num.replica.fetchers $numFetchers") + } + + override def reconfigure(configs: util.Map[String, _]): Unit = { + reconfigureCount += 1 + numFetchers = configs.get(ReplicationConfigs.NUM_REPLICA_FETCHERS_CONFIG).toString.toInt + } + + override def close(): Unit = { + } + + def verifyState(reconfigureCount: Int, numFetcher: Int = 1): Unit = { + assertEquals(reconfigureCount, this.reconfigureCount) + assertEquals(numFetcher, this.numFetchers) + } +} + class MockFileConfigProvider extends FileConfigProvider { @throws(classOf[IOException]) diff --git a/core/src/test/scala/unit/kafka/log/UnifiedLogTest.scala b/core/src/test/scala/unit/kafka/log/UnifiedLogTest.scala index d30d5a1040ee5..da54113ae5c7c 100755 --- a/core/src/test/scala/unit/kafka/log/UnifiedLogTest.scala +++ b/core/src/test/scala/unit/kafka/log/UnifiedLogTest.scala @@ -41,7 +41,7 @@ import org.apache.kafka.server.storage.log.{FetchIsolation, UnexpectedAppendOffs import org.apache.kafka.server.util.{KafkaScheduler, MockTime, Scheduler} import org.apache.kafka.storage.internals.checkpoint.{LeaderEpochCheckpointFile, PartitionMetadataFile} import org.apache.kafka.storage.internals.epoch.LeaderEpochFileCache -import org.apache.kafka.storage.internals.log.{AbortedTxn, AppendOrigin, Cleaner, EpochEntry, LogConfig, LogFileUtils, LogOffsetMetadata, LogOffsetSnapshot, LogOffsetsListener, LogSegment, LogSegments, LogStartOffsetIncrementReason, LogToClean, OffsetResultHolder, OffsetsOutOfOrderException, ProducerStateManager, ProducerStateManagerConfig, RecordValidationException, UnifiedLog, VerificationGuard} +import org.apache.kafka.storage.internals.log.{AbortedTxn, AppendOrigin, AsyncOffsetReader, Cleaner, EpochEntry, LogConfig, LogFileUtils, LogOffsetMetadata, LogOffsetSnapshot, LogOffsetsListener, LogSegment, LogSegments, LogStartOffsetIncrementReason, LogToClean, OffsetResultHolder, OffsetsOutOfOrderException, ProducerStateManager, ProducerStateManagerConfig, RecordValidationException, UnifiedLog, VerificationGuard} import org.apache.kafka.storage.internals.utils.Throttler import org.apache.kafka.storage.log.metrics.{BrokerTopicMetrics, BrokerTopicStats} import org.junit.jupiter.api.Assertions._ @@ -2416,6 +2416,193 @@ class UnifiedLogTest { KafkaConfig.fromProps(props) } + @Test + def testFetchEarliestPendingUploadTimestampNoRemoteStorage(): Unit = { + val logConfig = LogTestUtils.createLogConfig(segmentBytes = 200, indexIntervalBytes = 1) + val log = createLog(logDir, logConfig) + + // Test initial state before any records + assertFetchOffsetBySpecialTimestamp(log, None, new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, -1, Optional.of(-1)), + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) + + // Append records + val _ = prepareLogWithSequentialRecords(log, recordCount = 2) + + // Test state after records are appended + assertFetchOffsetBySpecialTimestamp(log, None, new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, -1, Optional.of(-1)), + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) + } + + @Test + def testFetchEarliestPendingUploadTimestampWithRemoteStorage(): Unit = { + val logStartOffset = 0 + val (remoteLogManager: RemoteLogManager, log: UnifiedLog, timestampAndEpochs: Seq[TimestampAndEpoch]) = prepare(logStartOffset) + + val (firstTimestamp, firstLeaderEpoch) = (timestampAndEpochs.head.timestamp, timestampAndEpochs.head.leaderEpoch) + val (secondTimestamp, secondLeaderEpoch) = (timestampAndEpochs(1).timestamp, timestampAndEpochs(1).leaderEpoch) + val (_, thirdLeaderEpoch) = (timestampAndEpochs(2).timestamp, timestampAndEpochs(2).leaderEpoch) + + doAnswer(ans => { + val timestamp = ans.getArgument(1).asInstanceOf[Long] + Optional.of(timestamp) + .filter(_ == timestampAndEpochs.head.timestamp) + .map[TimestampAndOffset](x => new TimestampAndOffset(x, 0L, Optional.of(timestampAndEpochs.head.leaderEpoch))) + }).when(remoteLogManager).findOffsetByTimestamp(ArgumentMatchers.eq(log.topicPartition), + anyLong(), anyLong(), ArgumentMatchers.eq(log.leaderEpochCache)) + + // Offset 0 (first timestamp) is in remote storage and deleted locally. Offset 1 (second timestamp) is in local storage. + log.updateLocalLogStartOffset(1) + log.updateHighestOffsetInRemoteStorage(0) + + // In the assertions below we test that offset 0 (first timestamp) is only in remote and offset 1 (second timestamp) is in local storage. + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(firstTimestamp, 0L, Optional.of(firstLeaderEpoch))), firstTimestamp) + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(secondTimestamp, 1L, Optional.of(secondLeaderEpoch))), secondTimestamp) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager), new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 0L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager), new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 0L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.LATEST_TIERED_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager), new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 1L, Optional.of(secondLeaderEpoch)), + ListOffsetsRequest.EARLIEST_LOCAL_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager), new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 3L, Optional.of(thirdLeaderEpoch)), + ListOffsetsRequest.LATEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager), new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 1L, Optional.of(secondLeaderEpoch)), + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) + } + + @Test + def testFetchEarliestPendingUploadTimestampWithRemoteStorageNoLocalDeletion(): Unit = { + val logStartOffset = 0 + val (remoteLogManager: RemoteLogManager, log: UnifiedLog, timestampAndEpochs: Seq[TimestampAndEpoch]) = prepare(logStartOffset) + + val (firstTimestamp, firstLeaderEpoch) = (timestampAndEpochs.head.timestamp, timestampAndEpochs.head.leaderEpoch) + val (secondTimestamp, secondLeaderEpoch) = (timestampAndEpochs(1).timestamp, timestampAndEpochs(1).leaderEpoch) + val (_, thirdLeaderEpoch) = (timestampAndEpochs(2).timestamp, timestampAndEpochs(2).leaderEpoch) + + // Offsets upto 1 are in remote storage + doAnswer(ans => { + val timestamp = ans.getArgument(1).asInstanceOf[Long] + Optional.of( + timestamp match { + case x if x == firstTimestamp => new TimestampAndOffset(x, 0L, Optional.of(firstLeaderEpoch)) + case x if x == secondTimestamp => new TimestampAndOffset(x, 1L, Optional.of(secondLeaderEpoch)) + case _ => null + } + ) + }).when(remoteLogManager).findOffsetByTimestamp(ArgumentMatchers.eq(log.topicPartition), + anyLong(), anyLong(), ArgumentMatchers.eq(log.leaderEpochCache)) + + // Offsets 0, 1 (first and second timestamps) are in remote storage and not deleted locally. + log.updateLocalLogStartOffset(0) + log.updateHighestOffsetInRemoteStorage(1) + + // In the assertions below we test that offset 0 (first timestamp) and offset 1 (second timestamp) are on both remote and local storage + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(firstTimestamp, 0L, Optional.of(firstLeaderEpoch))), firstTimestamp) + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(secondTimestamp, 1L, Optional.of(secondLeaderEpoch))), secondTimestamp) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager), new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 0L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 1L, Optional.of(secondLeaderEpoch)), + ListOffsetsRequest.LATEST_TIERED_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 0L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_LOCAL_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 3L, Optional.of(thirdLeaderEpoch)), + ListOffsetsRequest.LATEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 2L, Optional.of(thirdLeaderEpoch)), + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) + } + + @Test + def testFetchEarliestPendingUploadTimestampNoSegmentsUploaded(): Unit = { + val logStartOffset = 0 + val (remoteLogManager: RemoteLogManager, log: UnifiedLog, timestampAndEpochs: Seq[TimestampAndEpoch]) = prepare(logStartOffset) + + val (firstTimestamp, firstLeaderEpoch) = (timestampAndEpochs.head.timestamp, timestampAndEpochs.head.leaderEpoch) + val (secondTimestamp, secondLeaderEpoch) = (timestampAndEpochs(1).timestamp, timestampAndEpochs(1).leaderEpoch) + val (_, thirdLeaderEpoch) = (timestampAndEpochs(2).timestamp, timestampAndEpochs(2).leaderEpoch) + + // No offsets are in remote storage + doAnswer(_ => Optional.empty[TimestampAndOffset]()) + .when(remoteLogManager).findOffsetByTimestamp(ArgumentMatchers.eq(log.topicPartition), + anyLong(), anyLong(), ArgumentMatchers.eq(log.leaderEpochCache)) + + // Offsets 0, 1, 2 (first, second and third timestamps) are in local storage only and not uploaded to remote storage. + log.updateLocalLogStartOffset(0) + log.updateHighestOffsetInRemoteStorage(-1) + + // In the assertions below we test that offset 0 (first timestamp), offset 1 (second timestamp) and offset 2 (third timestamp) are only on the local storage. + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(firstTimestamp, 0L, Optional.of(firstLeaderEpoch))), firstTimestamp) + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(secondTimestamp, 1L, Optional.of(secondLeaderEpoch))), secondTimestamp) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 0L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, -1L, Optional.of(-1)), + ListOffsetsRequest.LATEST_TIERED_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 0L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_LOCAL_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 3L, Optional.of(thirdLeaderEpoch)), + ListOffsetsRequest.LATEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 0L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) + } + + @Test + def testFetchEarliestPendingUploadTimestampStaleHighestOffsetInRemote(): Unit = { + val logStartOffset = 100 + val (remoteLogManager: RemoteLogManager, log: UnifiedLog, timestampAndEpochs: Seq[TimestampAndEpoch]) = prepare(logStartOffset) + + val (firstTimestamp, firstLeaderEpoch) = (timestampAndEpochs.head.timestamp, timestampAndEpochs.head.leaderEpoch) + val (secondTimestamp, secondLeaderEpoch) = (timestampAndEpochs(1).timestamp, timestampAndEpochs(1).leaderEpoch) + val (_, thirdLeaderEpoch) = (timestampAndEpochs(2).timestamp, timestampAndEpochs(2).leaderEpoch) + + // Offsets 100, 101, 102 (first, second and third timestamps) are in local storage and not uploaded to remote storage. + // Tiered storage copy was disabled and then enabled again, because of which the remote log segments are deleted but + // the highest offset in remote storage has become stale + doAnswer(_ => Optional.empty[TimestampAndOffset]()) + .when(remoteLogManager).findOffsetByTimestamp(ArgumentMatchers.eq(log.topicPartition), + anyLong(), anyLong(), ArgumentMatchers.eq(log.leaderEpochCache)) + + log.updateLocalLogStartOffset(100) + log.updateHighestOffsetInRemoteStorage(50) + + // In the assertions below we test that offset 100 (first timestamp), offset 101 (second timestamp) and offset 102 (third timestamp) are only on the local storage. + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(firstTimestamp, 100L, Optional.of(firstLeaderEpoch))), firstTimestamp) + assertFetchOffsetByTimestamp(log, Some(remoteLogManager), Some(new TimestampAndOffset(secondTimestamp, 101L, Optional.of(secondLeaderEpoch))), secondTimestamp) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 100L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 50L, Optional.empty()), + ListOffsetsRequest.LATEST_TIERED_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 100L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_LOCAL_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 103L, Optional.of(thirdLeaderEpoch)), + ListOffsetsRequest.LATEST_TIMESTAMP) + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager),new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, 100L, Optional.of(firstLeaderEpoch)), + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) + } + + private def prepare(logStartOffset: Int): (RemoteLogManager, UnifiedLog, Seq[TimestampAndEpoch]) = { + val config: KafkaConfig = createKafkaConfigWithRLM + val purgatory = new DelayedOperationPurgatory[DelayedRemoteListOffsets]("RemoteListOffsets", config.brokerId) + val remoteLogManager = spy(new RemoteLogManager(config.remoteLogManagerConfig, + 0, + logDir.getAbsolutePath, + "clusterId", + mockTime, + _ => Optional.empty[UnifiedLog](), + (_, _) => {}, + brokerTopicStats, + new Metrics(), + Optional.empty)) + remoteLogManager.setDelayedOperationPurgatory(purgatory) + + val logConfig = LogTestUtils.createLogConfig(segmentBytes = 200, indexIntervalBytes = 1, remoteLogStorageEnable = true) + val log = createLog(logDir, logConfig, logStartOffset = logStartOffset, remoteStorageSystemEnable = true, remoteLogManager = Some(remoteLogManager)) + + // Verify earliest pending upload offset for empty log + assertFetchOffsetBySpecialTimestamp(log, Some(remoteLogManager), new TimestampAndOffset(ListOffsetsResponse.UNKNOWN_TIMESTAMP, logStartOffset, Optional.empty()), + ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) + + val timestampAndEpochs = prepareLogWithSequentialRecords(log, recordCount = 3) + (remoteLogManager, log, timestampAndEpochs) + } + /** * Test the Log truncate operations */ @@ -4786,6 +4973,44 @@ class UnifiedLogTest { (log, segmentWithOverflow) } + + private def assertFetchOffsetByTimestamp(log: UnifiedLog, remoteLogManagerOpt: Option[RemoteLogManager], expected: Option[TimestampAndOffset], timestamp: Long): Unit = { + val remoteOffsetReader = getRemoteOffsetReader(remoteLogManagerOpt) + val offsetResultHolder = log.fetchOffsetByTimestamp(timestamp, remoteOffsetReader) + assertTrue(offsetResultHolder.futureHolderOpt.isPresent) + offsetResultHolder.futureHolderOpt.get.taskFuture.get(1, TimeUnit.SECONDS) + assertTrue(offsetResultHolder.futureHolderOpt.get.taskFuture.isDone) + assertTrue(offsetResultHolder.futureHolderOpt.get.taskFuture.get().hasTimestampAndOffset) + assertEquals(expected.get, offsetResultHolder.futureHolderOpt.get.taskFuture.get().timestampAndOffset().orElse(null)) + } + + private def assertFetchOffsetBySpecialTimestamp(log: UnifiedLog, remoteLogManagerOpt: Option[RemoteLogManager], expected: TimestampAndOffset, timestamp: Long): Unit = { + val remoteOffsetReader = getRemoteOffsetReader(remoteLogManagerOpt) + val offsetResultHolder = log.fetchOffsetByTimestamp(timestamp, remoteOffsetReader) + assertEquals(new OffsetResultHolder(expected), offsetResultHolder) + } + + private def getRemoteOffsetReader(remoteLogManagerOpt: Option[Any]): Optional[AsyncOffsetReader] = { + remoteLogManagerOpt match { + case Some(remoteLogManager) => Optional.of(remoteLogManager.asInstanceOf[AsyncOffsetReader]) + case None => Optional.empty[AsyncOffsetReader]() + } + } + + private def prepareLogWithSequentialRecords(log: UnifiedLog, recordCount: Int): Seq[TimestampAndEpoch] = { + val firstTimestamp = mockTime.milliseconds() + + (0 until recordCount).map { i => + val timestampAndEpoch = TimestampAndEpoch(firstTimestamp + i, i) + log.appendAsLeader( + TestUtils.singletonRecords(value = TestUtils.randomBytes(10), timestamp = timestampAndEpoch.timestamp), + timestampAndEpoch.leaderEpoch + ) + timestampAndEpoch + } + } + + case class TimestampAndEpoch(timestamp: Long, leaderEpoch: Int) } object UnifiedLogTest { diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index 1d0dfe019d830..046ef52a7de90 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -1180,4 +1180,564 @@ class AbstractFetcherThreadTest { fetcher.processFetchRequest(partitionData, fetchRequestOpt) assertEquals(0, replicaState.logEndOffset, "FetchResponse should be ignored when leader epoch does not match") } + + private def emptyReplicaState(rlmEnabled: Boolean, partition: TopicPartition, fetcher: MockFetcherThread): PartitionState = { + // Follower begins with an empty log + val replicaState = PartitionState(Seq(), leaderEpoch = 0, highWatermark = 0L, rlmEnabled = rlmEnabled) + fetcher.setReplicaState(partition, replicaState) + fetcher.addPartitions(Map(partition -> initialFetchState(topicIds.get(partition.topic), fetchOffset = 0, leaderEpoch = 0))) + replicaState + } + + /** + * Test: Empty Follower Fetch with TieredStorage Disabled and Leader LogStartOffset = 0 + * + * Purpose: + * - Simulate a leader with logs starting at offset 0 and validate how the follower + * behaves when TieredStorage is disabled. + * + * Conditions: + * - TieredStorage: **Disabled** + * - Leader LogStartOffset: **0** + * + * Scenario: + * - The leader starts with a log at offset 0, containing three record batches offset at 0, 150, and 199. + * - The follower begins fetching, and we validate the correctness of its replica state as it fetches. + * + * Expected Outcomes: + * 1. The follower fetch state should transition to `FETCHING` initially. + * 2. After the first poll, one record batch is fetched. + * 3. After subsequent polls, the entire leader log is fetched: + * - Replica log size: 3 + * - Replica LogStartOffset: 0 + * - Replica LogEndOffset: 200 + * - Replica HighWatermark: 199 + */ + @Test + def testEmptyFollowerFetchTieredStorageDisabledLeaderLogStartOffsetZero(): Unit = { + val rlmEnabled = false + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LogStartOffset = LocalLogStartOffset = 0 + mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + mkBatch(baseOffset = 199, leaderEpoch = 0, new SimpleRecord("e".getBytes)) + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 199L, + rlmEnabled = rlmEnabled + ) + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(1, replicaState.log.size) + assertEquals(0, replicaState.logStartOffset) + assertEquals(1, replicaState.logEndOffset) + assertEquals(Some(1), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Only 1 record batch is returned after a poll so calling 'n' number of times to get the desired result. + for (_ <- 1 to 2) fetcher.doWork() + assertEquals(3, replicaState.log.size) + assertEquals(0, replicaState.logStartOffset) + assertEquals(200, replicaState.logEndOffset) + assertEquals(199, replicaState.highWatermark) + } + + /** + * Test: Empty Follower Fetch with TieredStorage Disabled and Leader LogStartOffset != 0 + * + * Purpose: + * - Validate follower behavior when the leader's log starts at a non-zero offset (10). + * + * Conditions: + * - TieredStorage: **Disabled** + * - Leader LogStartOffset: **10** + * + * Scenario: + * - The leader log starts at offset 10 with batches at 10, 150, and 199. + * - The follower starts fetching from offset 10. + * + * Expected Outcomes: + * 1. The follower's initial log is empty. + * 2. Replica offsets after polls: + * - LogStartOffset = 10 + * - LogEndOffset = 200 + * - HighWatermark = 199 + */ + @Test + def testEmptyFollowerFetchTieredStorageDisabledLeaderLogStartOffsetNonZero(): Unit = { + val rlmEnabled = false + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LogStartOffset = LocalLogStartOffset = 10 + mkBatch(baseOffset = 10, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + mkBatch(baseOffset = 199, leaderEpoch = 0, new SimpleRecord("e".getBytes)) + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 199L, + rlmEnabled = rlmEnabled + ) + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + // Follower gets out-of-range error (no messages received), fetch offset is updated from 0 to 10 + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(10, replicaState.logStartOffset) + assertEquals(10, replicaState.logEndOffset) + assertEquals(Some(10), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Only 1 record batch is returned after a poll so calling 'n' number of times to get the desired result. + for (_ <- 1 to 3) fetcher.doWork() + assertEquals(3, replicaState.log.size) + assertEquals(10, replicaState.logStartOffset) + assertEquals(200, replicaState.logEndOffset) + assertEquals(199, replicaState.highWatermark) + } + + /** + * Test: Empty Follower Fetch with TieredStorage Enabled, Leader LogStartOffset = 0, and No Local Deletions + * + * Purpose: + * - Simulate TieredStorage enabled and validate follower fetching behavior when the leader + * log starts at 0 and no segments have been uploaded or deleted locally. + * + * Conditions: + * - TieredStorage: **Enabled** + * - Leader LogStartOffset: **0** + * - Leader LocalLogStartOffset: **0** (No local segments deleted). + * + * Scenario: + * - The leader log contains three record batches at offsets 0, 150, and 199. + * - The follower starts fetching from offset 0. + * + * Expected Outcomes: + * 1. The replica log accurately reflects the leader's log: + * - LogStartOffset = 0 + * - LocalLogStartOffset = 0 + * - LogEndOffset = 200 + * - HighWatermark = 199 + */ + @Test + def testEmptyFollowerFetchTieredStorageEnabledLeaderLogStartOffsetZeroNoLocalDeletions(): Unit = { + val rlmEnabled = true + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LogStartOffset = LocalLogStartOffset = 0 + mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + mkBatch(baseOffset = 199, leaderEpoch = 0, new SimpleRecord("e".getBytes)) + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 199L, + rlmEnabled = rlmEnabled + ) + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(1, replicaState.log.size) + assertEquals(0, replicaState.logStartOffset) + assertEquals(0, replicaState.localLogStartOffset) + assertEquals(1, replicaState.logEndOffset) + assertEquals(199, replicaState.highWatermark) + assertEquals(Some(1), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Only 1 record batch is returned after a poll so calling 'n' number of times to get the desired result. + for (_ <- 1 to 2) fetcher.doWork() + assertEquals(3, replicaState.log.size) + assertEquals(0, replicaState.logStartOffset) + assertEquals(0, replicaState.localLogStartOffset) + assertEquals(200, replicaState.logEndOffset) + assertEquals(199, replicaState.highWatermark) + } + + /** + * Test: Empty Follower Fetch with TieredStorage Enabled, Leader LogStartOffset = 0, and Local Deletions + * + * Purpose: + * - Simulate TieredStorage enabled with some segments uploaded and deleted locally, causing + * a difference between the leader's LogStartOffset (0) and LocalLogStartOffset (> 0). + * + * Conditions: + * - TieredStorage: **Enabled** + * - Leader LogStartOffset: **0** + * - Leader LocalLogStartOffset: **100** (Some segments deleted locally). + * + * Scenario: + * - The leader log starts at offset 0 but the local leader log starts at offset 100. + * - The follower fetch operation begins from offset 0. + * + * Expected Outcomes: + * 1. After offset adjustments for local deletions: + * - LogStartOffset = 0 + * - LocalLogStartOffset = 100 + * - LogEndOffset = 200 + * - HighWatermark = 199 + */ + @Test + def testEmptyFollowerFetchTieredStorageEnabledLeaderLogStartOffsetZeroWithLocalDeletions(): Unit = { + val rlmEnabled = true + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LocalLogStartOffset = 100 + mkBatch(baseOffset = 100, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + mkBatch(baseOffset = 199, leaderEpoch = 0, new SimpleRecord("e".getBytes)) + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 199L, + rlmEnabled = rlmEnabled + ) + leaderState.logStartOffset = 0 + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(100, replicaState.localLogStartOffset) + assertEquals(100, replicaState.logEndOffset) + assertEquals(Some(100), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Only 1 record batch is returned after a poll so calling 'n' number of times to get the desired result. + for (_ <- 1 to 3) fetcher.doWork() + assertEquals(3, replicaState.log.size) + assertEquals(0, replicaState.logStartOffset) + assertEquals(100, replicaState.localLogStartOffset) + assertEquals(200, replicaState.logEndOffset) + assertEquals(199, replicaState.highWatermark) + } + + /** + * Test: Empty Follower Fetch with TieredStorage Enabled, Leader LogStartOffset != 0, and No Local Deletions + * + * Purpose: + * - Simulate TieredStorage enabled and validate follower fetch behavior when the leader's log + * starts at a non-zero offset and no local deletions have occurred. + * + * Conditions: + * - TieredStorage: **Enabled** + * - Leader LogStartOffset: **10** + * - Leader LocalLogStartOffset: **10** (No deletions). + * + * Scenario: + * - The leader log starts at offset 10 with batches at 10, 150, and 199. + * - The follower starts fetching from offset 10. + * + * Expected Outcomes: + * 1. After fetching, the replica log matches the leader: + * - LogStartOffset = 10 + * - LocalLogStartOffset = 10 + * - LogEndOffset = 200 + * - HighWatermark = 199 + */ + @Test + def testEmptyFollowerFetchTieredStorageEnabledLeaderLogStartOffsetNonZeroNoLocalDeletions(): Unit = { + val rlmEnabled = true + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LogStartOffset = LocalLogStartOffset = 10 + mkBatch(baseOffset = 10, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + mkBatch(baseOffset = 199, leaderEpoch = 0, new SimpleRecord("e".getBytes)) + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 199L, + rlmEnabled = rlmEnabled, + ) + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(10, replicaState.localLogStartOffset) + assertEquals(10, replicaState.logEndOffset) + assertEquals(Some(10), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Only 1 record batch is returned after a poll so calling 'n' number of times to get the desired result. + for (_ <- 1 to 3) fetcher.doWork() + assertEquals(3, replicaState.log.size) + assertEquals(10, replicaState.logStartOffset) + assertEquals(10, replicaState.localLogStartOffset) + assertEquals(200, replicaState.logEndOffset) + assertEquals(199, replicaState.highWatermark) + } + + /** + * Test: Empty Follower Fetch with TieredStorage Enabled, Leader LogStartOffset != 0, and Local Deletions + * + * Purpose: + * - Validate follower adjustments when the leader has log deletions causing + * LocalLogStartOffset > LogStartOffset. + * + * Conditions: + * - TieredStorage: **Enabled** + * - Leader LogStartOffset: **10** + * - Leader LocalLogStartOffset: **100** (All older segments deleted locally). + * + * Scenario: + * - The leader log starts at offset 10 but the local log starts at offset 100. + * - The follower fetch starts at offset 10 but adjusts for local deletions. + * + * Expected Outcomes: + * 1. Initial fetch offset adjustments: + * - First adjustment: LogEndOffset = 10 (after offset-out-of-range error) + * - Second adjustment: LogEndOffset = 100 (after offset-moved-to-tiered-storage error) + * 2. After successful fetches: + * - 3 record batches fetched + * - LogStartOffset = 10 + * - LocalLogStartOffset = 100 + * - LogEndOffset = 200 + * - HighWatermark = 199 + */ + @Test + def testEmptyFollowerFetchTieredStorageEnabledLeaderLogStartOffsetNonZeroWithLocalDeletions(): Unit = { + val rlmEnabled = true + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LocalLogStartOffset = 100 + mkBatch(baseOffset = 100, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + mkBatch(baseOffset = 199, leaderEpoch = 0, new SimpleRecord("e".getBytes)) + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 199L, + rlmEnabled = rlmEnabled, + ) + leaderState.logStartOffset = 10 + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + // On offset-out-of-range error, fetch offset is updated + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(10, replicaState.localLogStartOffset) + assertEquals(10, replicaState.logEndOffset) + assertEquals(Some(10), fetcher.fetchState(partition).map(_.fetchOffset())) + + fetcher.doWork() + // On offset-moved-to-tiered-storage error, fetch offset is updated + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(100, replicaState.localLogStartOffset) + assertEquals(100, replicaState.logEndOffset) + assertEquals(Some(100), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Only 1 record batch is returned after a poll so calling 'n' number of times to get the desired result. + for (_ <- 1 to 3) fetcher.doWork() + assertEquals(3, replicaState.log.size) + assertEquals(10, replicaState.logStartOffset) + assertEquals(100, replicaState.localLogStartOffset) + assertEquals(200, replicaState.logEndOffset) + assertEquals(199, replicaState.highWatermark) + } + + /** + * Test: Empty Follower Fetch with TieredStorage Enabled, All Local Segments Deleted + * + * Purpose: + * - Handle scenarios where all local segments have been deleted: + * - LocalLogStartOffset > LogStartOffset. + * - LocalLogStartOffset = LogEndOffset. + * + * Conditions: + * - TieredStorage: **Enabled** + * - Leader LogStartOffset: **0 or > 0** + * - Leader LocalLogStartOffset: Leader LogEndOffset (all segments deleted locally). + * + * Expected Outcomes: + * 1. Follower state is adjusted to reflect local deletions: + * - LocalLogStartOffset = LogEndOffset. + * - No new data remains to fetch. + */ + @Test + def testEmptyFollowerFetchTieredStorageEnabledLeaderLogStartOffsetZeroAllLocalSegmentsDeleted(): Unit = { + val rlmEnabled = true + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LocalLogStartOffset = 100 + mkBatch(baseOffset = 100, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 151L, + rlmEnabled = rlmEnabled + ) + leaderState.logStartOffset = 0 + // Set Local Log Start Offset to Log End Offset + leaderState.localLogStartOffset = 151 + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + + // On offset-moved-to-tiered-storage error, fetch offset is updated + fetcher.doWork() + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(151, replicaState.localLogStartOffset) + assertEquals(151, replicaState.logEndOffset) + assertEquals(151, replicaState.highWatermark) + assertEquals(Some(151), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Call once again to see if new data is received + fetcher.doWork() + // No metadata update expected + assertEquals(0, replicaState.log.size) + assertEquals(0, replicaState.logStartOffset) + assertEquals(151, replicaState.localLogStartOffset) + assertEquals(151, replicaState.logEndOffset) + assertEquals(151, replicaState.highWatermark) + } + + /** + * Test: Empty Follower Fetch with TieredStorage Enabled, Leader LogStartOffset != 0, and All Local Segments Deleted + * + * Purpose: + * - Validate follower behavior when TieredStorage is enabled, the leader's log starts at a non-zero offset, + * and all local log segments have been deleted. + * + * Conditions: + * - TieredStorage: **Enabled** + * - Leader LogStartOffset: **10** + * - Leader LocalLogStartOffset: **151** (all older segments deleted locally). + * + * Scenario: + * - The leader log contains record batches from offset 100, but all local segments up to offset 151 are deleted. + * - The follower starts at LogStartOffset = 10 and adjusts for local segment deletions. + * + * Expected Outcomes: + * 1. Follower detects offset adjustments due to local deletions: + * - LogStartOffset remains 10. + * - LocalLogStartOffset updates to 151. + * - LogEndOffset updates to 151. + * 2. HighWatermark aligns with the leader (151). + * 3. No new data is fetched since all relevant segments are deleted. + */ + @Test + def testEmptyFollowerFetchTieredStorageEnabledLeaderLogStartOffsetNonZeroAllLocalSegmentsDeleted(): Unit = { + val rlmEnabled = true + val partition = new TopicPartition("topic1", 0) + val mockLeaderEndpoint = new MockLeaderEndPoint(version = version) + val mockTierStateMachine = new MockTierStateMachine(mockLeaderEndpoint) + val fetcher = new MockFetcherThread(mockLeaderEndpoint, mockTierStateMachine) + + val replicaState = emptyReplicaState(rlmEnabled, partition, fetcher) + + val leaderLog = Seq( + // LocalLogStartOffset = 100 + mkBatch(baseOffset = 100, leaderEpoch = 0, new SimpleRecord("c".getBytes)), + mkBatch(baseOffset = 150, leaderEpoch = 0, new SimpleRecord("d".getBytes)), + ) + + val leaderState = PartitionState( + leaderLog, + leaderEpoch = 0, + highWatermark = 151L, + rlmEnabled = rlmEnabled + ) + leaderState.logStartOffset = 10 + // Set Local Log Start Offset to Log End Offset + leaderState.localLogStartOffset = 151 + fetcher.mockLeader.setLeaderState(partition, leaderState) + fetcher.mockLeader.setReplicaPartitionStateCallback(fetcher.replicaPartitionState) + + fetcher.doWork() + + // On offset-out-of-range error, fetch offset is updated + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(10, replicaState.localLogStartOffset) + assertEquals(10, replicaState.logEndOffset) + assertEquals(Some(10), fetcher.fetchState(partition).map(_.fetchOffset())) + + // On offset-moved-to-tiered-storage error, fetch offset is updated + fetcher.doWork() + assertEquals(Option(ReplicaState.FETCHING), fetcher.fetchState(partition).map(_.state)) + assertEquals(0, replicaState.log.size) + assertEquals(151, replicaState.localLogStartOffset) + assertEquals(151, replicaState.logEndOffset) + assertEquals(151, replicaState.highWatermark) + assertEquals(Some(151), fetcher.fetchState(partition).map(_.fetchOffset())) + + // Call once again to see if new data is received + fetcher.doWork() + // No metadata update expected + assertEquals(0, replicaState.log.size) + assertEquals(10, replicaState.logStartOffset) + assertEquals(151, replicaState.localLogStartOffset) + assertEquals(151, replicaState.logEndOffset) + assertEquals(151, replicaState.highWatermark) + } } \ No newline at end of file diff --git a/docker/README.md b/docker/README.md index 9c2916d929381..c4b9d49d0eaf1 100644 --- a/docker/README.md +++ b/docker/README.md @@ -130,6 +130,10 @@ python docker_build_test.py kafka/test --image-tag=3.6.0 --image-type=jvm --kafk ``` python docker_build_test.py kafka/test --image-tag=3.8.0 --image-type=native --kafka-url=https://archive.apache.org/dist/kafka/3.8.0/kafka_2.13-3.8.0.tgz ``` +- Example(local build archive with jvm or native image type) :- To build and test an image named test with local build archive +``` +python docker_build_test.py kafka/test --image-tag=local-build --image-type= --kafka-archive= +``` Creating a Release Candidate ---------------------------- diff --git a/docker/common.py b/docker/common.py index 9c0f901823fa5..f04a484a187b0 100644 --- a/docker/common.py +++ b/docker/common.py @@ -33,12 +33,14 @@ def get_input(message): raise ValueError("This field cannot be empty") return value -def build_docker_image_runner(command, image_type): +def build_docker_image_runner(command, image_type, kafka_archive=None): temp_dir_path = tempfile.mkdtemp() current_dir = os.path.dirname(os.path.realpath(__file__)) copy_tree(f"{current_dir}/{image_type}", f"{temp_dir_path}/{image_type}") copy_tree(f"{current_dir}/resources", f"{temp_dir_path}/{image_type}/resources") copy_file(f"{current_dir}/server.properties", f"{temp_dir_path}/{image_type}") + if kafka_archive: + copy_file(kafka_archive, f"{temp_dir_path}/{image_type}/kafka.tgz") command = command.replace("$DOCKER_FILE", f"{temp_dir_path}/{image_type}/Dockerfile") command = command.replace("$DOCKER_DIR", f"{temp_dir_path}/{image_type}") try: diff --git a/docker/docker_build_test.py b/docker/docker_build_test.py index 793148573f395..fab6e65263df6 100755 --- a/docker/docker_build_test.py +++ b/docker/docker_build_test.py @@ -25,9 +25,11 @@ Example command:- docker_build_test.py --image-tag --image-type --kafka-url + docker_build_test.py --image-tag --image-type --kafka-archive This command will build an image with as image name, as image_tag (it will be latest by default), as image type (jvm by default), for the kafka inside the image and run tests on the image. + can be passed as an alternative to to use a local kafka archive. The path of kafka_archive should be absolute. -b can be passed as additional argument if you just want to build the image. -t can be passed if you just want to run tests on the image. """ @@ -41,10 +43,6 @@ import tempfile import os -def build_docker_image(image, tag, kafka_url, image_type): - image = f'{image}:{tag}' - build_docker_image_runner(f"docker build -f $DOCKER_FILE -t {image} --build-arg kafka_url={kafka_url} --build-arg build_date={date.today()} $DOCKER_DIR", image_type) - def run_docker_tests(image, tag, kafka_url, image_type): temp_dir_path = tempfile.mkdtemp() try: @@ -69,16 +67,20 @@ def run_docker_tests(image, tag, kafka_url, image_type): parser.add_argument("image", help="Image name that you want to keep for the Docker image") parser.add_argument("--image-tag", "-tag", default="latest", dest="tag", help="Image tag that you want to add to the image") parser.add_argument("--image-type", "-type", choices=["jvm", "native"], default="jvm", dest="image_type", help="Image type you want to build") - parser.add_argument("--kafka-url", "-u", dest="kafka_url", help="Kafka url to be used to download kafka binary tarball in the docker image") parser.add_argument("--build", "-b", action="store_true", dest="build_only", default=False, help="Only build the image, don't run tests") parser.add_argument("--test", "-t", action="store_true", dest="test_only", default=False, help="Only run the tests, don't build the image") + + archive_group = parser.add_mutually_exclusive_group(required=True) + archive_group.add_argument("--kafka-url", "-u", dest="kafka_url", help="Kafka url to be used to download kafka binary tarball in the docker image") + archive_group.add_argument("--kafka-archive", "-a", dest="kafka_archive", help="Kafka archive to be used to extract kafka binary tarball in the docker image") + args = parser.parse_args() if args.build_only or not (args.build_only or args.test_only): if args.kafka_url: - build_docker_image(args.image, args.tag, args.kafka_url, args.image_type) - else: - raise ValueError("--kafka-url is a required argument for docker image") + build_docker_image_runner(f"docker build -f $DOCKER_FILE -t {args.image}:{args.tag} --build-arg kafka_url={args.kafka_url} --build-arg build_date={date.today()} --no-cache --progress=plain $DOCKER_DIR", args.image_type) + elif args.kafka_archive: + build_docker_image_runner(f"docker build -f $DOCKER_FILE -t {args.image}:{args.tag} --build-arg build_date={date.today()} --no-cache --progress=plain $DOCKER_DIR", args.image_type, args.kafka_archive) if args.test_only or not (args.build_only or args.test_only): run_docker_tests(args.image, args.tag, args.kafka_url, args.image_type) diff --git a/docker/jvm/Dockerfile b/docker/jvm/Dockerfile index f98f50a2e0390..3d2f06820d4b4 100644 --- a/docker/jvm/Dockerfile +++ b/docker/jvm/Dockerfile @@ -23,20 +23,27 @@ USER root # Get kafka from https://archive.apache.org/dist/kafka and pass the url through build arguments ARG kafka_url +ENV KAFKA_URL=$kafka_url + COPY jsa_launch /etc/kafka/docker/jsa_launch COPY server.properties /etc/kafka/docker/server.properties +COPY *kafka.tgz kafka.tgz + RUN set -eux ; \ apk update ; \ apk upgrade ; \ - apk add --no-cache wget gcompat gpg gpg-agent procps bash; \ + apk add --no-cache bash; \ + if [ -n "$KAFKA_URL" ]; then \ + apk add --no-cache wget gcompat gpg gpg-agent procps; \ + wget -nv -O kafka.tgz "$KAFKA_URL"; \ + wget -nv -O kafka.tgz.asc "$KAFKA_URL.asc"; \ + wget -nv -O KEYS https://downloads.apache.org/kafka/KEYS; \ + gpg --import KEYS; \ + gpg --batch --verify kafka.tgz.asc kafka.tgz; \ + fi; \ mkdir opt/kafka; \ - wget -nv -O kafka.tgz "$kafka_url"; \ - wget -nv -O kafka.tgz.asc "$kafka_url.asc"; \ - tar xfz kafka.tgz -C /opt/kafka --strip-components 1; \ - wget -nv -O KEYS https://downloads.apache.org/kafka/KEYS; \ - gpg --import KEYS; \ - gpg --batch --verify kafka.tgz.asc kafka.tgz + tar xfz kafka.tgz -C opt/kafka --strip-components 1; # Generate jsa files using dynamic CDS for kafka server start command and kafka storage format command RUN /etc/kafka/docker/jsa_launch @@ -53,6 +60,9 @@ USER root ARG kafka_url ARG build_date +ENV KAFKA_URL=$kafka_url + +COPY *kafka.tgz kafka.tgz LABEL org.label-schema.name="kafka" \ org.label-schema.description="Apache Kafka" \ @@ -60,17 +70,25 @@ LABEL org.label-schema.name="kafka" \ org.label-schema.vcs-url="https://github.com/apache/kafka" \ maintainer="Apache Kafka" -RUN set -eux ; \ +RUN mkdir opt/kafka; \ + set -eux ; \ apk update ; \ apk upgrade ; \ - apk add --no-cache wget gcompat gpg gpg-agent procps bash; \ - mkdir opt/kafka; \ - wget -nv -O kafka.tgz "$kafka_url"; \ - wget -nv -O kafka.tgz.asc "$kafka_url.asc"; \ - tar xfz kafka.tgz -C /opt/kafka --strip-components 1; \ - wget -nv -O KEYS https://downloads.apache.org/kafka/KEYS; \ - gpg --import KEYS; \ - gpg --batch --verify kafka.tgz.asc kafka.tgz; \ + apk add --no-cache bash; \ + if [ -n "$KAFKA_URL" ]; then \ + apk add --no-cache wget gcompat gpg gpg-agent procps; \ + wget -nv -O kafka.tgz "$KAFKA_URL"; \ + wget -nv -O kafka.tgz.asc "$KAFKA_URL.asc"; \ + tar xfz kafka.tgz -C /opt/kafka --strip-components 1; \ + wget -nv -O KEYS https://downloads.apache.org/kafka/KEYS; \ + gpg --import KEYS; \ + gpg --batch --verify kafka.tgz.asc kafka.tgz; \ + rm kafka.tgz kafka.tgz.asc KEYS; \ + apk del wget gpg gpg-agent; \ + else \ + tar xfz kafka.tgz -C /opt/kafka --strip-components 1; \ + rm kafka.tgz; \ + fi; \ mkdir -p /var/lib/kafka/data /etc/kafka/secrets; \ mkdir -p /etc/kafka/docker /usr/logs /mnt/shared/config; \ adduser -h /home/appuser -D --shell /bin/bash appuser; \ @@ -79,8 +97,6 @@ RUN set -eux ; \ chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \ cp /opt/kafka/config/log4j2.yaml /etc/kafka/docker/log4j2.yaml; \ cp /opt/kafka/config/tools-log4j2.yaml /etc/kafka/docker/tools-log4j2.yaml; \ - rm kafka.tgz kafka.tgz.asc KEYS; \ - apk del wget gpg gpg-agent; \ apk cache clean; COPY server.properties /etc/kafka/docker/server.properties diff --git a/docker/native/Dockerfile b/docker/native/Dockerfile index ca85f35562df1..010edbcd51c3d 100644 --- a/docker/native/Dockerfile +++ b/docker/native/Dockerfile @@ -29,15 +29,18 @@ ENV TARGET_PATH="$KAFKA_DIR/kafka.Kafka" COPY native-image-configs $NATIVE_CONFIGS_DIR COPY native_command.sh native_command.sh -RUN mkdir $KAFKA_DIR; \ - microdnf install wget; \ - wget -nv -O kafka.tgz "$KAFKA_URL"; \ - wget -nv -O kafka.tgz.asc "$KAFKA_URL.asc"; \ +COPY *kafka.tgz /app + +RUN if [ -n "$KAFKA_URL" ]; then \ + microdnf install wget; \ + wget -nv -O kafka.tgz "$KAFKA_URL"; \ + wget -nv -O kafka.tgz.asc "$KAFKA_URL.asc"; \ + wget -nv -O KEYS https://downloads.apache.org/kafka/KEYS; \ + gpg --import KEYS; \ + gpg --batch --verify kafka.tgz.asc kafka.tgz; \ + fi; \ + mkdir $KAFKA_DIR; \ tar xfz kafka.tgz -C $KAFKA_DIR --strip-components 1; \ - wget -nv -O KEYS https://downloads.apache.org/kafka/KEYS; \ - gpg --import KEYS; \ - gpg --batch --verify kafka.tgz.asc kafka.tgz; \ - rm kafka.tgz ; \ # Build the native-binary of the apache kafka using graalVM native-image. /app/native_command.sh $NATIVE_IMAGE_PATH $NATIVE_CONFIGS_DIR $KAFKA_LIBS_DIR $TARGET_PATH diff --git a/docs/ops.html b/docs/ops.html index f804061007033..d6c0982854afc 100644 --- a/docs/ops.html +++ b/docs/ops.html @@ -4116,11 +4116,9 @@

$ bin/kafka-storage.sh format -t KAFKA_CLUSTER_ID --feature kraft.version=1 -c controller.properties - Cannot set kraft.version to 1 unless KIP-853 configuration is present. Try removing the --feature flag for kraft.version.

- Note: Currently it is not possible to convert clusters using a static controller quorum to - use a dynamic controller quorum. This function will be supported in the future release. + Note: To migrate from static voter set to dynamic voter set, please refer to the Upgrade section.

Add New Controller
If a dynamic controller cluster already exists, it can be expanded by first provisioning a new controller using the kafka-storage.sh tool and starting the controller. diff --git a/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java b/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java index 443804272844a..f87af4897a701 100644 --- a/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java +++ b/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java @@ -833,19 +833,28 @@ ConsumerGroup getOrMaybeCreateConsumerGroup( * Gets or creates a streams group without updating the groups map. * The group will be materialized during the replay. * + * If there is an empty classic consumer group of the same name, it will be deleted and a new streams + * group will be created. + * * @param groupId The group ID. + * @param records The record list to which the group tombstones are written + * if the group is empty and is a classic group. * * @return A StreamsGroup. * * Package private for testing. */ StreamsGroup getOrCreateStreamsGroup( - String groupId + String groupId, + List records ) { Group group = groups.get(groupId); if (group == null) { return new StreamsGroup(logContext, snapshotRegistry, groupId, metrics); + } else if (maybeDeleteEmptyClassicGroup(group, records)) { + log.info("[GroupId {}] Converted the empty classic group to a streams group.", groupId); + return new StreamsGroup(logContext, snapshotRegistry, groupId, metrics); } else { return castToStreamsGroup(group); } @@ -1871,7 +1880,7 @@ private CoordinatorResult stream boolean isJoining = memberEpoch == 0; StreamsGroup group; if (isJoining) { - group = getOrCreateStreamsGroup(groupId); + group = getOrCreateStreamsGroup(groupId, records); throwIfStreamsGroupIsFull(group); } else { group = getStreamsGroupOrThrow(groupId); @@ -6066,7 +6075,11 @@ public CoordinatorResult classicGroupJoin( // classicGroupJoinToConsumerGroup takes the join requests to non-empty consumer groups. // The empty consumer groups should be converted to classic groups in classicGroupJoinToClassicGroup. return classicGroupJoinToConsumerGroup((ConsumerGroup) group, context, request, responseFuture); - } else if (group.type() == CONSUMER || group.type() == CLASSIC) { + } else if (group.type() == CONSUMER || group.type() == CLASSIC || group.type() == STREAMS && group.isEmpty()) { + // classicGroupJoinToClassicGroup accepts: + // - classic groups + // - empty streams groups + // - empty consumer groups return classicGroupJoinToClassicGroup(context, request, responseFuture); } else { // Group exists but it's not a consumer group @@ -6107,6 +6120,8 @@ CoordinatorResult classicGroupJoinToClassicGroup( ClassicGroup group; if (maybeDeleteEmptyConsumerGroup(groupId, records)) { log.info("[GroupId {}] Converted the empty consumer group to a classic group.", groupId); + } else if (maybeDeleteEmptyStreamsGroup(groupId, records)) { + log.info("[GroupId {}] Converted the empty streams group to a classic group.", groupId); } boolean isNewGroup = !groups.containsKey(groupId); try { @@ -8398,6 +8413,13 @@ private static boolean isEmptyConsumerGroup(Group group) { return group != null && group.type() == CONSUMER && group.isEmpty(); } + /** + * @return true if the group is an empty streams group. + */ + private static boolean isEmptyStreamsGroup(Group group) { + return group != null && group.type() == STREAMS && group.isEmpty(); + } + /** * Write tombstones for the group if it's empty and is a classic group. * @@ -8435,6 +8457,26 @@ private boolean maybeDeleteEmptyConsumerGroup(String groupId, List records) { + Group group = groups.get(groupId, Long.MAX_VALUE); + if (isEmptyStreamsGroup(group)) { + // Add tombstones for the previous streams group. The tombstones won't actually be + // replayed because its coordinator result has a non-null appendFuture. + createGroupTombstoneRecords(group, records); + removeGroup(groupId); + return true; + } + return false; + } /** * Checks whether the given protocol type or name in the request is inconsistent with the group's. diff --git a/group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java b/group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java index 3f1e49b955d6c..efe2ad96435e7 100644 --- a/group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java +++ b/group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java @@ -18633,6 +18633,156 @@ public void testStreamsGroupEndpointInformationOnlyWhenEpochGreater() { assertNull(result.response().data().partitionsByUserEndpoint()); } + @Test + public void testStreamsGroupHeartbeatWithNonEmptyClassicGroup() { + String classicGroupId = "classic-group-id"; + String memberId = Uuid.randomUuid().toString(); + + String subtopology1 = "subtopology1"; + String fooTopicName = "foo"; + Topology topology = new Topology().setSubtopologies(List.of( + new Subtopology().setSubtopologyId(subtopology1).setSourceTopics(List.of(fooTopicName)) + )); + + GroupMetadataManagerTestContext context = new GroupMetadataManagerTestContext.Builder().build(); + ClassicGroup classicGroup = new ClassicGroup( + new LogContext(), + classicGroupId, + EMPTY, + context.time + ); + context.replay(GroupCoordinatorRecordHelpers.newGroupMetadataRecord(classicGroup, classicGroup.groupAssignment())); + + context.groupMetadataManager.getOrMaybeCreateClassicGroup(classicGroupId, false).transitionTo(PREPARING_REBALANCE); + assertThrows(GroupIdNotFoundException.class, () -> + context.streamsGroupHeartbeat( + new StreamsGroupHeartbeatRequestData() + .setGroupId(classicGroupId) + .setMemberId(memberId) + .setMemberEpoch(0) + .setRebalanceTimeoutMs(12000) + .setTopology(topology) + .setActiveTasks(List.of()) + .setStandbyTasks(List.of()) + .setWarmupTasks(List.of()))); + } + + @Test + public void testStreamsGroupHeartbeatWithEmptyClassicGroup() { + String classicGroupId = "classic-group-id"; + String memberId = Uuid.randomUuid().toString(); + String fooTopicName = "foo"; + String subtopology1 = "subtopology1"; + Topology topology = new Topology().setSubtopologies(List.of( + new Subtopology().setSubtopologyId(subtopology1).setSourceTopics(List.of(fooTopicName)) + )); + + MockTaskAssignor assignor = new MockTaskAssignor("sticky"); + GroupMetadataManagerTestContext context = new GroupMetadataManagerTestContext.Builder() + .withStreamsGroupTaskAssignors(List.of(assignor)) + .build(); + ClassicGroup classicGroup = new ClassicGroup( + new LogContext(), + classicGroupId, + EMPTY, + context.time + ); + context.replay(GroupCoordinatorRecordHelpers.newGroupMetadataRecord(classicGroup, classicGroup.groupAssignment())); + + CoordinatorResult result = context.streamsGroupHeartbeat( + new StreamsGroupHeartbeatRequestData() + .setGroupId(classicGroupId) + .setMemberId(memberId) + .setMemberEpoch(0) + .setRebalanceTimeoutMs(12000) + .setTopology(topology) + .setActiveTasks(List.of()) + .setStandbyTasks(List.of()) + .setWarmupTasks(List.of())); + + StreamsGroupMember expectedMember = StreamsGroupMember.Builder.withDefaults(memberId) + .setState(org.apache.kafka.coordinator.group.streams.MemberState.STABLE) + .setMemberEpoch(1) + .setPreviousMemberEpoch(0) + .setRebalanceTimeoutMs(5000) + .setClientId(DEFAULT_CLIENT_ID) + .setClientHost(DEFAULT_CLIENT_ADDRESS.toString()) + .setAssignedTasks(TasksTuple.EMPTY) + .setTasksPendingRevocation(TasksTuple.EMPTY) + .setRebalanceTimeoutMs(12000) + .setTopologyEpoch(0) + .build(); + + assertEquals(Errors.NONE.code(), result.response().data().errorCode()); + assertEquals( + List.of( + GroupCoordinatorRecordHelpers.newGroupMetadataTombstoneRecord(classicGroupId), + StreamsCoordinatorRecordHelpers.newStreamsGroupMemberRecord(classicGroupId, expectedMember), + StreamsCoordinatorRecordHelpers.newStreamsGroupTopologyRecord(classicGroupId, topology), + StreamsCoordinatorRecordHelpers.newStreamsGroupEpochRecord(classicGroupId, 1, 0), + StreamsCoordinatorRecordHelpers.newStreamsGroupTargetAssignmentRecord(classicGroupId, memberId, TasksTuple.EMPTY), + StreamsCoordinatorRecordHelpers.newStreamsGroupTargetAssignmentEpochRecord(classicGroupId, 1), + StreamsCoordinatorRecordHelpers.newStreamsGroupCurrentAssignmentRecord(classicGroupId, expectedMember) + ), + result.records() + ); + assertEquals( + Group.GroupType.STREAMS, + context.groupMetadataManager.streamsGroup(classicGroupId).type() + ); + } + + @Test + public void testClassicGroupJoinWithEmptyStreamsGroup() throws Exception { + String streamsGroupId = "streams-group-id"; + GroupMetadataManagerTestContext context = new GroupMetadataManagerTestContext.Builder() + .withStreamsGroup(new StreamsGroupBuilder(streamsGroupId, 10)) + .build(); + + JoinGroupRequestData request = new GroupMetadataManagerTestContext.JoinGroupRequestBuilder() + .withGroupId(streamsGroupId) + .withMemberId(UNKNOWN_MEMBER_ID) + .withDefaultProtocolTypeAndProtocols() + .build(); + GroupMetadataManagerTestContext.JoinResult joinResult = context.sendClassicGroupJoin(request, true); + + List expectedRecords = List.of( + StreamsCoordinatorRecordHelpers.newStreamsGroupTargetAssignmentEpochTombstoneRecord(streamsGroupId), + StreamsCoordinatorRecordHelpers.newStreamsGroupEpochTombstoneRecord(streamsGroupId), + StreamsCoordinatorRecordHelpers.newStreamsGroupTopologyRecordTombstone(streamsGroupId) + ); + + assertEquals(Errors.MEMBER_ID_REQUIRED.code(), joinResult.joinFuture.get().errorCode()); + assertEquals(expectedRecords, joinResult.records.subList(0, expectedRecords.size())); + assertEquals( + Group.GroupType.CLASSIC, + context.groupMetadataManager.getOrMaybeCreateClassicGroup(streamsGroupId, false).type() + ); + } + + @Test + public void testClassicGroupJoinWithNonEmptyStreamsGroup() throws Exception { + String streamsGroupId = "streams-group-id"; + String memberId = Uuid.randomUuid().toString(); + GroupMetadataManagerTestContext context = new GroupMetadataManagerTestContext.Builder() + .withStreamsGroup(new StreamsGroupBuilder(streamsGroupId, 10) + .withMember(StreamsGroupMember.Builder.withDefaults(memberId) + .setState(org.apache.kafka.coordinator.group.streams.MemberState.STABLE) + .setMemberEpoch(10) + .setPreviousMemberEpoch(10) + .build())) + .build(); + + JoinGroupRequestData request = new GroupMetadataManagerTestContext.JoinGroupRequestBuilder() + .withGroupId(streamsGroupId) + .withMemberId(UNKNOWN_MEMBER_ID) + .withDefaultProtocolTypeAndProtocols() + .build(); + + GroupMetadataManagerTestContext.JoinResult joinResult = context.sendClassicGroupJoin(request); + assertEquals(Errors.INCONSISTENT_GROUP_PROTOCOL.code(), joinResult.joinFuture.get().errorCode()); + } + @Test public void testConsumerGroupDynamicConfigs() { String groupId = "fooup"; diff --git a/release/README.md b/release/README.md index 1f56f6790112b..bde487fff355e 100644 --- a/release/README.md +++ b/release/README.md @@ -25,7 +25,7 @@ pip install -r requirements.txt # Usage -To start a release, first activate the virutalenv, and then run +To start a release, first activate the virtualenv, and then run the release script. ``` diff --git a/release/git.py b/release/git.py index 9cb106df2fdcb..02d7dc8cbe284 100644 --- a/release/git.py +++ b/release/git.py @@ -136,4 +136,3 @@ def push_ref(ref, remote=push_remote_name, **kwargs): def merge_ref(ref, **kwargs): __defaults(kwargs) cmd(f"Merging ref {ref}", f"git merge {ref}") - diff --git a/release/gpg.py b/release/gpg.py index 6b396119265c9..347389e308942 100644 --- a/release/gpg.py +++ b/release/gpg.py @@ -32,7 +32,7 @@ def key_exists(key_id): """ try: execute(f"gpg --list-keys {key_id}") - except Exception as e: + except Exception: return False return True @@ -70,13 +70,13 @@ def valid_passphrase(key_id, passphrase): with tempfile.TemporaryDirectory() as tmpdir: content = __file__ signature = tmpdir + '/sig.asc' - # if the agent is running, the suplied passphrase may be ignored + # if the agent is running, the supplied passphrase may be ignored agent_kill() try: sign(key_id, passphrase, content, signature) verify(content, signature) - except subprocess.CalledProcessError as e: - False + except subprocess.CalledProcessError: + return False return True @@ -88,5 +88,3 @@ def key_pass_id(key_id, passphrase): h.update(key_id.encode()) h.update(passphrase.encode()) return h.hexdigest() - - diff --git a/release/notes.py b/release/notes.py index 529c27b372883..e561fa03a2040 100644 --- a/release/notes.py +++ b/release/notes.py @@ -41,13 +41,13 @@ def query(query, **kwargs): Any additional keyword arguments are forwarded to jira.search_issues. """ results = [] - startAt = 0 + start_at = 0 new_results = None jira = JIRA(JIRA_BASE_URL) while new_results is None or len(new_results) == MAX_RESULTS: - new_results = jira.search_issues(query, startAt=startAt, maxResults=MAX_RESULTS, **kwargs) + new_results = jira.search_issues(query, startAt=start_at, maxResults=MAX_RESULTS, **kwargs) results += new_results - startAt += len(new_results) + start_at += len(new_results) return results @@ -172,5 +172,3 @@ def generate(version): except Exception as e: print(e, file=sys.stderr) sys.exit(1) - - diff --git a/release/preferences.py b/release/preferences.py index 9b57554585ad1..f32f1650eeddc 100644 --- a/release/preferences.py +++ b/release/preferences.py @@ -89,5 +89,3 @@ def as_json(): Export all saved preferences in JSON format. """ json.dumps(prefs, indent=2) - - diff --git a/release/release.py b/release/release.py index d0cba6f178216..92b76dee1e3ee 100644 --- a/release/release.py +++ b/release/release.py @@ -218,7 +218,7 @@ def verify_gpg_key(): if not gpg.key_exists(gpg_key_id): fail(f"GPG key {gpg_key_id} not found") if not gpg.valid_passphrase(gpg_key_id, gpg_passphrase): - fail(f"GPG passprase not valid for key {gpg_key_id}") + fail(f"GPG passphrase not valid for key {gpg_key_id}") preferences.once("verify_requirements", lambda: confirm_or_fail(templates.requirements_instructions(preferences.FILE, preferences.as_json()))) @@ -232,12 +232,12 @@ def verify_gpg_key(): jdk21_env = get_jdk(21) -def verify_prerequeisites(): +def verify_prerequisites(): print("Begin to check if you have met all the pre-requisites for the release process") def prereq(name, soft_check): try: result = soft_check() - if result == False: + if not result: fail(f"Pre-requisite not met: {name}") else: print(f"Pre-requisite met: {name}") @@ -250,7 +250,7 @@ def prereq(name, soft_check): return True -preferences.once(f"verify_prerequeisites", verify_prerequeisites) +preferences.once(f"verify_prerequisites", verify_prerequisites) # Validate that the release doesn't already exist git.fetch_tags() @@ -360,7 +360,7 @@ def delete_gitrefs(): # TODO: Many of these suggested validation steps could be automated # and would help pre-validate a lot of the stuff voters test -print(templates.sanity_check_instructions(release_version, rc_tag, apache_id)) +print(templates.sanity_check_instructions(release_version, rc_tag)) confirm_or_fail("Have you sufficiently verified the release artifacts?") # TODO: Can we close the staging repository via a REST API since we @@ -376,6 +376,5 @@ def delete_gitrefs(): git.switch_branch(starting_branch) git.delete_branch(release_version) -rc_vote_email_text = templates.rc_vote_email_text(release_version, rc, rc_tag, dev_branch, docs_release_version, apache_id) +rc_vote_email_text = templates.rc_vote_email_text(release_version, rc, rc_tag, dev_branch, docs_release_version) print(templates.rc_email_instructions(rc_vote_email_text)) - diff --git a/release/runtime.py b/release/runtime.py index c0d5d67eafb77..28765f0d92bcc 100644 --- a/release/runtime.py +++ b/release/runtime.py @@ -108,7 +108,7 @@ def _prefix(prefix_str, value_str): def cmd(action, cmd_arg, *args, **kwargs): """ - Execute an external command. This should be preferered over execute() + Execute an external command. This should be preferred over execute() when returning the output is not necessary, as the user will be given the option of retrying in case of a failure. """ @@ -144,5 +144,3 @@ def cmd(action, cmd_arg, *args, **kwargs): print(templates.cmd_failed()) fail("") - - diff --git a/release/svn.py b/release/svn.py index 4869f79ddf1a0..41c2fc6d28afc 100644 --- a/release/svn.py +++ b/release/svn.py @@ -27,7 +27,7 @@ SVN_DEV_URL="https://dist.apache.org/repos/dist/dev/kafka" -def delete_old_rc_directory_if_needed(rc_tag, src, work_dir): +def delete_old_rc_directory_if_needed(rc_tag, work_dir): svn_dev = os.path.join(work_dir, "svn_dev") cmd_desc = f"Check if {rc_tag} exists in the subversion repository." cmd_str = f"svn info --show-item revision {SVN_DEV_URL}/{rc_tag}" @@ -39,7 +39,7 @@ def delete_old_rc_directory_if_needed(rc_tag, src, work_dir): cmd(cmd_desc, cmd_str, cwd = svn_dev) def commit_artifacts(rc_tag, src, work_dir): - delete_old_rc_directory_if_needed(rc_tag, src, work_dir) + delete_old_rc_directory_if_needed(rc_tag, work_dir) svn_dev = os.path.join(work_dir, "svn_dev") dst = os.path.join(svn_dev, rc_tag) print(f"Copying {src} to {dst}") diff --git a/release/templates.py b/release/templates.py index e067f1eb475e2..2f9f07b0e3c1e 100644 --- a/release/templates.py +++ b/release/templates.py @@ -154,11 +154,11 @@ def deploy_instructions(): There will be more than one repository entries created, please close all of them. In some cases, you may get errors on some repositories while closing them, see KAFKA-15033. If this is not the first RC, you need to 'Drop' the previous artifacts. -Confirm the correct artifacts are visible at https://repository.apache.org/content/groups/staging/org/apache/kafka/ +Confirm the correct artifacts are visible at https://repository.apache.org/content/groups/staging/org/apache/kafka/ and build the +jvm and native Docker images following these instructions: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=34840886#ReleaseProcess-CreateJVMApacheKafkaDockerArtifacts(Forversions>=3.7.0) """ - -def sanity_check_instructions(release_version, rc_tag, apache_id): +def sanity_check_instructions(release_version, rc_tag): return f""" ******************************************************************************************************************************************************* Ok. We've built and staged everything for the {rc_tag}. @@ -189,14 +189,14 @@ def sanity_check_instructions(release_version, rc_tag, apache_id): """ -def rc_vote_email_text(release_version, rc, rc_tag, dev_branch, docs_version, apache_id): +def rc_vote_email_text(release_version, rc, rc_tag, dev_branch, docs_version): return f""" To: dev@kafka.apache.org, users@kafka.apache.org, kafka-clients@googlegroups.com Subject: [VOTE] {release_version} RC{rc} Hello Kafka users, developers and client-developers, -This is the first candidate for release of Apache Kafka {release_version}. +This is the candidate for release of Apache Kafka {release_version}. @@ -221,7 +221,7 @@ def rc_vote_email_text(release_version, rc, rc_tag, dev_branch, docs_version, ap https://repository.apache.org/content/groups/staging/org/apache/kafka/ * Javadoc: -https://dist.apache.org/repos/dist/dev/kafka/{rc_tag}/javadoc/ +https://dist.apache.org/repos/dist/dev/kafka/{rc_tag}/javadoc/index.html * Tag to be voted upon (off {dev_branch} branch) is the {release_version} tag: https://github.com/apache/kafka/releases/tag/{rc_tag} @@ -233,17 +233,16 @@ def rc_vote_email_text(release_version, rc, rc_tag, dev_branch, docs_version, ap https://kafka.apache.org/{docs_version}/protocol.html * Successful CI builds for the {dev_branch} branch: -Unit/integration tests: https://ci-builds.apache.org/job/Kafka/job/kafka/job/{dev_branch}// --- Confluent engineers can access the semphore build to provide the build number -System tests: https://confluent-open-source-kafka-system-test-results.s3-us-west-2.amazonaws.com/{dev_branch}//report.html +Unit/integration tests: https://github.com/apache/kafka/actions/runs/ +System tests: +/report.html> * Successful Docker Image Github Actions Pipeline for {dev_branch} branch: Docker Build Test Pipeline (JVM): https://github.com/apache/kafka/actions/runs/ Docker Build Test Pipeline (Native): https://github.com/apache/kafka/actions/runs/ -/************************************** - Thanks, """ @@ -294,5 +293,3 @@ def release_announcement_email_instructions(release_announcement_email): - Finally, validate all the links before shipping! Note that all substitutions are annotated with <> around them. """ - - diff --git a/release/textfiles.py b/release/textfiles.py index 7598b96067a55..20752659c4bd2 100644 --- a/release/textfiles.py +++ b/release/textfiles.py @@ -71,5 +71,3 @@ def replace(path, pattern, replacement, **kwargs): with open(path, "w") as f: for line in updated: f.write(line) - - diff --git a/server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java b/server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java index dd7c5937bdc9a..ceca9a6a7de92 100644 --- a/server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java +++ b/server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java @@ -283,7 +283,9 @@ public short fetchRequestVersion() { } public short listOffsetRequestVersion() { - if (this.isAtLeast(IBP_4_0_IV3)) { + if (this.isAtLeast(IBP_4_2_IV1)) { + return 11; + } else if (this.isAtLeast(IBP_4_0_IV3)) { return 10; } else if (this.isAtLeast(IBP_3_9_IV0)) { return 9; diff --git a/server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java b/server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java index 508d4bd900bb1..49a200f622521 100644 --- a/server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java +++ b/server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java @@ -266,8 +266,8 @@ public void assertLatestIsNotProduction() { @ParameterizedTest @EnumSource(value = MetadataVersion.class) public void testListOffsetsValueVersion(MetadataVersion metadataVersion) { - final short expectedVersion = 10; - if (metadataVersion.isAtLeast(IBP_4_0_IV3)) { + final short expectedVersion = 11; + if (metadataVersion.isAtLeast(IBP_4_2_IV1)) { assertEquals(expectedVersion, metadataVersion.listOffsetRequestVersion()); } else { assertTrue(metadataVersion.listOffsetRequestVersion() < expectedVersion); diff --git a/server/src/main/java/org/apache/kafka/server/share/acknowledge/ShareAcknowledgementBatch.java b/server/src/main/java/org/apache/kafka/server/share/acknowledge/ShareAcknowledgementBatch.java index bc29f37c62f98..96efa5eb29e8c 100644 --- a/server/src/main/java/org/apache/kafka/server/share/acknowledge/ShareAcknowledgementBatch.java +++ b/server/src/main/java/org/apache/kafka/server/share/acknowledge/ShareAcknowledgementBatch.java @@ -24,6 +24,9 @@ * The class abstracts the acknowledgement request for SharePartition class constructed * from {@link org.apache.kafka.common.message.ShareFetchRequestData.AcknowledgementBatch} and * {@link org.apache.kafka.common.message.ShareAcknowledgeRequestData.AcknowledgementBatch} classes. + *

+ * Acknowledge types are represented as a list of bytes, where each byte corresponds to an acknowledge + * type defined in {@link org.apache.kafka.clients.consumer.AcknowledgeType}. */ public record ShareAcknowledgementBatch( long firstOffset, diff --git a/storage/src/main/java/org/apache/kafka/storage/internals/log/UnifiedLog.java b/storage/src/main/java/org/apache/kafka/storage/internals/log/UnifiedLog.java index ca32e4f086a61..769f59d56dcec 100644 --- a/storage/src/main/java/org/apache/kafka/storage/internals/log/UnifiedLog.java +++ b/storage/src/main/java/org/apache/kafka/storage/internals/log/UnifiedLog.java @@ -1667,6 +1667,8 @@ public OffsetResultHolder fetchOffsetByTimestamp(long targetTimestamp, Optional< } else { return new OffsetResultHolder(new FileRecords.TimestampAndOffset(RecordBatch.NO_TIMESTAMP, -1L, Optional.of(-1))); } + } else if (targetTimestamp == ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) { + return fetchEarliestPendingUploadOffset(remoteOffsetReader); } else if (targetTimestamp == ListOffsetsRequest.MAX_TIMESTAMP) { // Cache to avoid race conditions. List segments = logSegments(); @@ -1709,6 +1711,31 @@ public OffsetResultHolder fetchOffsetByTimestamp(long targetTimestamp, Optional< }); } + private OffsetResultHolder fetchEarliestPendingUploadOffset(Optional remoteOffsetReader) { + if (remoteLogEnabled()) { + long curHighestRemoteOffset = highestOffsetInRemoteStorage(); + + if (curHighestRemoteOffset == -1L) { + if (localLogStartOffset() == logStartOffset()) { + // No segments have been uploaded yet + return fetchOffsetByTimestamp(ListOffsetsRequest.EARLIEST_TIMESTAMP, remoteOffsetReader); + } else { + // Leader currently does not know about the already uploaded segments + return new OffsetResultHolder(Optional.of(new FileRecords.TimestampAndOffset(RecordBatch.NO_TIMESTAMP, -1L, Optional.of(-1)))); + } + } else { + long earliestPendingUploadOffset = Math.max(curHighestRemoteOffset + 1, logStartOffset()); + OptionalInt epochForOffset = leaderEpochCache.epochForOffset(earliestPendingUploadOffset); + Optional epochResult = epochForOffset.isPresent() + ? Optional.of(epochForOffset.getAsInt()) + : Optional.empty(); + return new OffsetResultHolder(new FileRecords.TimestampAndOffset(RecordBatch.NO_TIMESTAMP, earliestPendingUploadOffset, epochResult)); + } + } else { + return new OffsetResultHolder(new FileRecords.TimestampAndOffset(RecordBatch.NO_TIMESTAMP, -1L, Optional.of(-1))); + } + } + /** * Checks if the log is empty. * @return Returns True when the log is empty. Otherwise, false. diff --git a/tests/kafkatest/services/verifiable_client.py b/tests/kafkatest/services/verifiable_client.py index 16617d621aab1..b6c192673282a 100644 --- a/tests/kafkatest/services/verifiable_client.py +++ b/tests/kafkatest/services/verifiable_client.py @@ -74,7 +74,8 @@ * `--enable-autocommit` * `--max-messages ` * `--assignment-strategy ` - * `--consumer.config ` - consumer config properties (typically empty) + * `--consumer.config ` - (DEPRECATED) consumer config properties (typically empty). This option will be removed in a future version. Use --command-config instead. + * `--command-config ` - command config properties Environment variables: * `LOG_DIR` - log output directory. Typically not needed if logs are written to stderr. @@ -97,7 +98,8 @@ * `--broker-list ` * `--max-messages ` * `--throughput ` - * `--producer.config ` - producer config properties (typically empty) + * `--producer.config ` - producer config properties (typically empty). This option will be removed in a future version. Use --command-config instead. + * `--command-config ` - command config properties Environment variables: * `LOG_DIR` - log output directory. Typically not needed if logs are written to stderr. diff --git a/tests/kafkatest/services/verifiable_consumer.py b/tests/kafkatest/services/verifiable_consumer.py index 8264566f1c2b9..99c403ce190f0 100644 --- a/tests/kafkatest/services/verifiable_consumer.py +++ b/tests/kafkatest/services/verifiable_consumer.py @@ -424,7 +424,7 @@ def start_cmd(self, node): if self.max_messages > 0: cmd += " --max-messages %s" % str(self.max_messages) - cmd += " --consumer.config %s" % VerifiableConsumer.CONFIG_FILE + cmd += " --command-config %s" % VerifiableConsumer.CONFIG_FILE cmd += " 2>> %s | tee -a %s &" % (VerifiableConsumer.STDOUT_CAPTURE, VerifiableConsumer.STDOUT_CAPTURE) return cmd diff --git a/tests/kafkatest/services/verifiable_producer.py b/tests/kafkatest/services/verifiable_producer.py index e7a8f411f3a0e..049de4ce98619 100644 --- a/tests/kafkatest/services/verifiable_producer.py +++ b/tests/kafkatest/services/verifiable_producer.py @@ -249,7 +249,7 @@ def start_cmd(self, node, idx): if self.repeating_keys is not None: cmd += " --repeating-keys %s " % str(self.repeating_keys) - cmd += " --producer.config %s" % VerifiableProducer.CONFIG_FILE + cmd += " --command-config %s" % VerifiableProducer.CONFIG_FILE cmd += " 2>> %s | tee -a %s &" % (VerifiableProducer.STDOUT_CAPTURE, VerifiableProducer.STDOUT_CAPTURE) return cmd diff --git a/tools/src/main/java/org/apache/kafka/tools/GetOffsetShell.java b/tools/src/main/java/org/apache/kafka/tools/GetOffsetShell.java index 8cc9428afbdbd..ae16d11d8edea 100644 --- a/tools/src/main/java/org/apache/kafka/tools/GetOffsetShell.java +++ b/tools/src/main/java/org/apache/kafka/tools/GetOffsetShell.java @@ -126,7 +126,7 @@ public GetOffsetShellOptions(String[] args) throws TerseException { .ofType(String.class); timeOpt = parser.accepts("time", "timestamp of the offsets before that. [Note: No offset is returned, if the timestamp greater than recently committed record timestamp is given.]") .withRequiredArg() - .describedAs(" / -1 or latest / -2 or earliest / -3 or max-timestamp / -4 or earliest-local / -5 or latest-tiered") + .describedAs(" / -1 or latest / -2 or earliest / -3 or max-timestamp / -4 or earliest-local / -5 or latest-tiered / -6 or earliest-pending-upload") .ofType(String.class) .defaultsTo("latest"); commandConfigOpt = parser.accepts("command-config", "Property file containing configs to be passed to Admin Client.") @@ -276,6 +276,8 @@ static OffsetSpec parseOffsetSpec(String listOffsetsTimestamp) throws TerseExcep return OffsetSpec.earliestLocal(); case "latest-tiered": return OffsetSpec.latestTiered(); + case "earliest-pending-upload": + return OffsetSpec.earliestPendingUpload(); default: long timestamp; @@ -283,7 +285,7 @@ static OffsetSpec parseOffsetSpec(String listOffsetsTimestamp) throws TerseExcep timestamp = Long.parseLong(listOffsetsTimestamp); } catch (NumberFormatException e) { throw new TerseException("Malformed time argument " + listOffsetsTimestamp + ". " + - "Please use -1 or latest / -2 or earliest / -3 or max-timestamp / -4 or earliest-local / -5 or latest-tiered, or a specified long format timestamp"); + "Please use -1 or latest / -2 or earliest / -3 or max-timestamp / -4 or earliest-local / -5 or latest-tiered / -6 or earliest-pending-upload, or a specified long format timestamp"); } if (timestamp == ListOffsetsRequest.EARLIEST_TIMESTAMP) { @@ -296,6 +298,8 @@ static OffsetSpec parseOffsetSpec(String listOffsetsTimestamp) throws TerseExcep return OffsetSpec.earliestLocal(); } else if (timestamp == ListOffsetsRequest.LATEST_TIERED_TIMESTAMP) { return OffsetSpec.latestTiered(); + } else if (timestamp == ListOffsetsRequest.EARLIEST_PENDING_UPLOAD_TIMESTAMP) { + return OffsetSpec.earliestPendingUpload(); } else { return OffsetSpec.forTimestamp(timestamp); } diff --git a/tools/src/main/java/org/apache/kafka/tools/VerifiableConsumer.java b/tools/src/main/java/org/apache/kafka/tools/VerifiableConsumer.java index 1fa1aed8c4f08..1ecea5331ddf8 100644 --- a/tools/src/main/java/org/apache/kafka/tools/VerifiableConsumer.java +++ b/tools/src/main/java/org/apache/kafka/tools/VerifiableConsumer.java @@ -597,7 +597,7 @@ private static ArgumentParser argParser() { .setDefault("earliest") .type(String.class) .dest("resetPolicy") - .help("Set reset policy (must be either 'earliest', 'latest', or 'none'"); + .help("Set reset policy (must be either 'earliest', 'latest', or 'none')"); parser.addArgument("--assignment-strategy") .action(store()) @@ -611,8 +611,17 @@ private static ArgumentParser argParser() { .action(store()) .required(false) .type(String.class) - .metavar("CONFIG_FILE") - .help("Consumer config properties file (config options shared with command line parameters will be overridden)."); + .metavar("CONFIG-FILE") + .help("(DEPRECATED) Consumer config properties file" + + "This option will be removed in a future version. Use --command-config instead"); + + parser.addArgument("--command-config") + .action(store()) + .required(false) + .type(String.class) + .metavar("CONFIG-FILE") + .dest("commandConfigFile") + .help("Config properties file (config options shared with command line parameters will be overridden)."); return parser; } @@ -622,16 +631,28 @@ public static VerifiableConsumer createFromArgs(ArgumentParser parser, String[] boolean useAutoCommit = res.getBoolean("useAutoCommit"); String configFile = res.getString("consumer.config"); + String commandConfigFile = res.getString("commandConfigFile"); String brokerHostAndPort = res.getString("bootstrapServer"); Properties consumerProps = new Properties(); + if (configFile != null && commandConfigFile != null) { + throw new ArgumentParserException("Options --consumer.config and --command-config are mutually exclusive.", parser); + } if (configFile != null) { + System.out.println("Option --consumer.config has been deprecated and will be removed in a future version. Use --command-config instead."); try { consumerProps.putAll(Utils.loadProps(configFile)); } catch (IOException e) { throw new ArgumentParserException(e.getMessage(), parser); } } + if (commandConfigFile != null) { + try { + consumerProps.putAll(Utils.loadProps(res.getString(commandConfigFile))); + } catch (IOException e) { + throw new ArgumentParserException(e.getMessage(), parser); + } + } GroupProtocol groupProtocol = GroupProtocol.of(res.getString("groupProtocol")); consumerProps.put(ConsumerConfig.GROUP_PROTOCOL_CONFIG, groupProtocol.name()); diff --git a/tools/src/main/java/org/apache/kafka/tools/VerifiableProducer.java b/tools/src/main/java/org/apache/kafka/tools/VerifiableProducer.java index 33f0b3142e8fd..bfbbb8a485417 100644 --- a/tools/src/main/java/org/apache/kafka/tools/VerifiableProducer.java +++ b/tools/src/main/java/org/apache/kafka/tools/VerifiableProducer.java @@ -161,8 +161,9 @@ private static ArgumentParser argParser() { .action(store()) .required(false) .type(String.class) - .metavar("CONFIG_FILE") - .help("Producer config properties file."); + .metavar("CONFIG-FILE") + .help("(DEPRECATED) Producer config properties file. " + + "This option will be removed in a future version. Use --command-config instead."); parser.addArgument("--message-create-time") .action(store()) @@ -189,6 +190,14 @@ private static ArgumentParser argParser() { .dest("repeatingKeys") .help("If specified, each produced record will have a key starting at 0 increment by 1 up to the number specified (exclusive), then the key is set to 0 again"); + parser.addArgument("--command-config") + .action(store()) + .required(false) + .type(String.class) + .metavar("CONFIG-FILE") + .dest("commandConfigFile") + .help("Config properties file (config options shared with command line parameters will be overridden)."); + return parser; } @@ -217,6 +226,7 @@ public static VerifiableProducer createFromArgs(ArgumentParser parser, String[] String topic = res.getString("topic"); int throughput = res.getInt("throughput"); String configFile = res.getString("producer.config"); + String commandConfigFile = res.getString("commandConfigFile"); Integer valuePrefix = res.getInt("valuePrefix"); Long createTime = res.getLong("createTime"); Integer repeatingKeys = res.getInt("repeatingKeys"); @@ -240,14 +250,25 @@ public static VerifiableProducer createFromArgs(ArgumentParser parser, String[] producerProps.put(ProducerConfig.ACKS_CONFIG, Integer.toString(res.getInt("acks"))); // No producer retries producerProps.put(ProducerConfig.RETRIES_CONFIG, "0"); + if (configFile != null && commandConfigFile != null) { + throw new ArgumentParserException("Options --producer.config and --command-config are mutually exclusive.", parser); + } + if (configFile != null) { + System.out.println("Option --producer.config has been deprecated and will be removed in a future version. Use --command-config instead."); try { producerProps.putAll(loadProps(configFile)); } catch (IOException e) { throw new ArgumentParserException(e.getMessage(), parser); } } - + if (commandConfigFile != null) { + try { + producerProps.putAll(loadProps(commandConfigFile)); + } catch (IOException e) { + throw new ArgumentParserException(e.getMessage(), parser); + } + } StringSerializer serializer = new StringSerializer(); KafkaProducer producer = new KafkaProducer<>(producerProps, serializer, serializer); diff --git a/tools/src/main/java/org/apache/kafka/tools/streams/StreamsGroupCommand.java b/tools/src/main/java/org/apache/kafka/tools/streams/StreamsGroupCommand.java index 0f68bf8290053..0c54f6c53f99d 100644 --- a/tools/src/main/java/org/apache/kafka/tools/streams/StreamsGroupCommand.java +++ b/tools/src/main/java/org/apache/kafka/tools/streams/StreamsGroupCommand.java @@ -881,6 +881,7 @@ private Collection getPartitionsToReset(String groupId) throws E List topics = opts.options.valuesOf(opts.inputTopicOpt); List partitions = offsetsUtils.parseTopicPartitionsToReset(topics); + offsetsUtils.checkAllTopicPartitionsValid(partitions); // if the user specified topics that do not belong to this group, we filter them out partitions = filterExistingGroupTopics(groupId, partitions); return partitions; diff --git a/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellParsingTest.java b/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellParsingTest.java index 53c1c4d79c95b..db53695a7be28 100644 --- a/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellParsingTest.java +++ b/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellParsingTest.java @@ -247,7 +247,7 @@ public void testInvalidTimeValue() { @Test public void testInvalidOffset() { assertEquals("Malformed time argument foo. " + - "Please use -1 or latest / -2 or earliest / -3 or max-timestamp / -4 or earliest-local / -5 or latest-tiered, or a specified long format timestamp", + "Please use -1 or latest / -2 or earliest / -3 or max-timestamp / -4 or earliest-local / -5 or latest-tiered / -6 or earliest-pending-upload, or a specified long format timestamp", assertThrows(TerseException.class, () -> GetOffsetShell.parseOffsetSpec("foo")).getMessage()); } diff --git a/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java b/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java index 9986daa7f3b0e..c1c7b27639aaf 100644 --- a/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java +++ b/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java @@ -367,6 +367,30 @@ public void testGetOffsetsByLatestTieredSpec() throws InterruptedException { } } + @ClusterTemplate("withRemoteStorage") + public void testGetOffsetsByEarliestTieredSpec() throws InterruptedException { + setUp(); + setUpRemoteLogTopics(); + + for (String time : new String[] {"-6", "earliest-pending-upload"}) { + // test topics disable remote log storage + // as remote log disabled, broker returns unknown offset of each topic partition and these + // unknown offsets are ignore by GetOffsetShell, hence we have empty result here. + assertEquals(List.of(), + executeAndParse("--topic-partitions", "topic\\d+:0", "--time", time)); + + // test topics enable remote log storage + TestUtils.waitForCondition(() -> + List.of( + new Row("topicRLS1", 0, 0L), + new Row("topicRLS2", 0, 1L), + new Row("topicRLS3", 0, 2L), + new Row("topicRLS4", 0, 3L)) + .equals(executeAndParse("--topic-partitions", "topicRLS.*:0", "--time", time)), + "testGetOffsetsByEarliestTieredSpec result not match"); + } + } + @ClusterTest public void testGetOffsetsByTimestamp() { setUp(); diff --git a/tools/src/test/java/org/apache/kafka/tools/consumer/group/ShareGroupCommandTest.java b/tools/src/test/java/org/apache/kafka/tools/consumer/group/ShareGroupCommandTest.java index b14d66c652ab6..9333bbbb65e12 100644 --- a/tools/src/test/java/org/apache/kafka/tools/consumer/group/ShareGroupCommandTest.java +++ b/tools/src/test/java/org/apache/kafka/tools/consumer/group/ShareGroupCommandTest.java @@ -1373,7 +1373,7 @@ public void testAlterShareGroupOffsetsArgsFailureWithoutResetOffsetsArgs() { } @Test - public void testAlterShareGroupFailureFailureWithNonExistentTopic() { + public void testAlterShareGroupFailureWithNonExistentTopic() { String group = "share-group"; String topic = "none"; String bootstrapServer = "localhost:9092"; diff --git a/tools/src/test/java/org/apache/kafka/tools/streams/StreamsGroupCommandTest.java b/tools/src/test/java/org/apache/kafka/tools/streams/StreamsGroupCommandTest.java index 6f38c47f15aa2..4f1e116437e63 100644 --- a/tools/src/test/java/org/apache/kafka/tools/streams/StreamsGroupCommandTest.java +++ b/tools/src/test/java/org/apache/kafka/tools/streams/StreamsGroupCommandTest.java @@ -43,6 +43,7 @@ import org.apache.kafka.common.Node; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.TopicPartitionInfo; +import org.apache.kafka.common.errors.UnknownTopicOrPartitionException; import org.apache.kafka.common.internals.KafkaFutureImpl; import org.apache.kafka.test.TestUtils; @@ -65,6 +66,7 @@ import joptsimple.OptionException; +import static org.apache.kafka.common.KafkaFuture.completedFuture; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -72,6 +74,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -293,21 +296,30 @@ public void testGroupStatesFromString() { @Test public void testAdminRequestsForResetOffsets() { Admin adminClient = mock(KafkaAdminClient.class); + String topic = "topic1"; String groupId = "foo-group"; List args = List.of("--bootstrap-server", "localhost:9092", "--group", groupId, "--reset-offsets", "--input-topic", "topic1", "--to-latest"); - List topics = List.of("topic1"); + List topics = List.of(topic); + DescribeTopicsResult describeTopicsResult = mock(DescribeTopicsResult.class); when(adminClient.describeStreamsGroups(List.of(groupId))) .thenReturn(describeStreamsResult(groupId, GroupState.DEAD)); + Map descriptions = Map.of( + topic, new TopicDescription(topic, false, List.of( + new TopicPartitionInfo(0, Node.noNode(), List.of(), List.of())) + )); + when(adminClient.describeTopics(anyCollection())) + .thenReturn(describeTopicsResult); when(adminClient.describeTopics(eq(topics), any(DescribeTopicsOptions.class))) - .thenReturn(describeTopicsResult(topics, 1)); + .thenReturn(describeTopicsResult); + when(describeTopicsResult.allTopicNames()).thenReturn(completedFuture(descriptions)); when(adminClient.listOffsets(any(), any())) .thenReturn(listOffsetsResult()); ListGroupsResult listGroupsResult = listGroupResult(groupId); when(adminClient.listGroups(any(ListGroupsOptions.class))).thenReturn(listGroupsResult); ListStreamsGroupOffsetsResult result = mock(ListStreamsGroupOffsetsResult.class); Map committedOffsetsMap = new HashMap<>(); - committedOffsetsMap.put(new TopicPartition("topic1", 0), mock(OffsetAndMetadata.class)); + committedOffsetsMap.put(new TopicPartition(topic, 0), mock(OffsetAndMetadata.class)); when(adminClient.listStreamsGroupOffsets(ArgumentMatchers.anyMap())).thenReturn(result); when(result.partitionsToOffsetAndMetadata(ArgumentMatchers.anyString())).thenReturn(KafkaFuture.completedFuture(committedOffsetsMap)); @@ -427,6 +439,43 @@ public void testDeleteNonStreamsGroup() { service.close(); } + + @Test + public void testResetOffsetsWithPartitionNotExist() { + Admin adminClient = mock(KafkaAdminClient.class); + String groupId = "foo-group"; + String topic = "topic"; + List args = new ArrayList<>(Arrays.asList("--bootstrap-server", "localhost:9092", "--group", groupId, "--reset-offsets", "--input-topic", "topic:3", "--to-latest")); + + when(adminClient.describeStreamsGroups(List.of(groupId))) + .thenReturn(describeStreamsResult(groupId, GroupState.DEAD)); + DescribeTopicsResult describeTopicsResult = mock(DescribeTopicsResult.class); + + Map descriptions = Map.of( + topic, new TopicDescription(topic, false, List.of( + new TopicPartitionInfo(0, Node.noNode(), List.of(), List.of())) + )); + when(adminClient.describeTopics(anyCollection())) + .thenReturn(describeTopicsResult); + when(adminClient.describeTopics(eq(List.of(topic)), any(DescribeTopicsOptions.class))) + .thenReturn(describeTopicsResult); + when(describeTopicsResult.allTopicNames()).thenReturn(completedFuture(descriptions)); + when(adminClient.listOffsets(any(), any())) + .thenReturn(listOffsetsResult()); + ListStreamsGroupOffsetsResult result = mock(ListStreamsGroupOffsetsResult.class); + Map committedOffsetsMap = Map.of( + new TopicPartition(topic, 0), + new OffsetAndMetadata(12, Optional.of(0), ""), + new TopicPartition(topic, 1), + new OffsetAndMetadata(12, Optional.of(0), "") + ); + + when(adminClient.listStreamsGroupOffsets(ArgumentMatchers.anyMap())).thenReturn(result); + when(result.partitionsToOffsetAndMetadata(ArgumentMatchers.anyString())).thenReturn(KafkaFuture.completedFuture(committedOffsetsMap)); + StreamsGroupCommand.StreamsGroupService service = getStreamsGroupService(args.toArray(new String[0]), adminClient); + assertThrows(UnknownTopicOrPartitionException.class, () -> service.resetOffsets()); + service.close(); + } private ListGroupsResult listGroupResult(String groupId) { ListGroupsResult listGroupsResult = mock(ListGroupsResult.class); diff --git a/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TransactionMetadata.java b/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TransactionMetadata.java index 96a92dd01c993..8efeedc3ec43d 100644 --- a/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TransactionMetadata.java +++ b/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TransactionMetadata.java @@ -117,6 +117,7 @@ public T inLock(Supplier function) { } } + // VisibleForTesting public void addPartitions(Collection partitions) { topicPartitions.addAll(partitions); } @@ -500,6 +501,7 @@ public String transactionalId() { return transactionalId; } + // VisibleForTesting public void setProducerId(long producerId) { this.producerId = producerId; } @@ -507,6 +509,7 @@ public long producerId() { return producerId; } + // VisibleForTesting public void setPrevProducerId(long prevProducerId) { this.prevProducerId = prevProducerId; } @@ -534,6 +537,7 @@ public int txnTimeoutMs() { return txnTimeoutMs; } + // VisibleForTesting public void state(TransactionState state) { this.state = state; } @@ -550,6 +554,7 @@ public long txnStartTimestamp() { return txnStartTimestamp; } + // VisibleForTesting public void txnLastUpdateTimestamp(long txnLastUpdateTimestamp) { this.txnLastUpdateTimestamp = txnLastUpdateTimestamp; } diff --git a/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TxnTransitMetadata.java b/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TxnTransitMetadata.java index 452c168687e56..b2e78d45da37e 100644 --- a/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TxnTransitMetadata.java +++ b/transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TxnTransitMetadata.java @@ -22,7 +22,7 @@ import java.util.HashSet; /** - * Immutable object representing the target transition of the transaction metadata + * Represent the target transition of the transaction metadata. The topicPartitions field is mutable. */ public record TxnTransitMetadata( long producerId,