From 90690173d2c0f189180ec8c0b74479c821b5c410 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Wed, 20 Aug 2025 15:57:07 +0800 Subject: [PATCH 01/14] KAFKA-19260; Move AutoTopicCreationManager to server module --- .../server/builders/KafkaApisBuilder.java | 2 +- .../server/AutoTopicCreationManager.scala | 279 ----------- .../scala/kafka/server/BrokerServer.scala | 4 +- .../kafka/server/ForwardingManager.scala | 29 +- .../main/scala/kafka/server/KafkaApis.scala | 37 +- .../main/scala/kafka/server/KafkaConfig.scala | 2 - .../server/AutoTopicCreationManagerTest.scala | 396 --------------- .../unit/kafka/server/KafkaApisTest.scala | 28 +- .../KRaftMetadataRequestBenchmark.java | 2 +- .../server/AutoTopicCreationManager.java | 58 +++ .../DefaultAutoTopicCreationManager.java | 268 ++++++++++ .../kafka/server/ForwardingManagerUtils.java | 42 ++ .../server/config/AbstractKafkaConfig.java | 8 + .../server/AutoTopicCreationManagerTest.java | 458 ++++++++++++++++++ 14 files changed, 876 insertions(+), 737 deletions(-) delete mode 100644 core/src/main/scala/kafka/server/AutoTopicCreationManager.scala delete mode 100644 core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala create mode 100644 server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java create mode 100644 server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java create mode 100644 server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java create mode 100644 server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java diff --git a/core/src/main/java/kafka/server/builders/KafkaApisBuilder.java b/core/src/main/java/kafka/server/builders/KafkaApisBuilder.java index 5e36641d54dd6..374d6bfb341b3 100644 --- a/core/src/main/java/kafka/server/builders/KafkaApisBuilder.java +++ b/core/src/main/java/kafka/server/builders/KafkaApisBuilder.java @@ -19,7 +19,6 @@ import kafka.coordinator.transaction.TransactionCoordinator; import kafka.network.RequestChannel; -import kafka.server.AutoTopicCreationManager; import kafka.server.FetchManager; import kafka.server.ForwardingManager; import kafka.server.KafkaApis; @@ -37,6 +36,7 @@ import org.apache.kafka.metadata.ConfigRepository; import org.apache.kafka.metadata.MetadataCache; import org.apache.kafka.server.ApiVersionManager; +import org.apache.kafka.server.AutoTopicCreationManager; import org.apache.kafka.server.ClientMetricsManager; import org.apache.kafka.server.DelegationTokenManager; import org.apache.kafka.server.authorizer.Authorizer; diff --git a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala b/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala deleted file mode 100644 index 7b98c8c16fbb9..0000000000000 --- a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala +++ /dev/null @@ -1,279 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.util.concurrent.ConcurrentHashMap -import java.util.{Collections, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.Logging -import org.apache.kafka.clients.ClientResponse -import org.apache.kafka.common.errors.InvalidTopicException -import org.apache.kafka.common.internals.Topic -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.CreateTopicsRequestData -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.protocol.{ApiKeys, Errors} -import org.apache.kafka.common.requests.{CreateTopicsRequest, CreateTopicsResponse, RequestContext, RequestHeader} -import org.apache.kafka.coordinator.group.GroupCoordinator -import org.apache.kafka.coordinator.share.ShareCoordinator -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.common.{ControllerRequestCompletionHandler, NodeToControllerChannelManager} -import org.apache.kafka.server.quota.ControllerMutationQuota - -import scala.collection.{Map, Seq, Set, mutable} -import scala.jdk.CollectionConverters._ -import scala.jdk.OptionConverters.RichOptional - -trait AutoTopicCreationManager { - - def createTopics( - topicNames: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] - - def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext - ): Unit - -} - -class DefaultAutoTopicCreationManager( - config: KafkaConfig, - channelManager: NodeToControllerChannelManager, - groupCoordinator: GroupCoordinator, - txnCoordinator: TransactionCoordinator, - shareCoordinator: ShareCoordinator -) extends AutoTopicCreationManager with Logging { - - private val inflightTopics = Collections.newSetFromMap(new ConcurrentHashMap[String, java.lang.Boolean]()) - - /** - * Initiate auto topic creation for the given topics. - * - * @param topics the topics to create - * @param controllerMutationQuota the controller mutation quota for topic creation - * @param metadataRequestContext defined when creating topics on behalf of the client. The goal here is to preserve - * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest - * inside Envelope to send to the controller when forwarding is enabled. - * @return auto created topic metadata responses - */ - override def createTopics( - topics: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val (creatableTopics, uncreatableTopicResponses) = filterCreatableTopics(topics) - - val creatableTopicResponses = if (creatableTopics.isEmpty) { - Seq.empty - } else { - sendCreateTopicRequest(creatableTopics, metadataRequestContext) - } - - uncreatableTopicResponses ++ creatableTopicResponses - } - - override def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext - ): Unit = { - - for ((_, creatableTopic) <- topics) { - if (creatableTopic.numPartitions() == -1) { - creatableTopic - .setNumPartitions(config.numPartitions) - } - if (creatableTopic.replicationFactor() == -1) { - creatableTopic - .setReplicationFactor(config.defaultReplicationFactor.shortValue) - } - } - - if (topics.nonEmpty) { - sendCreateTopicRequest(topics, Some(requestContext)) - } - } - - private def sendCreateTopicRequest( - creatableTopics: Map[String, CreatableTopic], - requestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val topicsToCreate = new CreateTopicsRequestData.CreatableTopicCollection(creatableTopics.size) - topicsToCreate.addAll(creatableTopics.values.asJavaCollection) - - val createTopicsRequest = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTimeoutMs(config.requestTimeoutMs) - .setTopics(topicsToCreate) - ) - - val requestCompletionHandler = new ControllerRequestCompletionHandler { - override def onTimeout(): Unit = { - clearInflightRequests(creatableTopics) - debug(s"Auto topic creation timed out for ${creatableTopics.keys}.") - } - - override def onComplete(response: ClientResponse): Unit = { - clearInflightRequests(creatableTopics) - if (response.authenticationException() != null) { - warn(s"Auto topic creation failed for ${creatableTopics.keys} with authentication exception") - } else if (response.versionMismatch() != null) { - warn(s"Auto topic creation failed for ${creatableTopics.keys} with invalid version exception") - } else { - if (response.hasResponse) { - response.responseBody() match { - case createTopicsResponse: CreateTopicsResponse => - createTopicsResponse.data().topics().forEach(topicResult => { - val error = Errors.forCode(topicResult.errorCode) - if (error != Errors.NONE) { - warn(s"Auto topic creation failed for ${topicResult.name} with error '${error.name}': ${topicResult.errorMessage}") - } - }) - case other => - warn(s"Auto topic creation request received unexpected response type: ${other.getClass.getSimpleName}") - } - } - debug(s"Auto topic creation completed for ${creatableTopics.keys} with response ${response.responseBody}.") - } - } - } - - val request = requestContext.map { context => - val requestVersion = - channelManager.controllerApiVersions.toScala match { - case None => - // We will rely on the Metadata request to be retried in the case - // that the latest version is not usable by the controller. - ApiKeys.CREATE_TOPICS.latestVersion() - case Some(nodeApiVersions) => - nodeApiVersions.latestUsableVersion(ApiKeys.CREATE_TOPICS) - } - - // Borrow client information such as client id and correlation id from the original request, - // in order to correlate the create request with the original metadata request. - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, - requestVersion, - context.clientId, - context.correlationId) - ForwardingManager.buildEnvelopeRequest(context, - createTopicsRequest.build(requestVersion).serializeWithHeader(requestHeader)) - }.getOrElse(createTopicsRequest) - - channelManager.sendRequest(request, requestCompletionHandler) - - val creatableTopicResponses = creatableTopics.keySet.toSeq.map { topic => - new MetadataResponseTopic() - .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - } - - info(s"Sent auto-creation request for ${creatableTopics.keys} to the active controller.") - creatableTopicResponses - } - - private def clearInflightRequests(creatableTopics: Map[String, CreatableTopic]): Unit = { - creatableTopics.keySet.foreach(inflightTopics.remove) - debug(s"Cleared inflight topic creation state for $creatableTopics") - } - - private def creatableTopic(topic: String): CreatableTopic = { - topic match { - case GROUP_METADATA_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.groupCoordinatorConfig.offsetsTopicPartitions) - .setReplicationFactor(config.groupCoordinatorConfig.offsetsTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections(groupCoordinator.groupMetadataTopicConfigs)) - case TRANSACTION_STATE_TOPIC_NAME => - val transactionLogConfig = new TransactionLogConfig(config) - new CreatableTopic() - .setName(topic) - .setNumPartitions(transactionLogConfig.transactionTopicPartitions) - .setReplicationFactor(transactionLogConfig.transactionTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections( - txnCoordinator.transactionTopicConfigs)) - case SHARE_GROUP_STATE_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.shareCoordinatorConfig.shareCoordinatorStateTopicNumPartitions()) - .setReplicationFactor(config.shareCoordinatorConfig.shareCoordinatorStateTopicReplicationFactor()) - .setConfigs(convertToTopicConfigCollections(shareCoordinator.shareGroupStateTopicConfigs())) - case topicName => - new CreatableTopic() - .setName(topicName) - .setNumPartitions(config.numPartitions) - .setReplicationFactor(config.defaultReplicationFactor.shortValue) - } - } - - private def convertToTopicConfigCollections(config: Properties): CreatableTopicConfigCollection = { - val topicConfigs = new CreatableTopicConfigCollection() - config.forEach { - case (name, value) => - topicConfigs.add(new CreatableTopicConfig() - .setName(name.toString) - .setValue(value.toString)) - } - topicConfigs - } - - private def isValidTopicName(topic: String): Boolean = { - try { - Topic.validate(topic) - true - } catch { - case _: InvalidTopicException => - false - } - } - - private def filterCreatableTopics( - topics: Set[String] - ): (Map[String, CreatableTopic], Seq[MetadataResponseTopic]) = { - - val creatableTopics = mutable.Map.empty[String, CreatableTopic] - val uncreatableTopics = mutable.Buffer.empty[MetadataResponseTopic] - - topics.foreach { topic => - // Attempt basic topic validation before sending any requests to the controller. - val validationError: Option[Errors] = if (!isValidTopicName(topic)) { - Some(Errors.INVALID_TOPIC_EXCEPTION) - } else if (!inflightTopics.add(topic)) { - Some(Errors.UNKNOWN_TOPIC_OR_PARTITION) - } else { - None - } - - validationError match { - case Some(error) => - uncreatableTopics += new MetadataResponseTopic() - .setErrorCode(error.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - case None => - creatableTopics.put(topic, creatableTopic(topic)) - } - } - - (creatableTopics, uncreatableTopics) - } -} diff --git a/core/src/main/scala/kafka/server/BrokerServer.scala b/core/src/main/scala/kafka/server/BrokerServer.scala index dfb4427b8eda3..b47bd9dd8655b 100644 --- a/core/src/main/scala/kafka/server/BrokerServer.scala +++ b/core/src/main/scala/kafka/server/BrokerServer.scala @@ -54,7 +54,7 @@ import org.apache.kafka.server.share.persister.{DefaultStatePersister, NoOpState import org.apache.kafka.server.share.session.ShareSessionCache import org.apache.kafka.server.util.timer.{SystemTimer, SystemTimerReaper} import org.apache.kafka.server.util.{Deadline, FutureUtils, KafkaScheduler} -import org.apache.kafka.server.{AssignmentsManager, BrokerFeatures, ClientMetricsManager, DefaultApiVersionManager, DelayedActionQueue, DelegationTokenManager, ProcessRole} +import org.apache.kafka.server.{AssignmentsManager, AutoTopicCreationManager, BrokerFeatures, ClientMetricsManager, DefaultApiVersionManager, DefaultAutoTopicCreationManager, DelayedActionQueue, DelegationTokenManager, ProcessRole} import org.apache.kafka.server.transaction.AddPartitionsToTxnManager import org.apache.kafka.storage.internals.log.LogDirFailureChannel import org.apache.kafka.storage.log.metrics.BrokerTopicStats @@ -387,7 +387,7 @@ class BrokerServer( autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, clientToControllerChannelManager, groupCoordinator, - transactionCoordinator, shareCoordinator) + () => transactionCoordinator.transactionTopicConfigs, shareCoordinator) dynamicConfigHandlers = Map[ConfigType, ConfigHandler]( ConfigType.TOPIC -> new TopicConfigHandler(replicaManager, config, quotaManagers), diff --git a/core/src/main/scala/kafka/server/ForwardingManager.scala b/core/src/main/scala/kafka/server/ForwardingManager.scala index 7737d2d2171f2..ce93a10c0e105 100644 --- a/core/src/main/scala/kafka/server/ForwardingManager.scala +++ b/core/src/main/scala/kafka/server/ForwardingManager.scala @@ -24,13 +24,13 @@ import org.apache.kafka.clients.{ClientResponse, NodeApiVersions} import org.apache.kafka.common.errors.TimeoutException import org.apache.kafka.common.metrics.Metrics import org.apache.kafka.common.protocol.Errors -import org.apache.kafka.common.requests.{AbstractRequest, AbstractResponse, EnvelopeRequest, EnvelopeResponse, RequestContext, RequestHeader} +import org.apache.kafka.common.requests.{AbstractRequest, AbstractResponse, EnvelopeResponse, RequestContext, RequestHeader} +import org.apache.kafka.server.ForwardingManagerUtils import org.apache.kafka.server.common.{ControllerRequestCompletionHandler, NodeToControllerChannelManager} import org.apache.kafka.server.metrics.ForwardingManagerMetrics import java.util.Optional import java.util.concurrent.TimeUnit -import scala.jdk.OptionConverters.RichOptional trait ForwardingManager { def close(): Unit @@ -90,29 +90,6 @@ trait ForwardingManager { def controllerApiVersions: Optional[NodeApiVersions] } -object ForwardingManager { - def apply( - channelManager: NodeToControllerChannelManager, - metrics: Metrics - ): ForwardingManager = { - new ForwardingManagerImpl(channelManager, metrics) - } - - private[server] def buildEnvelopeRequest(context: RequestContext, - forwardRequestBuffer: ByteBuffer): EnvelopeRequest.Builder = { - val principalSerde = context.principalSerde.toScala.getOrElse( - throw new IllegalArgumentException(s"Cannot deserialize principal from request context $context " + - "since there is no serde defined") - ) - val serializedPrincipal = principalSerde.serialize(context.principal) - new EnvelopeRequest.Builder( - forwardRequestBuffer, - serializedPrincipal, - context.clientAddress.getAddress - ) - } -} - class ForwardingManagerImpl( channelManager: NodeToControllerChannelManager, metrics: Metrics @@ -128,7 +105,7 @@ class ForwardingManagerImpl( requestToString: () => String, responseCallback: Option[AbstractResponse] => Unit ): Unit = { - val envelopeRequest = ForwardingManager.buildEnvelopeRequest(requestContext, requestBufferCopy) + val envelopeRequest = ForwardingManagerUtils.buildEnvelopeRequest(requestContext, requestBufferCopy) val requestCreationTimeMs = TimeUnit.NANOSECONDS.toMillis(requestCreationNs) class ForwardingResponseHandler extends ControllerRequestCompletionHandler { diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index 2862d09036213..ff2a1c39733d0 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -60,7 +60,7 @@ import org.apache.kafka.common.{Node, TopicIdPartition, TopicPartition, Uuid} import org.apache.kafka.coordinator.group.{Group, GroupConfig, GroupConfigManager, GroupCoordinator} import org.apache.kafka.coordinator.share.ShareCoordinator import org.apache.kafka.metadata.{ConfigRepository, MetadataCache} -import org.apache.kafka.server.{ApiVersionManager, ClientMetricsManager, DelegationTokenManager, ProcessRole} +import org.apache.kafka.server.{ApiVersionManager, AutoTopicCreationManager, ClientMetricsManager, DelegationTokenManager, ProcessRole} import org.apache.kafka.server.authorizer._ import org.apache.kafka.server.common.{GroupVersion, RequestLocal, ShareVersion, StreamsVersion, TransactionVersion} import org.apache.kafka.server.config.DelegationTokenManagerConfigs @@ -906,23 +906,26 @@ class KafkaApis(val requestChannel: RequestChannel, request: RequestChannel.Request, fetchAllTopics: Boolean, allowAutoTopicCreation: Boolean, - topics: Set[String], + topics: util.Set[String], listenerName: ListenerName, errorUnavailableEndpoints: Boolean, errorUnavailableListeners: Boolean ): Seq[MetadataResponseTopic] = { - val topicResponses = metadataCache.getTopicMetadata(topics.asJava, listenerName, + val topicResponses = metadataCache.getTopicMetadata(topics, listenerName, errorUnavailableEndpoints, errorUnavailableListeners) if (topics.isEmpty || topicResponses.size == topics.size || fetchAllTopics) { topicResponses.asScala } else { - val nonExistingTopics = topics.diff(topicResponses.asScala.map(_.name).toSet) + val existingTopics = topicResponses.stream().map(topic => topic.name).collect(Collectors.toSet()) + val nonExistingTopics = topics.stream(). + filter(topic => !existingTopics.contains(topic)). + collect(Collectors.toSet()) + val nonExistingTopicResponses = if (allowAutoTopicCreation) { - val controllerMutationQuota = quotas.controllerMutation.newPermissiveQuotaFor(request.session, request.header.clientId()) - autoTopicCreationManager.createTopics(nonExistingTopics, controllerMutationQuota, Some(request.context)) + autoTopicCreationManager.createTopics(nonExistingTopics, Optional.of(request.context)) } else { - nonExistingTopics.map { topic => + nonExistingTopics.stream().map(topic => { val error = try { Topic.validate(topic) Errors.UNKNOWN_TOPIC_OR_PARTITION @@ -938,10 +941,12 @@ class KafkaApis(val requestChannel: RequestChannel, Topic.isInternal(topic), util.Collections.emptyList() ) - } + }).toList } - topicResponses.asScala ++ nonExistingTopicResponses + val responses = new util.ArrayList[MetadataResponseTopic](topicResponses) + responses.addAll(nonExistingTopicResponses) + responses.asScala.toSeq } } @@ -1022,7 +1027,7 @@ class KafkaApis(val requestChannel: RequestChannel, val errorUnavailableListeners = requestVersion >= 6 val allowAutoCreation = config.autoCreateTopicsEnable && metadataRequest.allowAutoTopicCreation && !metadataRequest.isAllTopics - val topicMetadata = getTopicMetadata(request, metadataRequest.isAllTopics, allowAutoCreation, authorizedTopics, + val topicMetadata = getTopicMetadata(request, metadataRequest.isAllTopics, allowAutoCreation, authorizedTopics.asJava, request.context.listenerName, errorUnavailableEndpoints, errorUnavailableListeners) var clusterAuthorizedOperations = Int.MinValue // Default value in the schema @@ -1341,11 +1346,11 @@ class KafkaApis(val requestChannel: RequestChannel, (shareCoordinator.partitionFor(SharePartitionKey.getInstance(key)), SHARE_GROUP_STATE_TOPIC_NAME) } - val topicMetadata = metadataCache.getTopicMetadata(Set(internalTopicName).asJava, request.context.listenerName, false, false).asScala + val internalTopics = util.Set.of(internalTopicName) + val topicMetadata = metadataCache.getTopicMetadata(internalTopics, request.context.listenerName, false, false).asScala if (topicMetadata.headOption.isEmpty) { - val controllerMutationQuota = quotas.controllerMutation.newPermissiveQuotaFor(request.session, request.header.clientId) - autoTopicCreationManager.createTopics(Seq(internalTopicName).toSet, controllerMutationQuota, None) + autoTopicCreationManager.createTopics(internalTopics, Optional.empty) (Errors.COORDINATOR_NOT_AVAILABLE, Node.noNode) } else { if (topicMetadata.head.errorCode != Errors.NONE.code) { @@ -2862,12 +2867,12 @@ class KafkaApis(val requestChannel: RequestChannel, requestHelper.sendMaybeThrottle(request, streamsGroupHeartbeatRequest.getErrorResponse(exception)) } else { val responseData = response.data() - val topicsToCreate = response.creatableTopics().asScala - if (topicsToCreate.nonEmpty) { + val topicsToCreate = response.creatableTopics() + if (!topicsToCreate.isEmpty) { val createTopicUnauthorized = if(!authHelper.authorize(request.context, CREATE, CLUSTER, CLUSTER_NAME, logIfDenied = false)) - authHelper.partitionSeqByAuthorized(request.context, CREATE, TOPIC, topicsToCreate.keys.toSeq)(identity[String])._2 + authHelper.partitionSeqByAuthorized(request.context, CREATE, TOPIC, topicsToCreate.keySet.asScala.toSeq)(identity[String])._2 else Set.empty if (createTopicUnauthorized.nonEmpty) { diff --git a/core/src/main/scala/kafka/server/KafkaConfig.scala b/core/src/main/scala/kafka/server/KafkaConfig.scala index 2bdadb02fb89d..ae84053a3e153 100755 --- a/core/src/main/scala/kafka/server/KafkaConfig.scala +++ b/core/src/main/scala/kafka/server/KafkaConfig.scala @@ -311,7 +311,6 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _]) /** ********* Log Configuration ***********/ val autoCreateTopicsEnable = getBoolean(ServerLogConfigs.AUTO_CREATE_TOPICS_ENABLE_CONFIG) - val numPartitions = getInt(ServerLogConfigs.NUM_PARTITIONS_CONFIG) def logSegmentBytes = getInt(ServerLogConfigs.LOG_SEGMENT_BYTES_CONFIG) def logFlushIntervalMessages = getLong(ServerLogConfigs.LOG_FLUSH_INTERVAL_MESSAGES_CONFIG) def logCleanerThreads = getInt(CleanerConfig.LOG_CLEANER_THREADS_PROP) @@ -347,7 +346,6 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _]) /** ********* Replication configuration ***********/ val controllerSocketTimeoutMs: Int = getInt(ReplicationConfigs.CONTROLLER_SOCKET_TIMEOUT_MS_CONFIG) - val defaultReplicationFactor: Int = getInt(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG) val replicaLagTimeMaxMs = getLong(ReplicationConfigs.REPLICA_LAG_TIME_MAX_MS_CONFIG) val replicaSocketTimeoutMs = getInt(ReplicationConfigs.REPLICA_SOCKET_TIMEOUT_MS_CONFIG) val replicaSocketReceiveBufferBytes = getInt(ReplicationConfigs.REPLICA_SOCKET_RECEIVE_BUFFER_BYTES_CONFIG) diff --git a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala b/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala deleted file mode 100644 index a2cfb3b54b501..0000000000000 --- a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala +++ /dev/null @@ -1,396 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.net.InetAddress -import java.nio.ByteBuffer -import java.util -import java.util.concurrent.atomic.AtomicBoolean -import java.util.{Collections, Optional, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.TestUtils -import org.apache.kafka.clients.{ClientResponse, NodeApiVersions, RequestCompletionHandler} -import org.apache.kafka.common.Node -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.{ApiVersionsResponseData, CreateTopicsRequestData} -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.network.{ClientInformation, ListenerName} -import org.apache.kafka.common.protocol.{ApiKeys, ByteBufferAccessor, Errors} -import org.apache.kafka.common.requests._ -import org.apache.kafka.common.security.auth.{KafkaPrincipal, KafkaPrincipalSerde, SecurityProtocol} -import org.apache.kafka.common.utils.{SecurityUtils, Utils} -import org.apache.kafka.coordinator.group.{GroupCoordinator, GroupCoordinatorConfig} -import org.apache.kafka.coordinator.share.{ShareCoordinator, ShareCoordinatorConfig} -import org.apache.kafka.metadata.MetadataCache -import org.apache.kafka.server.config.ServerConfigs -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.common.{ControllerRequestCompletionHandler, NodeToControllerChannelManager} -import org.apache.kafka.server.quota.ControllerMutationQuota -import org.junit.jupiter.api.Assertions.{assertEquals, assertThrows, assertTrue} -import org.junit.jupiter.api.{BeforeEach, Test} -import org.mockito.ArgumentMatchers.any -import org.mockito.Mockito.never -import org.mockito.{ArgumentCaptor, ArgumentMatchers, Mockito} - -import scala.collection.{Map, Seq} - -class AutoTopicCreationManagerTest { - - private val requestTimeout = 100 - private var config: KafkaConfig = _ - private val metadataCache = Mockito.mock(classOf[MetadataCache]) - private val brokerToController = Mockito.mock(classOf[NodeToControllerChannelManager]) - private val groupCoordinator = Mockito.mock(classOf[GroupCoordinator]) - private val transactionCoordinator = Mockito.mock(classOf[TransactionCoordinator]) - private val shareCoordinator = Mockito.mock(classOf[ShareCoordinator]) - private var autoTopicCreationManager: AutoTopicCreationManager = _ - - private val internalTopicPartitions = 2 - private val internalTopicReplicationFactor: Short = 2 - - @BeforeEach - def setup(): Unit = { - val props = TestUtils.createBrokerConfig(1) - props.setProperty(ServerConfigs.REQUEST_TIMEOUT_MS_CONFIG, requestTimeout.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicPartitions.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicPartitions.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_REPLICATION_FACTOR_CONFIG , internalTopicPartitions.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_NUM_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - - config = KafkaConfig.fromProps(props) - val aliveBrokers = util.List.of(new Node(0, "host0", 0), new Node(1, "host1", 1)) - - Mockito.when(metadataCache.getAliveBrokerNodes(any(classOf[ListenerName]))).thenReturn(aliveBrokers) - } - - @Test - def testCreateOffsetTopic(): Unit = { - Mockito.when(groupCoordinator.groupMetadataTopicConfigs).thenReturn(new Properties) - testCreateTopic(GROUP_METADATA_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateTxnTopic(): Unit = { - Mockito.when(transactionCoordinator.transactionTopicConfigs).thenReturn(new Properties) - testCreateTopic(TRANSACTION_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateShareStateTopic(): Unit = { - Mockito.when(shareCoordinator.shareGroupStateTopicConfigs()).thenReturn(new Properties) - testCreateTopic(SHARE_GROUP_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateNonInternalTopic(): Unit = { - testCreateTopic("topic", isInternal = false) - } - - private def testCreateTopic(topicName: String, - isInternal: Boolean, - numPartitions: Int = 1, - replicationFactor: Short = 1): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - val topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection - topicsCollection.add(getNewTopic(topicName, numPartitions, replicationFactor)) - val requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - - // Calling twice with the same topic will only trigger one forwarding. - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - - Mockito.verify(brokerToController).sendRequest( - ArgumentMatchers.eq(requestBody), - any(classOf[ControllerRequestCompletionHandler])) - } - - @Test - def testTopicCreationWithMetadataContextPassPrincipal(): Unit = { - val topicName = "topic" - - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val serializeIsCalled = new AtomicBoolean(false) - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - assertEquals(principal, userPrincipal) - serializeIsCalled.set(true) - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - - val requestContext = initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - - assertTrue(serializeIsCalled.get()) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - assertEquals(userPrincipal, SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal))) - } - - @Test - def testTopicCreationWithMetadataContextWhenPrincipalSerdeNotDefined(): Unit = { - val topicName = "topic" - - val requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.empty()) - - // Throw upon undefined principal serde when building the forward request - assertThrows(classOf[IllegalArgumentException], () => autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext))) - } - - @Test - def testTopicCreationWithMetadataContextNoRetryUponUnsupportedVersion(): Unit = { - val topicName = "topic" - - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - - val requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.of(principalSerde)) - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - - // Should only trigger once - val argumentCaptor = ArgumentCaptor.forClass(classOf[ControllerRequestCompletionHandler]) - Mockito.verify(brokerToController).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Complete with unsupported version will not trigger a retry, but cleanup the inflight topics instead - val header = new RequestHeader(ApiKeys.ENVELOPE, 0, "client", 1) - val response = new EnvelopeResponse(ByteBuffer.allocate(0), Errors.UNSUPPORTED_VERSION) - val clientResponse = new ClientResponse(header, null, null, - 0, 0, false, null, null, response) - argumentCaptor.getValue.asInstanceOf[RequestCompletionHandler].onComplete(clientResponse) - Mockito.verify(brokerToController, Mockito.times(1)).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Could do the send again as inflight topics are cleared. - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - Mockito.verify(brokerToController, Mockito.times(2)).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - } - - @Test - def testCreateStreamsInternalTopics(): Unit = { - val topicConfig = new CreatableTopicConfigCollection() - topicConfig.add(new CreatableTopicConfig().setName("cleanup.policy").setValue("compact")) - - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(3).setReplicationFactor(2).setConfigs(topicConfig), - "stream-topic-2" -> new CreatableTopic().setName("stream-topic-2").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, ApiKeys.CREATE_TOPICS.latestVersion(), "clientId", 0) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - val topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection - topicsCollection.add(getNewTopic("stream-topic-1", 3, 2.toShort).setConfigs(topicConfig)) - topicsCollection.add(getNewTopic("stream-topic-2", 1, 1.toShort)) - val requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - .build(ApiKeys.CREATE_TOPICS.latestVersion()) - - val forwardedRequestBuffer = capturedRequest.requestData().duplicate() - assertEquals(requestHeader, RequestHeader.parse(forwardedRequestBuffer)) - assertEquals(requestBody.data(), CreateTopicsRequest.parse(new ByteBufferAccessor(forwardedRequestBuffer), - ApiKeys.CREATE_TOPICS.latestVersion()).data()) - } - - @Test - def testCreateStreamsInternalTopicsWithEmptyTopics(): Unit = { - val topics = Map.empty[String, CreatableTopic] - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext) - - Mockito.verify(brokerToController, never()).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - any(classOf[ControllerRequestCompletionHandler])) - } - - @Test - def testCreateStreamsInternalTopicsWithDefaultConfig(): Unit = { - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(-1).setReplicationFactor(-1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, ApiKeys.CREATE_TOPICS.latestVersion(), "clientId", 0) - val topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection - topicsCollection.add(getNewTopic("stream-topic-1", config.numPartitions, config.defaultReplicationFactor.toShort)) - val requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - .build(ApiKeys.CREATE_TOPICS.latestVersion()) - val forwardedRequestBuffer = capturedRequest.requestData().duplicate() - assertEquals(requestHeader, RequestHeader.parse(forwardedRequestBuffer)) - assertEquals(requestBody.data(), CreateTopicsRequest.parse(new ByteBufferAccessor(forwardedRequestBuffer), - ApiKeys.CREATE_TOPICS.latestVersion()).data()) - } - - @Test - def testCreateStreamsInternalTopicsPassesPrincipal(): Unit = { - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(-1).setReplicationFactor(-1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - assertEquals(new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user"), SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal))) - } - - private def initializeRequestContextWithUserPrincipal(): RequestContext = { - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - } - - private def initializeRequestContext(kafkaPrincipal: KafkaPrincipal, - principalSerde: Optional[KafkaPrincipalSerde]): RequestContext = { - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - val createTopicApiVersion = new ApiVersionsResponseData.ApiVersion() - .setApiKey(ApiKeys.CREATE_TOPICS.id) - .setMinVersion(ApiKeys.CREATE_TOPICS.oldestVersion()) - .setMaxVersion(ApiKeys.CREATE_TOPICS.latestVersion()) - Mockito.when(brokerToController.controllerApiVersions()) - .thenReturn(Optional.of(NodeApiVersions.create(Collections.singleton(createTopicApiVersion)))) - - val requestHeader = new RequestHeader(ApiKeys.METADATA, ApiKeys.METADATA.latestVersion, - "clientId", 0) - new RequestContext(requestHeader, "1", InetAddress.getLocalHost, Optional.empty(), - kafkaPrincipal, ListenerName.forSecurityProtocol(SecurityProtocol.PLAINTEXT), - SecurityProtocol.PLAINTEXT, ClientInformation.EMPTY, false, principalSerde) - } - - private def createTopicAndVerifyResult(error: Errors, - topicName: String, - isInternal: Boolean, - metadataContext: Option[RequestContext] = None): Unit = { - val topicResponses = autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, metadataContext) - - val expectedResponses = Seq(new MetadataResponseTopic() - .setErrorCode(error.code()) - .setIsInternal(isInternal) - .setName(topicName)) - - assertEquals(expectedResponses, topicResponses) - } - - private def getNewTopic(topicName: String, numPartitions: Int, replicationFactor: Short): CreatableTopic = { - new CreatableTopic() - .setName(topicName) - .setNumPartitions(numPartitions) - .setReplicationFactor(replicationFactor) - } -} diff --git a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala index a7a949df55d52..a1fa5cd73a324 100644 --- a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala +++ b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala @@ -87,7 +87,7 @@ import org.apache.kafka.network.Session import org.apache.kafka.network.metrics.{RequestChannelMetrics, RequestMetrics} import org.apache.kafka.raft.QuorumConfig import org.apache.kafka.security.authorizer.AclEntry -import org.apache.kafka.server.{ClientMetricsManager, SimpleApiVersionManager} +import org.apache.kafka.server.{AutoTopicCreationManager, ClientMetricsManager, SimpleApiVersionManager} import org.apache.kafka.server.authorizer.{Action, AuthorizationResult, Authorizer} import org.apache.kafka.server.common.{FeatureVersion, FinalizedFeatures, GroupVersion, KRaftVersion, MetadataVersion, RequestLocal, ShareVersion, StreamsVersion, TransactionVersion} import org.apache.kafka.server.config.{KRaftConfigs, ReplicationConfigs, ServerConfigs, ServerLogConfigs} @@ -948,7 +948,7 @@ class KafkaApisTest extends Logging { assertEquals(expectedMetadataResponse, response.topicMetadata()) if (enableAutoTopicCreation) { - assertTrue(capturedRequest.getValue.isDefined) + assertTrue(capturedRequest.getValue.isPresent) assertEquals(request.context, capturedRequest.getValue.get) } } @@ -956,8 +956,8 @@ class KafkaApisTest extends Logging { private def verifyTopicCreation(topicName: String, enableAutoTopicCreation: Boolean, isInternal: Boolean, - request: RequestChannel.Request): ArgumentCaptor[Option[RequestContext]] = { - val capturedRequest: ArgumentCaptor[Option[RequestContext]] = ArgumentCaptor.forClass(classOf[Option[RequestContext]]) + request: RequestChannel.Request): ArgumentCaptor[Optional[RequestContext]] = { + val capturedRequest: ArgumentCaptor[Optional[RequestContext]] = ArgumentCaptor.forClass(classOf[Optional[RequestContext]]) if (enableAutoTopicCreation) { when(clientControllerQuotaManager.newPermissiveQuotaFor( @@ -966,13 +966,13 @@ class KafkaApisTest extends Logging { )).thenReturn(ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA) when(autoTopicCreationManager.createTopics( - ArgumentMatchers.eq(Set(topicName)), - ArgumentMatchers.eq(ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA), - capturedRequest.capture())).thenReturn( - Seq(new MetadataResponseTopic() - .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) - .setIsInternal(isInternal) - .setName(topicName)) + ArgumentMatchers.eq(util.Set.of(topicName)), + capturedRequest.capture() + )).thenReturn( + util.List.of(new MetadataResponseTopic() + .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) + .setIsInternal(isInternal) + .setName(topicName)) ) } capturedRequest @@ -4196,7 +4196,7 @@ class KafkaApisTest extends Logging { val responseTopics = response.topicMetadata().asScala.map { metadata => metadata.topic() } // verify we don't create topic when getAllTopicMetadata - verify(autoTopicCreationManager, never).createTopics(any(), any(), any()) + verify(autoTopicCreationManager, never).createTopics(any(), any()) assertEquals(List("remaining-topic"), responseTopics) assertTrue(response.topicsByError(Errors.UNKNOWN_TOPIC_OR_PARTITION).isEmpty) } @@ -10882,11 +10882,11 @@ class KafkaApisTest extends Logging { kafkaApis = createKafkaApis() kafkaApis.handle(requestChannelRequest, RequestLocal.noCaching) - val missingTopics = Map("test" -> new CreatableTopic()) + val missingTopics = util.Map.of("test", new CreatableTopic()) val streamsGroupHeartbeatResponse = new StreamsGroupHeartbeatResponseData() .setMemberId("member") - future.complete(new StreamsGroupHeartbeatResult(streamsGroupHeartbeatResponse, missingTopics.asJava)) + future.complete(new StreamsGroupHeartbeatResult(streamsGroupHeartbeatResponse, missingTopics)) val response = verifyNoThrottling[StreamsGroupHeartbeatResponse](requestChannelRequest) assertEquals(streamsGroupHeartbeatResponse, response.data) verify(autoTopicCreationManager).createStreamsInternalTopics(missingTopics, requestChannelRequest.context) diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/metadata/KRaftMetadataRequestBenchmark.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/metadata/KRaftMetadataRequestBenchmark.java index 788915f7c2462..b7847ab07cf0a 100644 --- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/metadata/KRaftMetadataRequestBenchmark.java +++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/metadata/KRaftMetadataRequestBenchmark.java @@ -19,7 +19,6 @@ import kafka.coordinator.transaction.TransactionCoordinator; import kafka.network.RequestChannel; -import kafka.server.AutoTopicCreationManager; import kafka.server.ClientRequestQuotaManager; import kafka.server.FetchManager; import kafka.server.ForwardingManager; @@ -56,6 +55,7 @@ import org.apache.kafka.network.RequestConvertToJson; import org.apache.kafka.network.metrics.RequestChannelMetrics; import org.apache.kafka.raft.QuorumConfig; +import org.apache.kafka.server.AutoTopicCreationManager; import org.apache.kafka.server.ClientMetricsManager; import org.apache.kafka.server.SimpleApiVersionManager; import org.apache.kafka.server.common.FinalizedFeatures; diff --git a/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java new file mode 100644 index 0000000000000..a61bff08af59a --- /dev/null +++ b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.server; + +import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopic; +import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic; +import org.apache.kafka.common.requests.RequestContext; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +public interface AutoTopicCreationManager { + + /** + * Initiate auto topic creation for the given topics. + * + * @param topics the topics to create + * @param metadataRequestContext defined when creating topics on behalf of the client. The goal here is to preserve + * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest + * inside Envelope to send to the controller when forwarding is enabled. + * @return auto created topic metadata responses + */ + List createTopics( + Set topics, + Optional metadataRequestContext + ); + + /** + * Initiate auto topic creation for the given topics. + * This method is used for creating internal topics for streams. + * + * @param topics the topics to create + * @param metadataRequestContext defined when creating topics on behalf of the client. The goal here is to preserve + * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest + * inside Envelope to send to the controller when forwarding is enabled. + */ + void createStreamsInternalTopics( + Map topics, + RequestContext metadataRequestContext + ); +} diff --git a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java new file mode 100644 index 0000000000000..298ed79640032 --- /dev/null +++ b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java @@ -0,0 +1,268 @@ +/* + * 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.server; + +import org.apache.kafka.clients.ClientResponse; +import org.apache.kafka.common.errors.InvalidTopicException; +import org.apache.kafka.common.internals.Topic; +import org.apache.kafka.common.message.CreateTopicsRequestData; +import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopic; +import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopicConfig; +import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopicConfigCollection; +import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic; +import org.apache.kafka.common.protocol.ApiKeys; +import org.apache.kafka.common.protocol.Errors; +import org.apache.kafka.common.requests.AbstractRequest; +import org.apache.kafka.common.requests.CreateTopicsRequest; +import org.apache.kafka.common.requests.CreateTopicsResponse; +import org.apache.kafka.common.requests.RequestContext; +import org.apache.kafka.common.requests.RequestHeader; +import org.apache.kafka.coordinator.group.GroupCoordinator; +import org.apache.kafka.coordinator.group.GroupCoordinatorConfig; +import org.apache.kafka.coordinator.share.ShareCoordinator; +import org.apache.kafka.coordinator.share.ShareCoordinatorConfig; +import org.apache.kafka.coordinator.transaction.TransactionLogConfig; +import org.apache.kafka.server.common.ControllerRequestCompletionHandler; +import org.apache.kafka.server.common.NodeToControllerChannelManager; +import org.apache.kafka.server.config.AbstractKafkaConfig; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; +import java.util.stream.Stream; + +public class DefaultAutoTopicCreationManager implements AutoTopicCreationManager { + + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultAutoTopicCreationManager.class); + + private final AbstractKafkaConfig config; + private final NodeToControllerChannelManager channelManager; + private final GroupCoordinator groupCoordinator; + private final ShareCoordinator shareCoordinator; + private final Supplier transactionTopicConfigsSupplier; + private final Set inflightTopics = ConcurrentHashMap.newKeySet(); + + public DefaultAutoTopicCreationManager( + AbstractKafkaConfig config, + NodeToControllerChannelManager channelManager, + GroupCoordinator groupCoordinator, + Supplier transactionTopicConfigsSupplier, + ShareCoordinator shareCoordinator + ) { + this.config = config; + this.channelManager = channelManager; + this.groupCoordinator = groupCoordinator; + this.shareCoordinator = shareCoordinator; + this.transactionTopicConfigsSupplier = transactionTopicConfigsSupplier; + } + + @Override + public List createTopics( + Set topics, + Optional metadataRequestContext + ) { + var creatableTopics = new HashMap(); + var uncreatableTopicResponses = new ArrayList(); + topics.forEach(topic -> { + // Attempt basic topic validation before sending any requests to the controller. + Optional validationError; + if (!isValidTopicName(topic)) { + validationError = Optional.of(Errors.INVALID_TOPIC_EXCEPTION); + } else if (!inflightTopics.add(topic)) { + validationError = Optional.of(Errors.UNKNOWN_TOPIC_OR_PARTITION); + } else { + validationError = Optional.empty(); + } + + if (validationError.isPresent()) { + uncreatableTopicResponses.add(new MetadataResponseTopic() + .setErrorCode(validationError.get().code()) + .setName(topic) + .setIsInternal(Topic.isInternal(topic))); + } else { + creatableTopics.put(topic, creatableTopic(topic)); + } + }); + var creatableTopicResponses = creatableTopics.isEmpty() ? + List.of() : sendCreateTopicRequest(creatableTopics, metadataRequestContext); + return Stream.concat(uncreatableTopicResponses.stream(), creatableTopicResponses.stream()) + .toList(); + } + + @Override + public void createStreamsInternalTopics( + Map topics, + RequestContext requestContext + ) { + if (topics.isEmpty()) { + return; + } + topics.values().forEach(creatableTopic -> { + if (creatableTopic.numPartitions() == -1) { + creatableTopic.setNumPartitions(config.numPartitions()); + } + if (creatableTopic.replicationFactor() == -1) { + creatableTopic.setReplicationFactor((short) config.defaultReplicationFactor()); + } + }); + sendCreateTopicRequest(topics, Optional.of(requestContext)); + } + + private List sendCreateTopicRequest( + Map creatableTopics, + Optional requestContext + ) { + var topicsToCreate = new CreateTopicsRequestData.CreatableTopicCollection(creatableTopics.size()); + topicsToCreate.addAll(creatableTopics.values()); + var createTopicsRequest = new CreateTopicsRequest.Builder( + new CreateTopicsRequestData() + .setTimeoutMs(config.requestTimeoutMs()) + .setTopics(topicsToCreate) + ); + + var requestCompletionHandler = new ControllerRequestCompletionHandler() { + @Override + public void onTimeout() { + clearInflightRequests(creatableTopics); + LOGGER.debug("Auto topic creation timed out for {}.", creatableTopics.keySet()); + } + + @Override + public void onComplete(ClientResponse response) { + clearInflightRequests(creatableTopics); + if (response.authenticationException() != null) { + LOGGER.warn("Auto topic creation failed for {} with authentication exception.", creatableTopics.keySet()); + } else if (response.versionMismatch() != null) { + LOGGER.warn("Auto topic creation failed for {} with invalid version exception.", creatableTopics.keySet()); + } else { + if (response.hasResponse()) { + if (response.responseBody() instanceof CreateTopicsResponse createTopicsResponse) { + createTopicsResponse.data().topics().forEach(topicResult -> { + var error = Errors.forCode(topicResult.errorCode()); + if (error != Errors.NONE) { + LOGGER.warn("Auto topic creation failed for {} with error '{}': {}.", topicResult.name(), error.name(), topicResult.errorMessage()); + } + }); + } else { + LOGGER.warn("Auto topic creation request received unexpected response type: {}.", response.responseBody().getClass().getSimpleName()); + } + } + LOGGER.debug("Auto topic creation completed for {} with response {}.", creatableTopics.keySet(), response.responseBody()); + } + } + }; + + var request = requestContext.>map(context -> { + short requestVersion = channelManager.controllerApiVersions() + .map(nodeApiVersions -> nodeApiVersions.latestUsableVersion(ApiKeys.CREATE_TOPICS)) + // We will rely on the Metadata request to be retried in the case + // that the latest version is not usable by the controller. + .orElseGet(ApiKeys.CREATE_TOPICS::latestVersion); + + // Borrow client information such as client id and correlation id from the original request, + // in order to correlate the create request with the original metadata request. + var requestHeader = new RequestHeader( + ApiKeys.CREATE_TOPICS, + requestVersion, + context.clientId(), + context.correlationId() + ); + + return ForwardingManagerUtils.buildEnvelopeRequest(context, createTopicsRequest.build(requestVersion).serializeWithHeader(requestHeader)); + }).orElse(createTopicsRequest); + + channelManager.sendRequest(request, requestCompletionHandler); + + var creatableTopicResponses = creatableTopics.keySet().stream() + .map(topic -> new MetadataResponseTopic() + .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) + .setName(topic) + .setIsInternal(Topic.isInternal(topic))) + .toList(); + + LOGGER.info("Sent auto-creation request for {} to the active controller.", creatableTopics.keySet()); + return creatableTopicResponses; + } + + private void clearInflightRequests(Map creatableTopics) { + creatableTopics.keySet().forEach(inflightTopics::remove); + LOGGER.debug("Cleared inflight topic creation state for {}.", creatableTopics); + } + + private CreatableTopic creatableTopic(String topic) { + return switch (topic) { + case Topic.GROUP_METADATA_TOPIC_NAME -> { + var groupCoordinatorConfig = new GroupCoordinatorConfig(config); + yield new CreatableTopic() + .setName(topic) + .setNumPartitions(groupCoordinatorConfig.offsetsTopicPartitions()) + .setReplicationFactor(groupCoordinatorConfig.offsetsTopicReplicationFactor()) + .setConfigs(convertToTopicConfigCollections(groupCoordinator.groupMetadataTopicConfigs())); + } + case Topic.TRANSACTION_STATE_TOPIC_NAME -> { + var transactionLogConfig = new TransactionLogConfig(config); + yield new CreatableTopic() + .setName(topic) + .setNumPartitions(transactionLogConfig.transactionTopicPartitions()) + .setReplicationFactor(transactionLogConfig.transactionTopicReplicationFactor()) + .setConfigs(convertToTopicConfigCollections(transactionTopicConfigsSupplier.get())); + } + case Topic.SHARE_GROUP_STATE_TOPIC_NAME -> { + var shareCoordinatorConfig = new ShareCoordinatorConfig(config); + yield new CreatableTopic() + .setName(topic) + .setNumPartitions(shareCoordinatorConfig.shareCoordinatorStateTopicNumPartitions()) + .setReplicationFactor(shareCoordinatorConfig.shareCoordinatorStateTopicReplicationFactor()) + .setConfigs(convertToTopicConfigCollections(shareCoordinator.shareGroupStateTopicConfigs())); + } + default -> new CreatableTopic() + .setName(topic) + .setNumPartitions(config.numPartitions()) + .setReplicationFactor((short) config.defaultReplicationFactor()); + }; + } + + private static CreatableTopicConfigCollection convertToTopicConfigCollections(Properties config) { + return new CreatableTopicConfigCollection( + config.entrySet().stream() + .map(entry -> new CreatableTopicConfig() + .setName(entry.getKey().toString()) + .setValue(entry.getValue().toString())) + .toList() + .iterator() + ); + } + + private static boolean isValidTopicName(String topic) { + try { + Topic.validate(topic); + return true; + } catch (InvalidTopicException e) { + return false; + } + } +} diff --git a/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java b/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java new file mode 100644 index 0000000000000..43a9dfc276c95 --- /dev/null +++ b/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java @@ -0,0 +1,42 @@ +/* + * 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.server; + +import org.apache.kafka.common.requests.EnvelopeRequest; +import org.apache.kafka.common.requests.RequestContext; + +import java.nio.ByteBuffer; + +public class ForwardingManagerUtils { + + public static EnvelopeRequest.Builder buildEnvelopeRequest( + RequestContext context, + ByteBuffer forwardRequestBuffer + ) { + var principalSerde = context.principalSerde.orElseThrow(() -> new IllegalArgumentException( + String.format("Cannot deserialize principal from request context %s since there is no serde defined", context)) + ); + + var serializedPrincipal = principalSerde.serialize(context.principal); + return new EnvelopeRequest.Builder( + forwardRequestBuffer, + serializedPrincipal, + context.clientAddress.getAddress() + ); + } +} diff --git a/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java b/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java index 16d617227273e..7ec7382166d07 100644 --- a/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java +++ b/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java @@ -152,6 +152,14 @@ public Map effectiveListenerSecurityProtocolMap( } } + public int numPartitions() { + return getInt(ServerLogConfigs.NUM_PARTITIONS_CONFIG); + } + + public int defaultReplicationFactor() { + return getInt(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG).shortValue(); + } + public static Map getMap(String propName, String propValue) { try { return Csv.parseCsvMap(propValue); diff --git a/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java new file mode 100644 index 0000000000000..2e91afeb6042e --- /dev/null +++ b/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java @@ -0,0 +1,458 @@ +/* + * 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.server; + +import org.apache.kafka.clients.ClientResponse; +import org.apache.kafka.clients.NodeApiVersions; +import org.apache.kafka.common.Node; +import org.apache.kafka.common.message.ApiVersionsResponseData; +import org.apache.kafka.common.message.CreateTopicsRequestData; +import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopic; +import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopicConfig; +import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopicConfigCollection; +import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic; +import org.apache.kafka.common.network.ClientInformation; +import org.apache.kafka.common.network.ListenerName; +import org.apache.kafka.common.protocol.ApiKeys; +import org.apache.kafka.common.protocol.ByteBufferAccessor; +import org.apache.kafka.common.protocol.Errors; +import org.apache.kafka.common.requests.AbstractRequest; +import org.apache.kafka.common.requests.CreateTopicsRequest; +import org.apache.kafka.common.requests.EnvelopeRequest; +import org.apache.kafka.common.requests.EnvelopeResponse; +import org.apache.kafka.common.requests.RequestContext; +import org.apache.kafka.common.requests.RequestHeader; +import org.apache.kafka.common.security.auth.KafkaPrincipal; +import org.apache.kafka.common.security.auth.KafkaPrincipalSerde; +import org.apache.kafka.common.security.auth.SecurityProtocol; +import org.apache.kafka.common.utils.SecurityUtils; +import org.apache.kafka.common.utils.Utils; +import org.apache.kafka.coordinator.group.GroupCoordinator; +import org.apache.kafka.coordinator.group.GroupCoordinatorConfig; +import org.apache.kafka.coordinator.share.ShareCoordinator; +import org.apache.kafka.coordinator.share.ShareCoordinatorConfig; +import org.apache.kafka.coordinator.transaction.TransactionLogConfig; +import org.apache.kafka.metadata.MetadataCache; +import org.apache.kafka.server.common.ControllerRequestCompletionHandler; +import org.apache.kafka.server.common.NodeToControllerChannelManager; +import org.apache.kafka.server.config.AbstractKafkaConfig; +import org.apache.kafka.server.config.KRaftConfigs; +import org.apache.kafka.server.config.ServerConfigs; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.ByteBuffer; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.apache.kafka.common.internals.Topic.GROUP_METADATA_TOPIC_NAME; +import static org.apache.kafka.common.internals.Topic.SHARE_GROUP_STATE_TOPIC_NAME; +import static org.apache.kafka.common.internals.Topic.TRANSACTION_STATE_TOPIC_NAME; +import static org.junit.jupiter.api.Assertions.assertEquals; +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.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +public class AutoTopicCreationManagerTest { + @SuppressWarnings("unchecked") + private static final Class> ABSTRACT_REQUEST_BUILDER_CLASS = + (Class>) (Class) AbstractRequest.Builder.class; + + private final int requestTimeout = 100; + private AbstractKafkaConfig config; + private final MetadataCache metadataCache = Mockito.mock(MetadataCache.class); + private final NodeToControllerChannelManager brokerToController = Mockito.mock(NodeToControllerChannelManager.class); + private final GroupCoordinator groupCoordinator = Mockito.mock(GroupCoordinator.class); + private final ShareCoordinator shareCoordinator = Mockito.mock(ShareCoordinator.class); + private AutoTopicCreationManager autoTopicCreationManager; + + private final int internalTopicPartitions = 2; + private final short internalTopicReplicationFactor = 2; + + @BeforeEach + public void setup() { + var props = new Properties(); + props.setProperty(KRaftConfigs.NODE_ID_CONFIG, "1"); + props.setProperty(KRaftConfigs.PROCESS_ROLES_CONFIG, "broker"); + props.setProperty(ServerConfigs.REQUEST_TIMEOUT_MS_CONFIG, String.valueOf(requestTimeout)); + + props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, String.valueOf(internalTopicPartitions)); + props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_REPLICATION_FACTOR_CONFIG, String.valueOf(internalTopicPartitions)); + props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_REPLICATION_FACTOR_CONFIG, String.valueOf(internalTopicPartitions)); + + props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, String.valueOf(internalTopicReplicationFactor)); + props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_PARTITIONS_CONFIG, String.valueOf(internalTopicReplicationFactor)); + props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_NUM_PARTITIONS_CONFIG, String.valueOf(internalTopicReplicationFactor)); + + config = new AbstractKafkaConfig(AbstractKafkaConfig.CONFIG_DEF, props, Map.of(), false) { }; + var aliveBrokers = List.of(new Node(0, "host0", 0), new Node(1, "host1", 1)); + Mockito.when(metadataCache.getAliveBrokerNodes(any(ListenerName.class))).thenReturn(aliveBrokers); + } + + @Test + public void testCreateOffsetTopic() { + Mockito.when(groupCoordinator.groupMetadataTopicConfigs()).thenReturn(new Properties()); + testCreateTopic(GROUP_METADATA_TOPIC_NAME, true, internalTopicPartitions, internalTopicReplicationFactor); + } + + @Test + public void testCreateTxnTopic() { + testCreateTopic(TRANSACTION_STATE_TOPIC_NAME, true, internalTopicPartitions, internalTopicReplicationFactor); + } + + @Test + public void testCreateShareStateTopic() { + Mockito.when(shareCoordinator.shareGroupStateTopicConfigs()).thenReturn(new Properties()); + testCreateTopic(SHARE_GROUP_STATE_TOPIC_NAME, true, internalTopicPartitions, internalTopicReplicationFactor); + } + + @Test + public void testCreateNonInternalTopic() { + testCreateTopic("topic", false, 1, (short) 1); + } + + private void testCreateTopic( + String topicName, + Boolean isInternal, + int numPartitions, + short replicationFactor + ) { + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + groupCoordinator, + Properties::new, + shareCoordinator); + + var topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection(); + topicsCollection.add(getNewTopic(topicName, numPartitions, replicationFactor)); + var requestBody = new CreateTopicsRequest.Builder( + new CreateTopicsRequestData() + .setTopics(topicsCollection) + .setTimeoutMs(requestTimeout)); + + // Calling twice with the same topic will only trigger one forwarding. + createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal); + createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal); + + verify(brokerToController).sendRequest( + ArgumentMatchers.eq(requestBody), + any(ControllerRequestCompletionHandler.class)); + } + + @Test + public void testTopicCreationWithMetadataContextPassPrincipal() throws UnknownHostException { + var topicName = "topic"; + var userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user"); + var serializeIsCalled = new AtomicBoolean(false); + var principalSerde = new KafkaPrincipalSerde() { + @Override + public byte[] serialize(KafkaPrincipal principal) { + assertEquals(principal, userPrincipal); + serializeIsCalled.set(true); + return Utils.utf8(principal.toString()); + } + + @Override + public KafkaPrincipal deserialize(byte[] bytes) { + return SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)); + } + }; + + var requestContext = initializeRequestContext(userPrincipal, Optional.of(principalSerde)); + + autoTopicCreationManager.createTopics(Set.of(topicName), Optional.of(requestContext)); + + assertTrue(serializeIsCalled.get()); + + var argumentCaptor = ArgumentCaptor.forClass(ABSTRACT_REQUEST_BUILDER_CLASS); + verify(brokerToController).sendRequest( + argumentCaptor.capture(), + any(ControllerRequestCompletionHandler.class)); + var capturedRequest = ((EnvelopeRequest.Builder) argumentCaptor.getValue()).build(ApiKeys.ENVELOPE.latestVersion()); + assertEquals(userPrincipal, SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal()))); + } + + @Test + public void testTopicCreationWithMetadataContextWhenPrincipalSerdeNotDefined() throws UnknownHostException { + var topicName = "topic"; + var requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.empty()); + + // Throw upon undefined principal serde when building the forward request + assertThrows(IllegalArgumentException.class, () -> autoTopicCreationManager.createTopics( + Set.of(topicName), Optional.of(requestContext))); + } + + @Test + public void testTopicCreationWithMetadataContextNoRetryUponUnsupportedVersion() throws UnknownHostException { + var topicName = "topic"; + var principalSerde = new KafkaPrincipalSerde() { + @Override + public byte[] serialize(KafkaPrincipal principal) { + return Utils.utf8(principal.toString()); + } + + @Override + public KafkaPrincipal deserialize(byte[] bytes) { + return SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)); + } + }; + + var requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.of(principalSerde)); + autoTopicCreationManager.createTopics(Set.of(topicName), Optional.of(requestContext)); + autoTopicCreationManager.createTopics(Set.of(topicName), Optional.of(requestContext)); + + // Should only trigger once + var argumentCaptor = ArgumentCaptor.forClass(ControllerRequestCompletionHandler.class); + verify(brokerToController).sendRequest(any(), argumentCaptor.capture()); + + // Complete with unsupported version will not trigger a retry, but cleanup the inflight topics instead + var header = new RequestHeader(ApiKeys.ENVELOPE, (short) 0, "client", 1); + var response = new EnvelopeResponse(ByteBuffer.allocate(0), Errors.UNSUPPORTED_VERSION); + var clientResponse = new ClientResponse(header, null, null, + 0, 0, false, null, null, response); + argumentCaptor.getValue().onComplete(clientResponse); + verify(brokerToController, times(1)).sendRequest( + any(ABSTRACT_REQUEST_BUILDER_CLASS), + argumentCaptor.capture()); + + // Could do the send again as inflight topics are cleared. + autoTopicCreationManager.createTopics(Set.of(topicName), Optional.of(requestContext)); + verify(brokerToController, times(2)).sendRequest( + any(ABSTRACT_REQUEST_BUILDER_CLASS), + argumentCaptor.capture()); + } + + @Test + public void testCreateStreamsInternalTopics() throws UnknownHostException { + var topicConfig = new CreatableTopicConfigCollection(); + topicConfig.add(new CreatableTopicConfig().setName("cleanup.policy").setValue("compact")); + + var topics = new LinkedHashMap(); + topics.put("stream-topic-1", new CreatableTopic() + .setName("stream-topic-1") + .setNumPartitions(3) + .setReplicationFactor((short) 2) + .setConfigs(topicConfig)); + topics.put("stream-topic-2", new CreatableTopic() + .setName("stream-topic-2") + .setNumPartitions(1) + .setReplicationFactor((short) 1)); + var requestContext = initializeRequestContextWithUserPrincipal(); + + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + groupCoordinator, + Properties::new, + shareCoordinator); + + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); + + var argumentCaptor = ArgumentCaptor.forClass(ABSTRACT_REQUEST_BUILDER_CLASS); + verify(brokerToController).sendRequest( + argumentCaptor.capture(), + any(ControllerRequestCompletionHandler.class)); + + var requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, ApiKeys.CREATE_TOPICS.latestVersion(), "clientId", 0); + var capturedRequest = ((EnvelopeRequest.Builder) argumentCaptor.getValue()).build(ApiKeys.ENVELOPE.latestVersion()); + var topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection(); + topicsCollection.add(getNewTopic("stream-topic-1", 3, (short) 2).setConfigs(topicConfig)); + topicsCollection.add(getNewTopic("stream-topic-2", 1, (short) 1)); + var requestBody = new CreateTopicsRequest.Builder( + new CreateTopicsRequestData() + .setTopics(topicsCollection) + .setTimeoutMs(requestTimeout)) + .build(ApiKeys.CREATE_TOPICS.latestVersion()); + + var forwardedRequestBuffer = capturedRequest.requestData().duplicate(); + assertEquals(requestHeader, RequestHeader.parse(forwardedRequestBuffer)); + assertEquals(requestBody.data(), CreateTopicsRequest.parse(new ByteBufferAccessor(forwardedRequestBuffer), + ApiKeys.CREATE_TOPICS.latestVersion()).data()); + } + + @Test + public void testCreateStreamsInternalTopicsWithEmptyTopics() throws UnknownHostException { + var topics = Map.of(); + var requestContext = initializeRequestContextWithUserPrincipal(); + + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + groupCoordinator, + Properties::new, + shareCoordinator); + + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); + + verify(brokerToController, never()).sendRequest( + any(ABSTRACT_REQUEST_BUILDER_CLASS), + any(ControllerRequestCompletionHandler.class)); + } + + @Test + public void testCreateStreamsInternalTopicsWithDefaultConfig() throws UnknownHostException { + var topics = Map.of( + "stream-topic-1", + new CreatableTopic() + .setName("stream-topic-1") + .setNumPartitions(-1) + .setReplicationFactor((short) -1)); + var requestContext = initializeRequestContextWithUserPrincipal(); + + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + groupCoordinator, + Properties::new, + shareCoordinator); + + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); + + var argumentCaptor = ArgumentCaptor.forClass(ABSTRACT_REQUEST_BUILDER_CLASS); + verify(brokerToController).sendRequest( + argumentCaptor.capture(), + any(ControllerRequestCompletionHandler.class)); + + var capturedRequest = ((EnvelopeRequest.Builder) argumentCaptor.getValue()).build(ApiKeys.ENVELOPE.latestVersion()); + + var requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, ApiKeys.CREATE_TOPICS.latestVersion(), "clientId", 0); + var topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection(); + topicsCollection.add(getNewTopic("stream-topic-1", config.numPartitions(), (short) config.defaultReplicationFactor())); + var requestBody = new CreateTopicsRequest.Builder( + new CreateTopicsRequestData() + .setTopics(topicsCollection) + .setTimeoutMs(requestTimeout)) + .build(ApiKeys.CREATE_TOPICS.latestVersion()); + var forwardedRequestBuffer = capturedRequest.requestData().duplicate(); + assertEquals(requestHeader, RequestHeader.parse(forwardedRequestBuffer)); + assertEquals(requestBody.data(), CreateTopicsRequest.parse(new ByteBufferAccessor(forwardedRequestBuffer), + ApiKeys.CREATE_TOPICS.latestVersion()).data()); + } + + @Test + public void testCreateStreamsInternalTopicsPassesPrincipal() throws UnknownHostException { + var topics = Map.of( + "stream-topic-1", + new CreatableTopic() + .setName("stream-topic-1") + .setNumPartitions(-1) + .setReplicationFactor((short) -1)); + var requestContext = initializeRequestContextWithUserPrincipal(); + + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + groupCoordinator, + Properties::new, + shareCoordinator); + + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); + + var argumentCaptor = ArgumentCaptor.forClass(ABSTRACT_REQUEST_BUILDER_CLASS); + + verify(brokerToController).sendRequest( + argumentCaptor.capture(), + any(ControllerRequestCompletionHandler.class)); + var capturedRequest = ((EnvelopeRequest.Builder) argumentCaptor.getValue()) + .build(ApiKeys.ENVELOPE.latestVersion()); + assertEquals(new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user"), + SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal()))); + } + + private RequestContext initializeRequestContextWithUserPrincipal() throws UnknownHostException { + var userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user"); + var principalSerde = new KafkaPrincipalSerde() { + @Override + public byte[] serialize(KafkaPrincipal principal) { + return Utils.utf8(principal.toString()); + } + @Override + public KafkaPrincipal deserialize(byte[] bytes) { + return SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)); + } + }; + return initializeRequestContext(userPrincipal, Optional.of(principalSerde)); + } + + private RequestContext initializeRequestContext( + KafkaPrincipal kafkaPrincipal, + Optional principalSerde + ) throws UnknownHostException { + + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + groupCoordinator, + Properties::new, + shareCoordinator); + + var createTopicApiVersion = new ApiVersionsResponseData.ApiVersion() + .setApiKey(ApiKeys.CREATE_TOPICS.id) + .setMinVersion(ApiKeys.CREATE_TOPICS.oldestVersion()) + .setMaxVersion(ApiKeys.CREATE_TOPICS.latestVersion()); + Mockito.when(brokerToController.controllerApiVersions()) + .thenReturn(Optional.of(NodeApiVersions.create(List.of(createTopicApiVersion)))); + + var requestHeader = new RequestHeader(ApiKeys.METADATA, ApiKeys.METADATA.latestVersion(), + "clientId", 0); + return new RequestContext(requestHeader, "1", InetAddress.getLocalHost(), Optional.empty(), + kafkaPrincipal, ListenerName.forSecurityProtocol(SecurityProtocol.PLAINTEXT), + SecurityProtocol.PLAINTEXT, ClientInformation.EMPTY, false, principalSerde); + } + + private void createTopicAndVerifyResult( + Errors error, + String topicName, + boolean isInternal + ) { + var topicResponses = autoTopicCreationManager.createTopics(Set.of(topicName), Optional.empty()); + + var expectedResponses = List.of(new MetadataResponseTopic() + .setErrorCode(error.code()) + .setIsInternal(isInternal) + .setName(topicName)); + + assertEquals(expectedResponses, topicResponses); + } + + private static CreatableTopic getNewTopic( + String topicName, + int numPartitions, + short replicationFactor + ) { + return new CreatableTopic() + .setName(topicName) + .setNumPartitions(numPartitions) + .setReplicationFactor(replicationFactor); + } +} From ff79734d23e51f2f82c0cc15cfbfd73d88988b8d Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Mon, 25 Aug 2025 21:02:33 +0800 Subject: [PATCH 02/14] Address comments --- .../scala/kafka/server/BrokerServer.scala | 7 +++-- .../main/scala/kafka/server/KafkaApis.scala | 2 +- .../server/AutoTopicCreationManager.java | 21 ++++++++----- .../DefaultAutoTopicCreationManager.java | 23 ++++++-------- .../kafka/server/ForwardingManagerUtils.java | 5 +--- .../server/AutoTopicCreationManagerTest.java | 30 ++++++++----------- 6 files changed, 41 insertions(+), 47 deletions(-) diff --git a/core/src/main/scala/kafka/server/BrokerServer.scala b/core/src/main/scala/kafka/server/BrokerServer.scala index b47bd9dd8655b..54972d8a92ed2 100644 --- a/core/src/main/scala/kafka/server/BrokerServer.scala +++ b/core/src/main/scala/kafka/server/BrokerServer.scala @@ -386,8 +386,11 @@ class BrokerServer( producerIdManagerSupplier, metrics, metadataCache, Time.SYSTEM) autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, clientToControllerChannelManager, groupCoordinator, - () => transactionCoordinator.transactionTopicConfigs, shareCoordinator) + config, clientToControllerChannelManager, + () => groupCoordinator.groupMetadataTopicConfigs(), + () => transactionCoordinator.transactionTopicConfigs, + () => shareCoordinator.shareGroupStateTopicConfigs() + ) dynamicConfigHandlers = Map[ConfigType, ConfigHandler]( ConfigType.TOPIC -> new TopicConfigHandler(replicaManager, config, quotaManagers), diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index ff2a1c39733d0..1462207245ec5 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -1350,7 +1350,7 @@ class KafkaApis(val requestChannel: RequestChannel, val topicMetadata = metadataCache.getTopicMetadata(internalTopics, request.context.listenerName, false, false).asScala if (topicMetadata.headOption.isEmpty) { - autoTopicCreationManager.createTopics(internalTopics, Optional.empty) + autoTopicCreationManager.createInternalTopics(internalTopics) (Errors.COORDINATOR_NOT_AVAILABLE, Node.noNode) } else { if (topicMetadata.head.errorCode != Errors.NONE.code) { diff --git a/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java index a61bff08af59a..ea35b2481efc1 100644 --- a/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java @@ -37,10 +37,18 @@ public interface AutoTopicCreationManager { * inside Envelope to send to the controller when forwarding is enabled. * @return auto created topic metadata responses */ - List createTopics( - Set topics, - Optional metadataRequestContext - ); + List createTopics(Set topics, Optional metadataRequestContext); + + /** + * Initiate auto topic creation for the given topics. + * This method is used for creating internal topics. + * + * @param topics the topics to create + * @return auto created topic metadata responses + */ + default List createInternalTopics(Set topics) { + return createTopics(topics, Optional.empty()); + } /** * Initiate auto topic creation for the given topics. @@ -51,8 +59,5 @@ List createTopics( * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest * inside Envelope to send to the controller when forwarding is enabled. */ - void createStreamsInternalTopics( - Map topics, - RequestContext metadataRequestContext - ); + void createStreamsInternalTopics(Map topics, RequestContext metadataRequestContext); } diff --git a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java index 298ed79640032..9451f3e3a6f9c 100644 --- a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java @@ -32,9 +32,7 @@ import org.apache.kafka.common.requests.CreateTopicsResponse; import org.apache.kafka.common.requests.RequestContext; import org.apache.kafka.common.requests.RequestHeader; -import org.apache.kafka.coordinator.group.GroupCoordinator; import org.apache.kafka.coordinator.group.GroupCoordinatorConfig; -import org.apache.kafka.coordinator.share.ShareCoordinator; import org.apache.kafka.coordinator.share.ShareCoordinatorConfig; import org.apache.kafka.coordinator.transaction.TransactionLogConfig; import org.apache.kafka.server.common.ControllerRequestCompletionHandler; @@ -61,30 +59,27 @@ public class DefaultAutoTopicCreationManager implements AutoTopicCreationManager private final AbstractKafkaConfig config; private final NodeToControllerChannelManager channelManager; - private final GroupCoordinator groupCoordinator; - private final ShareCoordinator shareCoordinator; + private final Supplier groupCoordinator; + private final Supplier shareCoordinator; private final Supplier transactionTopicConfigsSupplier; private final Set inflightTopics = ConcurrentHashMap.newKeySet(); public DefaultAutoTopicCreationManager( AbstractKafkaConfig config, NodeToControllerChannelManager channelManager, - GroupCoordinator groupCoordinator, + Supplier groupCoordinatorConfigsSupplier, Supplier transactionTopicConfigsSupplier, - ShareCoordinator shareCoordinator + Supplier shareCoordinatorConfigsSupplier ) { this.config = config; this.channelManager = channelManager; - this.groupCoordinator = groupCoordinator; - this.shareCoordinator = shareCoordinator; + this.groupCoordinator = groupCoordinatorConfigsSupplier; + this.shareCoordinator = shareCoordinatorConfigsSupplier; this.transactionTopicConfigsSupplier = transactionTopicConfigsSupplier; } @Override - public List createTopics( - Set topics, - Optional metadataRequestContext - ) { + public List createTopics(Set topics, Optional metadataRequestContext) { var creatableTopics = new HashMap(); var uncreatableTopicResponses = new ArrayList(); topics.forEach(topic -> { @@ -221,7 +216,7 @@ yield new CreatableTopic() .setName(topic) .setNumPartitions(groupCoordinatorConfig.offsetsTopicPartitions()) .setReplicationFactor(groupCoordinatorConfig.offsetsTopicReplicationFactor()) - .setConfigs(convertToTopicConfigCollections(groupCoordinator.groupMetadataTopicConfigs())); + .setConfigs(convertToTopicConfigCollections(groupCoordinator.get())); } case Topic.TRANSACTION_STATE_TOPIC_NAME -> { var transactionLogConfig = new TransactionLogConfig(config); @@ -237,7 +232,7 @@ yield new CreatableTopic() .setName(topic) .setNumPartitions(shareCoordinatorConfig.shareCoordinatorStateTopicNumPartitions()) .setReplicationFactor(shareCoordinatorConfig.shareCoordinatorStateTopicReplicationFactor()) - .setConfigs(convertToTopicConfigCollections(shareCoordinator.shareGroupStateTopicConfigs())); + .setConfigs(convertToTopicConfigCollections(shareCoordinator.get())); } default -> new CreatableTopic() .setName(topic) diff --git a/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java b/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java index 43a9dfc276c95..6fc623a3fbdf1 100644 --- a/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java +++ b/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java @@ -24,10 +24,7 @@ public class ForwardingManagerUtils { - public static EnvelopeRequest.Builder buildEnvelopeRequest( - RequestContext context, - ByteBuffer forwardRequestBuffer - ) { + public static EnvelopeRequest.Builder buildEnvelopeRequest(RequestContext context, ByteBuffer forwardRequestBuffer) { var principalSerde = context.principalSerde.orElseThrow(() -> new IllegalArgumentException( String.format("Cannot deserialize principal from request context %s since there is no serde defined", context)) ); diff --git a/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java index 2e91afeb6042e..30d8e0011f9fb 100644 --- a/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java +++ b/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java @@ -42,9 +42,7 @@ import org.apache.kafka.common.security.auth.SecurityProtocol; import org.apache.kafka.common.utils.SecurityUtils; import org.apache.kafka.common.utils.Utils; -import org.apache.kafka.coordinator.group.GroupCoordinator; import org.apache.kafka.coordinator.group.GroupCoordinatorConfig; -import org.apache.kafka.coordinator.share.ShareCoordinator; import org.apache.kafka.coordinator.share.ShareCoordinatorConfig; import org.apache.kafka.coordinator.transaction.TransactionLogConfig; import org.apache.kafka.metadata.MetadataCache; @@ -91,8 +89,6 @@ public class AutoTopicCreationManagerTest { private AbstractKafkaConfig config; private final MetadataCache metadataCache = Mockito.mock(MetadataCache.class); private final NodeToControllerChannelManager brokerToController = Mockito.mock(NodeToControllerChannelManager.class); - private final GroupCoordinator groupCoordinator = Mockito.mock(GroupCoordinator.class); - private final ShareCoordinator shareCoordinator = Mockito.mock(ShareCoordinator.class); private AutoTopicCreationManager autoTopicCreationManager; private final int internalTopicPartitions = 2; @@ -120,7 +116,6 @@ public void setup() { @Test public void testCreateOffsetTopic() { - Mockito.when(groupCoordinator.groupMetadataTopicConfigs()).thenReturn(new Properties()); testCreateTopic(GROUP_METADATA_TOPIC_NAME, true, internalTopicPartitions, internalTopicReplicationFactor); } @@ -131,7 +126,6 @@ public void testCreateTxnTopic() { @Test public void testCreateShareStateTopic() { - Mockito.when(shareCoordinator.shareGroupStateTopicConfigs()).thenReturn(new Properties()); testCreateTopic(SHARE_GROUP_STATE_TOPIC_NAME, true, internalTopicPartitions, internalTopicReplicationFactor); } @@ -149,9 +143,9 @@ private void testCreateTopic( autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, brokerToController, - groupCoordinator, Properties::new, - shareCoordinator); + Properties::new, + Properties::new); var topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection(); topicsCollection.add(getNewTopic(topicName, numPartitions, replicationFactor)); @@ -272,9 +266,9 @@ public void testCreateStreamsInternalTopics() throws UnknownHostException { autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, brokerToController, - groupCoordinator, Properties::new, - shareCoordinator); + Properties::new, + Properties::new); autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); @@ -308,9 +302,9 @@ public void testCreateStreamsInternalTopicsWithEmptyTopics() throws UnknownHostE autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, brokerToController, - groupCoordinator, Properties::new, - shareCoordinator); + Properties::new, + Properties::new); autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); @@ -332,9 +326,9 @@ public void testCreateStreamsInternalTopicsWithDefaultConfig() throws UnknownHos autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, brokerToController, - groupCoordinator, Properties::new, - shareCoordinator); + Properties::new, + Properties::new); autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); @@ -372,9 +366,9 @@ public void testCreateStreamsInternalTopicsPassesPrincipal() throws UnknownHostE autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, brokerToController, - groupCoordinator, Properties::new, - shareCoordinator); + Properties::new, + Properties::new); autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); @@ -412,9 +406,9 @@ private RequestContext initializeRequestContext( autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, brokerToController, - groupCoordinator, Properties::new, - shareCoordinator); + Properties::new, + Properties::new); var createTopicApiVersion = new ApiVersionsResponseData.ApiVersion() .setApiKey(ApiKeys.CREATE_TOPICS.id) From 6674bc95a1868d14178a9bbaa8297df02a90879e Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Tue, 26 Aug 2025 00:13:18 +0800 Subject: [PATCH 03/14] fix failed test --- core/src/test/scala/unit/kafka/server/KafkaApisTest.scala | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala index a1fa5cd73a324..2b90b9910e09a 100644 --- a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala +++ b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala @@ -773,7 +773,7 @@ class KafkaApisTest extends Logging { when(clientRequestQuotaManager.maybeRecordAndGetThrottleTimeMs(any[RequestChannel.Request](), any[Long])).thenReturn(0) - val capturedRequest = verifyTopicCreation(topicName, enableAutoTopicCreation = true, isInternal = true, request) + verifyTopicCreation(topicName, enableAutoTopicCreation = true, isInternal = true, request) kafkaApis = createKafkaApis(authorizer = Some(authorizer), overrideProperties = topicConfigOverride) kafkaApis.handleFindCoordinatorRequest(request) @@ -786,10 +786,6 @@ class KafkaApisTest extends Logging { assertEquals(key, response.data.coordinators.get(0).key) } else { assertEquals(Errors.COORDINATOR_NOT_AVAILABLE.code, response.data.errorCode) - assertTrue(capturedRequest.getValue.isEmpty) - } - if (checkAutoCreateTopic) { - assertTrue(capturedRequest.getValue.isEmpty) } } @@ -947,7 +943,7 @@ class KafkaApisTest extends Logging { assertEquals(expectedMetadataResponse, response.topicMetadata()) - if (enableAutoTopicCreation) { + if (enableAutoTopicCreation && isInternal) { assertTrue(capturedRequest.getValue.isPresent) assertEquals(request.context, capturedRequest.getValue.get) } From 5654a428cff7ac60f987f403ec52da9228764ef8 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Mon, 1 Sep 2025 23:21:20 +0800 Subject: [PATCH 04/14] Apply changes from AutoTopicCreationManager and test --- .../server/AutoTopicCreationManager.scala | 267 ------------- .../server/AutoTopicCreationManagerTest.scala | 359 ------------------ .../DefaultAutoTopicCreationManager.java | 8 - ... DefaultAutoTopicCreationManagerTest.java} | 42 +- 4 files changed, 1 insertion(+), 675 deletions(-) delete mode 100644 core/src/main/scala/kafka/server/AutoTopicCreationManager.scala delete mode 100644 core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala rename server/src/test/java/org/apache/kafka/server/{AutoTopicCreationManagerTest.java => DefaultAutoTopicCreationManagerTest.java} (90%) diff --git a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala b/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala deleted file mode 100644 index d23fc7db88a01..0000000000000 --- a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala +++ /dev/null @@ -1,267 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.util.concurrent.ConcurrentHashMap -import java.util.{Collections, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.Logging -import org.apache.kafka.clients.ClientResponse -import org.apache.kafka.common.errors.InvalidTopicException -import org.apache.kafka.common.internals.Topic -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.CreateTopicsRequestData -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.protocol.{ApiKeys, Errors} -import org.apache.kafka.common.requests.{CreateTopicsRequest, CreateTopicsResponse, RequestContext, RequestHeader} -import org.apache.kafka.coordinator.group.GroupCoordinator -import org.apache.kafka.coordinator.share.ShareCoordinator -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.common.{ControllerRequestCompletionHandler, NodeToControllerChannelManager} -import org.apache.kafka.server.quota.ControllerMutationQuota - -import scala.collection.{Map, Seq, Set, mutable} -import scala.jdk.CollectionConverters._ -import scala.jdk.OptionConverters.RichOptional - -trait AutoTopicCreationManager { - - def createTopics( - topicNames: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] - - def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext - ): Unit - -} - -class DefaultAutoTopicCreationManager( - config: KafkaConfig, - channelManager: NodeToControllerChannelManager, - groupCoordinator: GroupCoordinator, - txnCoordinator: TransactionCoordinator, - shareCoordinator: ShareCoordinator -) extends AutoTopicCreationManager with Logging { - - private val inflightTopics = Collections.newSetFromMap(new ConcurrentHashMap[String, java.lang.Boolean]()) - - /** - * Initiate auto topic creation for the given topics. - * - * @param topics the topics to create - * @param controllerMutationQuota the controller mutation quota for topic creation - * @param metadataRequestContext defined when creating topics on behalf of the client. The goal here is to preserve - * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest - * inside Envelope to send to the controller when forwarding is enabled. - * @return auto created topic metadata responses - */ - override def createTopics( - topics: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val (creatableTopics, uncreatableTopicResponses) = filterCreatableTopics(topics) - - val creatableTopicResponses = if (creatableTopics.isEmpty) { - Seq.empty - } else { - sendCreateTopicRequest(creatableTopics, metadataRequestContext) - } - - uncreatableTopicResponses ++ creatableTopicResponses - } - - override def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext - ): Unit = { - if (topics.nonEmpty) { - sendCreateTopicRequest(topics, Some(requestContext)) - } - } - - private def sendCreateTopicRequest( - creatableTopics: Map[String, CreatableTopic], - requestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val topicsToCreate = new CreateTopicsRequestData.CreatableTopicCollection(creatableTopics.size) - topicsToCreate.addAll(creatableTopics.values.asJavaCollection) - - val createTopicsRequest = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTimeoutMs(config.requestTimeoutMs) - .setTopics(topicsToCreate) - ) - - val requestCompletionHandler = new ControllerRequestCompletionHandler { - override def onTimeout(): Unit = { - clearInflightRequests(creatableTopics) - debug(s"Auto topic creation timed out for ${creatableTopics.keys}.") - } - - override def onComplete(response: ClientResponse): Unit = { - clearInflightRequests(creatableTopics) - if (response.authenticationException() != null) { - warn(s"Auto topic creation failed for ${creatableTopics.keys} with authentication exception") - } else if (response.versionMismatch() != null) { - warn(s"Auto topic creation failed for ${creatableTopics.keys} with invalid version exception") - } else { - if (response.hasResponse) { - response.responseBody() match { - case createTopicsResponse: CreateTopicsResponse => - createTopicsResponse.data().topics().forEach(topicResult => { - val error = Errors.forCode(topicResult.errorCode) - if (error != Errors.NONE) { - warn(s"Auto topic creation failed for ${topicResult.name} with error '${error.name}': ${topicResult.errorMessage}") - } - }) - case other => - warn(s"Auto topic creation request received unexpected response type: ${other.getClass.getSimpleName}") - } - } - debug(s"Auto topic creation completed for ${creatableTopics.keys} with response ${response.responseBody}.") - } - } - } - - val request = requestContext.map { context => - val requestVersion = - channelManager.controllerApiVersions.toScala match { - case None => - // We will rely on the Metadata request to be retried in the case - // that the latest version is not usable by the controller. - ApiKeys.CREATE_TOPICS.latestVersion() - case Some(nodeApiVersions) => - nodeApiVersions.latestUsableVersion(ApiKeys.CREATE_TOPICS) - } - - // Borrow client information such as client id and correlation id from the original request, - // in order to correlate the create request with the original metadata request. - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, - requestVersion, - context.clientId, - context.correlationId) - ForwardingManager.buildEnvelopeRequest(context, - createTopicsRequest.build(requestVersion).serializeWithHeader(requestHeader)) - }.getOrElse(createTopicsRequest) - - channelManager.sendRequest(request, requestCompletionHandler) - - val creatableTopicResponses = creatableTopics.keySet.toSeq.map { topic => - new MetadataResponseTopic() - .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - } - - info(s"Sent auto-creation request for ${creatableTopics.keys} to the active controller.") - creatableTopicResponses - } - - private def clearInflightRequests(creatableTopics: Map[String, CreatableTopic]): Unit = { - creatableTopics.keySet.foreach(inflightTopics.remove) - debug(s"Cleared inflight topic creation state for $creatableTopics") - } - - private def creatableTopic(topic: String): CreatableTopic = { - topic match { - case GROUP_METADATA_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.groupCoordinatorConfig.offsetsTopicPartitions) - .setReplicationFactor(config.groupCoordinatorConfig.offsetsTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections(groupCoordinator.groupMetadataTopicConfigs)) - case TRANSACTION_STATE_TOPIC_NAME => - val transactionLogConfig = new TransactionLogConfig(config) - new CreatableTopic() - .setName(topic) - .setNumPartitions(transactionLogConfig.transactionTopicPartitions) - .setReplicationFactor(transactionLogConfig.transactionTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections( - txnCoordinator.transactionTopicConfigs)) - case SHARE_GROUP_STATE_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.shareCoordinatorConfig.shareCoordinatorStateTopicNumPartitions()) - .setReplicationFactor(config.shareCoordinatorConfig.shareCoordinatorStateTopicReplicationFactor()) - .setConfigs(convertToTopicConfigCollections(shareCoordinator.shareGroupStateTopicConfigs())) - case topicName => - new CreatableTopic() - .setName(topicName) - .setNumPartitions(config.numPartitions) - .setReplicationFactor(config.defaultReplicationFactor.shortValue) - } - } - - private def convertToTopicConfigCollections(config: Properties): CreatableTopicConfigCollection = { - val topicConfigs = new CreatableTopicConfigCollection() - config.forEach { - case (name, value) => - topicConfigs.add(new CreatableTopicConfig() - .setName(name.toString) - .setValue(value.toString)) - } - topicConfigs - } - - private def isValidTopicName(topic: String): Boolean = { - try { - Topic.validate(topic) - true - } catch { - case _: InvalidTopicException => - false - } - } - - private def filterCreatableTopics( - topics: Set[String] - ): (Map[String, CreatableTopic], Seq[MetadataResponseTopic]) = { - - val creatableTopics = mutable.Map.empty[String, CreatableTopic] - val uncreatableTopics = mutable.Buffer.empty[MetadataResponseTopic] - - topics.foreach { topic => - // Attempt basic topic validation before sending any requests to the controller. - val validationError: Option[Errors] = if (!isValidTopicName(topic)) { - Some(Errors.INVALID_TOPIC_EXCEPTION) - } else if (!inflightTopics.add(topic)) { - Some(Errors.UNKNOWN_TOPIC_OR_PARTITION) - } else { - None - } - - validationError match { - case Some(error) => - uncreatableTopics += new MetadataResponseTopic() - .setErrorCode(error.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - case None => - creatableTopics.put(topic, creatableTopic(topic)) - } - } - - (creatableTopics, uncreatableTopics) - } -} diff --git a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala b/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala deleted file mode 100644 index 3065588a87aaf..0000000000000 --- a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala +++ /dev/null @@ -1,359 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.net.InetAddress -import java.nio.ByteBuffer -import java.util -import java.util.concurrent.atomic.AtomicBoolean -import java.util.{Collections, Optional, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.TestUtils -import org.apache.kafka.clients.{ClientResponse, NodeApiVersions, RequestCompletionHandler} -import org.apache.kafka.common.Node -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.{ApiVersionsResponseData, CreateTopicsRequestData} -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.network.{ClientInformation, ListenerName} -import org.apache.kafka.common.protocol.{ApiKeys, ByteBufferAccessor, Errors} -import org.apache.kafka.common.requests._ -import org.apache.kafka.common.security.auth.{KafkaPrincipal, KafkaPrincipalSerde, SecurityProtocol} -import org.apache.kafka.common.utils.{SecurityUtils, Utils} -import org.apache.kafka.coordinator.group.{GroupCoordinator, GroupCoordinatorConfig} -import org.apache.kafka.coordinator.share.{ShareCoordinator, ShareCoordinatorConfig} -import org.apache.kafka.metadata.MetadataCache -import org.apache.kafka.server.config.ServerConfigs -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.common.{ControllerRequestCompletionHandler, NodeToControllerChannelManager} -import org.apache.kafka.server.quota.ControllerMutationQuota -import org.junit.jupiter.api.Assertions.{assertEquals, assertThrows, assertTrue} -import org.junit.jupiter.api.{BeforeEach, Test} -import org.mockito.ArgumentMatchers.any -import org.mockito.Mockito.never -import org.mockito.{ArgumentCaptor, ArgumentMatchers, Mockito} - -import scala.collection.{Map, Seq} - -class AutoTopicCreationManagerTest { - - private val requestTimeout = 100 - private var config: KafkaConfig = _ - private val metadataCache = Mockito.mock(classOf[MetadataCache]) - private val brokerToController = Mockito.mock(classOf[NodeToControllerChannelManager]) - private val groupCoordinator = Mockito.mock(classOf[GroupCoordinator]) - private val transactionCoordinator = Mockito.mock(classOf[TransactionCoordinator]) - private val shareCoordinator = Mockito.mock(classOf[ShareCoordinator]) - private var autoTopicCreationManager: AutoTopicCreationManager = _ - - private val internalTopicPartitions = 2 - private val internalTopicReplicationFactor: Short = 2 - - @BeforeEach - def setup(): Unit = { - val props = TestUtils.createBrokerConfig(1) - props.setProperty(ServerConfigs.REQUEST_TIMEOUT_MS_CONFIG, requestTimeout.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicPartitions.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicPartitions.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_REPLICATION_FACTOR_CONFIG , internalTopicPartitions.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_NUM_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - - config = KafkaConfig.fromProps(props) - val aliveBrokers = util.List.of(new Node(0, "host0", 0), new Node(1, "host1", 1)) - - Mockito.when(metadataCache.getAliveBrokerNodes(any(classOf[ListenerName]))).thenReturn(aliveBrokers) - } - - @Test - def testCreateOffsetTopic(): Unit = { - Mockito.when(groupCoordinator.groupMetadataTopicConfigs).thenReturn(new Properties) - testCreateTopic(GROUP_METADATA_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateTxnTopic(): Unit = { - Mockito.when(transactionCoordinator.transactionTopicConfigs).thenReturn(new Properties) - testCreateTopic(TRANSACTION_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateShareStateTopic(): Unit = { - Mockito.when(shareCoordinator.shareGroupStateTopicConfigs()).thenReturn(new Properties) - testCreateTopic(SHARE_GROUP_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateNonInternalTopic(): Unit = { - testCreateTopic("topic", isInternal = false) - } - - private def testCreateTopic(topicName: String, - isInternal: Boolean, - numPartitions: Int = 1, - replicationFactor: Short = 1): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - val topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection - topicsCollection.add(getNewTopic(topicName, numPartitions, replicationFactor)) - val requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - - // Calling twice with the same topic will only trigger one forwarding. - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - - Mockito.verify(brokerToController).sendRequest( - ArgumentMatchers.eq(requestBody), - any(classOf[ControllerRequestCompletionHandler])) - } - - @Test - def testTopicCreationWithMetadataContextPassPrincipal(): Unit = { - val topicName = "topic" - - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val serializeIsCalled = new AtomicBoolean(false) - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - assertEquals(principal, userPrincipal) - serializeIsCalled.set(true) - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - - val requestContext = initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - - assertTrue(serializeIsCalled.get()) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - assertEquals(userPrincipal, SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal))) - } - - @Test - def testTopicCreationWithMetadataContextWhenPrincipalSerdeNotDefined(): Unit = { - val topicName = "topic" - - val requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.empty()) - - // Throw upon undefined principal serde when building the forward request - assertThrows(classOf[IllegalArgumentException], () => autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext))) - } - - @Test - def testTopicCreationWithMetadataContextNoRetryUponUnsupportedVersion(): Unit = { - val topicName = "topic" - - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - - val requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.of(principalSerde)) - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - - // Should only trigger once - val argumentCaptor = ArgumentCaptor.forClass(classOf[ControllerRequestCompletionHandler]) - Mockito.verify(brokerToController).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Complete with unsupported version will not trigger a retry, but cleanup the inflight topics instead - val header = new RequestHeader(ApiKeys.ENVELOPE, 0, "client", 1) - val response = new EnvelopeResponse(ByteBuffer.allocate(0), Errors.UNSUPPORTED_VERSION) - val clientResponse = new ClientResponse(header, null, null, - 0, 0, false, null, null, response) - argumentCaptor.getValue.asInstanceOf[RequestCompletionHandler].onComplete(clientResponse) - Mockito.verify(brokerToController, Mockito.times(1)).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Could do the send again as inflight topics are cleared. - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - Mockito.verify(brokerToController, Mockito.times(2)).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - } - - @Test - def testCreateStreamsInternalTopics(): Unit = { - val topicConfig = new CreatableTopicConfigCollection() - topicConfig.add(new CreatableTopicConfig().setName("cleanup.policy").setValue("compact")) - - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(3).setReplicationFactor(2).setConfigs(topicConfig), - "stream-topic-2" -> new CreatableTopic().setName("stream-topic-2").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, ApiKeys.CREATE_TOPICS.latestVersion(), "clientId", 0) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - val topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection - topicsCollection.add(getNewTopic("stream-topic-1", 3, 2.toShort).setConfigs(topicConfig)) - topicsCollection.add(getNewTopic("stream-topic-2", 1, 1.toShort)) - val requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - .build(ApiKeys.CREATE_TOPICS.latestVersion()) - - val forwardedRequestBuffer = capturedRequest.requestData().duplicate() - assertEquals(requestHeader, RequestHeader.parse(forwardedRequestBuffer)) - assertEquals(requestBody.data(), CreateTopicsRequest.parse(new ByteBufferAccessor(forwardedRequestBuffer), - ApiKeys.CREATE_TOPICS.latestVersion()).data()) - } - - @Test - def testCreateStreamsInternalTopicsWithEmptyTopics(): Unit = { - val topics = Map.empty[String, CreatableTopic] - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext) - - Mockito.verify(brokerToController, never()).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - any(classOf[ControllerRequestCompletionHandler])) - } - - @Test - def testCreateStreamsInternalTopicsPassesPrincipal(): Unit = { - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(-1).setReplicationFactor(-1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - assertEquals(new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user"), SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal))) - } - - private def initializeRequestContextWithUserPrincipal(): RequestContext = { - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - } - - private def initializeRequestContext(kafkaPrincipal: KafkaPrincipal, - principalSerde: Optional[KafkaPrincipalSerde]): RequestContext = { - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator) - - val createTopicApiVersion = new ApiVersionsResponseData.ApiVersion() - .setApiKey(ApiKeys.CREATE_TOPICS.id) - .setMinVersion(ApiKeys.CREATE_TOPICS.oldestVersion()) - .setMaxVersion(ApiKeys.CREATE_TOPICS.latestVersion()) - Mockito.when(brokerToController.controllerApiVersions()) - .thenReturn(Optional.of(NodeApiVersions.create(Collections.singleton(createTopicApiVersion)))) - - val requestHeader = new RequestHeader(ApiKeys.METADATA, ApiKeys.METADATA.latestVersion, - "clientId", 0) - new RequestContext(requestHeader, "1", InetAddress.getLocalHost, Optional.empty(), - kafkaPrincipal, ListenerName.forSecurityProtocol(SecurityProtocol.PLAINTEXT), - SecurityProtocol.PLAINTEXT, ClientInformation.EMPTY, false, principalSerde) - } - - private def createTopicAndVerifyResult(error: Errors, - topicName: String, - isInternal: Boolean, - metadataContext: Option[RequestContext] = None): Unit = { - val topicResponses = autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, metadataContext) - - val expectedResponses = Seq(new MetadataResponseTopic() - .setErrorCode(error.code()) - .setIsInternal(isInternal) - .setName(topicName)) - - assertEquals(expectedResponses, topicResponses) - } - - private def getNewTopic(topicName: String, numPartitions: Int, replicationFactor: Short): CreatableTopic = { - new CreatableTopic() - .setName(topicName) - .setNumPartitions(numPartitions) - .setReplicationFactor(replicationFactor) - } -} diff --git a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java index 9451f3e3a6f9c..517291ac31d74 100644 --- a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java @@ -116,14 +116,6 @@ public void createStreamsInternalTopics( if (topics.isEmpty()) { return; } - topics.values().forEach(creatableTopic -> { - if (creatableTopic.numPartitions() == -1) { - creatableTopic.setNumPartitions(config.numPartitions()); - } - if (creatableTopic.replicationFactor() == -1) { - creatableTopic.setReplicationFactor((short) config.defaultReplicationFactor()); - } - }); sendCreateTopicRequest(topics, Optional.of(requestContext)); } diff --git a/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java similarity index 90% rename from server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java rename to server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java index 30d8e0011f9fb..82822339d992d 100644 --- a/server/src/test/java/org/apache/kafka/server/AutoTopicCreationManagerTest.java +++ b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java @@ -80,7 +80,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -public class AutoTopicCreationManagerTest { +public class DefaultAutoTopicCreationManagerTest { @SuppressWarnings("unchecked") private static final Class> ABSTRACT_REQUEST_BUILDER_CLASS = (Class>) (Class) AbstractRequest.Builder.class; @@ -313,46 +313,6 @@ public void testCreateStreamsInternalTopicsWithEmptyTopics() throws UnknownHostE any(ControllerRequestCompletionHandler.class)); } - @Test - public void testCreateStreamsInternalTopicsWithDefaultConfig() throws UnknownHostException { - var topics = Map.of( - "stream-topic-1", - new CreatableTopic() - .setName("stream-topic-1") - .setNumPartitions(-1) - .setReplicationFactor((short) -1)); - var requestContext = initializeRequestContextWithUserPrincipal(); - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - Properties::new, - Properties::new, - Properties::new); - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); - - var argumentCaptor = ArgumentCaptor.forClass(ABSTRACT_REQUEST_BUILDER_CLASS); - verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(ControllerRequestCompletionHandler.class)); - - var capturedRequest = ((EnvelopeRequest.Builder) argumentCaptor.getValue()).build(ApiKeys.ENVELOPE.latestVersion()); - - var requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, ApiKeys.CREATE_TOPICS.latestVersion(), "clientId", 0); - var topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection(); - topicsCollection.add(getNewTopic("stream-topic-1", config.numPartitions(), (short) config.defaultReplicationFactor())); - var requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - .build(ApiKeys.CREATE_TOPICS.latestVersion()); - var forwardedRequestBuffer = capturedRequest.requestData().duplicate(); - assertEquals(requestHeader, RequestHeader.parse(forwardedRequestBuffer)); - assertEquals(requestBody.data(), CreateTopicsRequest.parse(new ByteBufferAccessor(forwardedRequestBuffer), - ApiKeys.CREATE_TOPICS.latestVersion()).data()); - } - @Test public void testCreateStreamsInternalTopicsPassesPrincipal() throws UnknownHostException { var topics = Map.of( From 326913d80d3eac58ff05a725f7149b40f7d476b1 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Fri, 12 Sep 2025 21:58:37 +0800 Subject: [PATCH 05/14] Address comments --- .../DefaultAutoTopicCreationManager.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java index 517291ac31d74..4c60958396e33 100644 --- a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java @@ -84,23 +84,20 @@ public List createTopics(Set topics, Optional(); topics.forEach(topic -> { // Attempt basic topic validation before sending any requests to the controller. - Optional validationError; + Optional validationError = Optional.empty(); if (!isValidTopicName(topic)) { validationError = Optional.of(Errors.INVALID_TOPIC_EXCEPTION); } else if (!inflightTopics.add(topic)) { validationError = Optional.of(Errors.UNKNOWN_TOPIC_OR_PARTITION); - } else { - validationError = Optional.empty(); } - if (validationError.isPresent()) { - uncreatableTopicResponses.add(new MetadataResponseTopic() - .setErrorCode(validationError.get().code()) - .setName(topic) - .setIsInternal(Topic.isInternal(topic))); - } else { - creatableTopics.put(topic, creatableTopic(topic)); - } + validationError.ifPresentOrElse( + error -> uncreatableTopicResponses.add(new MetadataResponseTopic() + .setErrorCode(error.code()) + .setName(topic) + .setIsInternal(Topic.isInternal(topic))), + () -> creatableTopics.put(topic, creatableTopic(topic)) + ); }); var creatableTopicResponses = creatableTopics.isEmpty() ? List.of() : sendCreateTopicRequest(creatableTopics, metadataRequestContext); From d8e5280a318cf8b99e247ead96bb2d76a395fc11 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Sat, 13 Sep 2025 00:02:21 +0800 Subject: [PATCH 06/14] fix fail test --- .../apache/kafka/server/DefaultAutoTopicCreationManagerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java index 82822339d992d..411c69ac5c161 100644 --- a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java +++ b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java @@ -100,6 +100,7 @@ public void setup() { props.setProperty(KRaftConfigs.NODE_ID_CONFIG, "1"); props.setProperty(KRaftConfigs.PROCESS_ROLES_CONFIG, "broker"); props.setProperty(ServerConfigs.REQUEST_TIMEOUT_MS_CONFIG, String.valueOf(requestTimeout)); + props.setProperty(KRaftConfigs.CONTROLLER_LISTENER_NAMES_CONFIG, "CONTROLLER"); props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, String.valueOf(internalTopicPartitions)); props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_REPLICATION_FACTOR_CONFIG, String.valueOf(internalTopicPartitions)); From 6e834254a5cd7d607193c7f138abc7a3c32f3d43 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Tue, 23 Sep 2025 23:45:09 +0800 Subject: [PATCH 07/14] update --- .../server/AutoTopicCreationManager.scala | 449 -------------- .../scala/kafka/server/BrokerServer.scala | 4 +- .../main/scala/kafka/server/KafkaApis.scala | 10 +- .../server/AutoTopicCreationManagerTest.scala | 587 ------------------ .../kafka/server/ExpiringErrorCacheTest.scala | 400 ------------ .../unit/kafka/server/KafkaApisTest.scala | 6 +- .../server/AutoTopicCreationManager.java | 27 +- .../DefaultAutoTopicCreationManager.java | 116 +++- .../kafka/server/ExpiringErrorCache.java | 96 +++ .../DefaultAutoTopicCreationManagerTest.java | 263 +++++++- .../kafka/server/ExpiringErrorCacheTest.java | 412 ++++++++++++ 11 files changed, 911 insertions(+), 1459 deletions(-) delete mode 100644 core/src/main/scala/kafka/server/AutoTopicCreationManager.scala delete mode 100644 core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala delete mode 100644 core/src/test/scala/unit/kafka/server/ExpiringErrorCacheTest.scala create mode 100644 server/src/main/java/org/apache/kafka/server/ExpiringErrorCache.java create mode 100644 server/src/test/java/org/apache/kafka/server/ExpiringErrorCacheTest.java diff --git a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala b/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala deleted file mode 100644 index 5da400e29de94..0000000000000 --- a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala +++ /dev/null @@ -1,449 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.locks.ReentrantLock -import java.util.{Collections, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.Logging -import org.apache.kafka.clients.ClientResponse -import org.apache.kafka.common.errors.InvalidTopicException -import org.apache.kafka.common.internals.Topic -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.CreateTopicsRequestData -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.protocol.{ApiKeys, Errors} -import org.apache.kafka.common.requests.{CreateTopicsRequest, CreateTopicsResponse, RequestContext, RequestHeader} -import org.apache.kafka.coordinator.group.GroupCoordinator -import org.apache.kafka.coordinator.share.ShareCoordinator -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.common.{ControllerRequestCompletionHandler, NodeToControllerChannelManager} -import org.apache.kafka.server.quota.ControllerMutationQuota -import org.apache.kafka.common.utils.Time - -import scala.collection.{Map, Seq, Set, mutable} -import scala.jdk.CollectionConverters._ -import scala.jdk.OptionConverters.RichOptional - -trait AutoTopicCreationManager { - - def createTopics( - topicNames: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] - - def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext, - timeoutMs: Long - ): Unit - - def getStreamsInternalTopicCreationErrors( - topicNames: Set[String], - currentTimeMs: Long - ): Map[String, String] - - def close(): Unit = {} - -} - -/** - * Thread-safe cache that stores topic creation errors with per-entry expiration. - * - Expiration: maintained by a min-heap (priority queue) on expiration time - * - Capacity: enforced by evicting entries with earliest expiration time (not LRU) - * - Updates: old entries remain in queue but are ignored via reference equality check - */ -private[server] class ExpiringErrorCache(maxSize: Int, time: Time) { - - private case class Entry(topicName: String, errorMessage: String, expirationTimeMs: Long) - - private val byTopic = new ConcurrentHashMap[String, Entry]() - private val expiryQueue = new java.util.PriorityQueue[Entry](11, new java.util.Comparator[Entry] { - override def compare(a: Entry, b: Entry): Int = java.lang.Long.compare(a.expirationTimeMs, b.expirationTimeMs) - }) - private val lock = new ReentrantLock() - - def put(topicName: String, errorMessage: String, ttlMs: Long): Unit = { - lock.lock() - try { - val currentTimeMs = time.milliseconds() - val expirationTimeMs = currentTimeMs + ttlMs - val entry = Entry(topicName, errorMessage, expirationTimeMs) - byTopic.put(topicName, entry) - expiryQueue.add(entry) - - // Clean up expired entries and enforce capacity - while (!expiryQueue.isEmpty && - (expiryQueue.peek().expirationTimeMs <= currentTimeMs || byTopic.size() > maxSize)) { - val evicted = expiryQueue.poll() - val current = byTopic.get(evicted.topicName) - if (current != null && (current eq evicted)) { - byTopic.remove(evicted.topicName) - } - } - } finally { - lock.unlock() - } - } - - def getErrorsForTopics(topicNames: Set[String], currentTimeMs: Long): Map[String, String] = { - val result = mutable.Map.empty[String, String] - topicNames.foreach { topicName => - val entry = byTopic.get(topicName) - if (entry != null && entry.expirationTimeMs > currentTimeMs) { - result.put(topicName, entry.errorMessage) - } - } - result.toMap - } - - private[server] def clear(): Unit = { - lock.lock() - try { - byTopic.clear() - expiryQueue.clear() - } finally { - lock.unlock() - } - } -} - - -class DefaultAutoTopicCreationManager( - config: KafkaConfig, - channelManager: NodeToControllerChannelManager, - groupCoordinator: GroupCoordinator, - txnCoordinator: TransactionCoordinator, - shareCoordinator: ShareCoordinator, - time: Time, - topicErrorCacheCapacity: Int = 1000 -) extends AutoTopicCreationManager with Logging { - - private val inflightTopics = Collections.newSetFromMap(new ConcurrentHashMap[String, java.lang.Boolean]()) - - // Hardcoded default capacity; can be overridden in tests via constructor param - private val topicCreationErrorCache = new ExpiringErrorCache(topicErrorCacheCapacity, time) - - /** - * Initiate auto topic creation for the given topics. - * - * @param topics the topics to create - * @param controllerMutationQuota the controller mutation quota for topic creation - * @param metadataRequestContext defined when creating topics on behalf of the client. The goal here is to preserve - * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest - * inside Envelope to send to the controller when forwarding is enabled. - * @return auto created topic metadata responses - */ - override def createTopics( - topics: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val (creatableTopics, uncreatableTopicResponses) = filterCreatableTopics(topics) - - val creatableTopicResponses = if (creatableTopics.isEmpty) { - Seq.empty - } else { - sendCreateTopicRequest(creatableTopics, metadataRequestContext) - } - - uncreatableTopicResponses ++ creatableTopicResponses - } - - override def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext, - timeoutMs: Long - ): Unit = { - if (topics.nonEmpty) { - sendCreateTopicRequestWithErrorCaching(topics, Some(requestContext), timeoutMs) - } - } - - override def getStreamsInternalTopicCreationErrors( - topicNames: Set[String], - currentTimeMs: Long - ): Map[String, String] = { - topicCreationErrorCache.getErrorsForTopics(topicNames, currentTimeMs) - } - - private def sendCreateTopicRequest( - creatableTopics: Map[String, CreatableTopic], - requestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val topicsToCreate = new CreateTopicsRequestData.CreatableTopicCollection(creatableTopics.size) - topicsToCreate.addAll(creatableTopics.values.asJavaCollection) - - val createTopicsRequest = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTimeoutMs(config.requestTimeoutMs) - .setTopics(topicsToCreate) - ) - - val requestCompletionHandler = new ControllerRequestCompletionHandler { - override def onTimeout(): Unit = { - clearInflightRequests(creatableTopics) - debug(s"Auto topic creation timed out for ${creatableTopics.keys}.") - } - - override def onComplete(response: ClientResponse): Unit = { - clearInflightRequests(creatableTopics) - if (response.authenticationException() != null) { - warn(s"Auto topic creation failed for ${creatableTopics.keys} with authentication exception") - } else if (response.versionMismatch() != null) { - warn(s"Auto topic creation failed for ${creatableTopics.keys} with invalid version exception") - } else { - if (response.hasResponse) { - response.responseBody() match { - case createTopicsResponse: CreateTopicsResponse => - createTopicsResponse.data().topics().forEach(topicResult => { - val error = Errors.forCode(topicResult.errorCode) - if (error != Errors.NONE) { - warn(s"Auto topic creation failed for ${topicResult.name} with error '${error.name}': ${topicResult.errorMessage}") - } - }) - case other => - warn(s"Auto topic creation request received unexpected response type: ${other.getClass.getSimpleName}") - } - } - debug(s"Auto topic creation completed for ${creatableTopics.keys} with response ${response.responseBody}.") - } - } - } - - val request = requestContext.map { context => - val requestVersion = - channelManager.controllerApiVersions.toScala match { - case None => - // We will rely on the Metadata request to be retried in the case - // that the latest version is not usable by the controller. - ApiKeys.CREATE_TOPICS.latestVersion() - case Some(nodeApiVersions) => - nodeApiVersions.latestUsableVersion(ApiKeys.CREATE_TOPICS) - } - - // Borrow client information such as client id and correlation id from the original request, - // in order to correlate the create request with the original metadata request. - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, - requestVersion, - context.clientId, - context.correlationId) - ForwardingManager.buildEnvelopeRequest(context, - createTopicsRequest.build(requestVersion).serializeWithHeader(requestHeader)) - }.getOrElse(createTopicsRequest) - - channelManager.sendRequest(request, requestCompletionHandler) - - val creatableTopicResponses = creatableTopics.keySet.toSeq.map { topic => - new MetadataResponseTopic() - .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - } - - info(s"Sent auto-creation request for ${creatableTopics.keys} to the active controller.") - creatableTopicResponses - } - - private def clearInflightRequests(creatableTopics: Map[String, CreatableTopic]): Unit = { - creatableTopics.keySet.foreach(inflightTopics.remove) - debug(s"Cleared inflight topic creation state for $creatableTopics") - } - - private def creatableTopic(topic: String): CreatableTopic = { - topic match { - case GROUP_METADATA_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.groupCoordinatorConfig.offsetsTopicPartitions) - .setReplicationFactor(config.groupCoordinatorConfig.offsetsTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections(groupCoordinator.groupMetadataTopicConfigs)) - case TRANSACTION_STATE_TOPIC_NAME => - val transactionLogConfig = new TransactionLogConfig(config) - new CreatableTopic() - .setName(topic) - .setNumPartitions(transactionLogConfig.transactionTopicPartitions) - .setReplicationFactor(transactionLogConfig.transactionTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections( - txnCoordinator.transactionTopicConfigs)) - case SHARE_GROUP_STATE_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.shareCoordinatorConfig.shareCoordinatorStateTopicNumPartitions()) - .setReplicationFactor(config.shareCoordinatorConfig.shareCoordinatorStateTopicReplicationFactor()) - .setConfigs(convertToTopicConfigCollections(shareCoordinator.shareGroupStateTopicConfigs())) - case topicName => - new CreatableTopic() - .setName(topicName) - .setNumPartitions(config.numPartitions) - .setReplicationFactor(config.defaultReplicationFactor.shortValue) - } - } - - private def convertToTopicConfigCollections(config: Properties): CreatableTopicConfigCollection = { - val topicConfigs = new CreatableTopicConfigCollection() - config.forEach { - case (name, value) => - topicConfigs.add(new CreatableTopicConfig() - .setName(name.toString) - .setValue(value.toString)) - } - topicConfigs - } - - private def isValidTopicName(topic: String): Boolean = { - try { - Topic.validate(topic) - true - } catch { - case _: InvalidTopicException => - false - } - } - - private def filterCreatableTopics( - topics: Set[String] - ): (Map[String, CreatableTopic], Seq[MetadataResponseTopic]) = { - - val creatableTopics = mutable.Map.empty[String, CreatableTopic] - val uncreatableTopics = mutable.Buffer.empty[MetadataResponseTopic] - - topics.foreach { topic => - // Attempt basic topic validation before sending any requests to the controller. - val validationError: Option[Errors] = if (!isValidTopicName(topic)) { - Some(Errors.INVALID_TOPIC_EXCEPTION) - } else if (!inflightTopics.add(topic)) { - Some(Errors.UNKNOWN_TOPIC_OR_PARTITION) - } else { - None - } - - validationError match { - case Some(error) => - uncreatableTopics += new MetadataResponseTopic() - .setErrorCode(error.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - case None => - creatableTopics.put(topic, creatableTopic(topic)) - } - } - - (creatableTopics, uncreatableTopics) - } - - private def sendCreateTopicRequestWithErrorCaching( - creatableTopics: Map[String, CreatableTopic], - requestContext: Option[RequestContext], - timeoutMs: Long - ): Seq[MetadataResponseTopic] = { - val topicsToCreate = new CreateTopicsRequestData.CreatableTopicCollection(creatableTopics.size) - topicsToCreate.addAll(creatableTopics.values.asJavaCollection) - - val createTopicsRequest = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTimeoutMs(config.requestTimeoutMs) - .setTopics(topicsToCreate) - ) - - val requestCompletionHandler = new ControllerRequestCompletionHandler { - override def onTimeout(): Unit = { - clearInflightRequests(creatableTopics) - debug(s"Auto topic creation timed out for ${creatableTopics.keys}.") - cacheTopicCreationErrors(creatableTopics.keys.toSet, "Auto topic creation timed out.", timeoutMs) - } - - override def onComplete(response: ClientResponse): Unit = { - clearInflightRequests(creatableTopics) - if (response.authenticationException() != null) { - val authException = response.authenticationException() - warn(s"Auto topic creation failed for ${creatableTopics.keys} with authentication exception: ${authException.getMessage}") - cacheTopicCreationErrors(creatableTopics.keys.toSet, authException.getMessage, timeoutMs) - } else if (response.versionMismatch() != null) { - val versionException = response.versionMismatch() - warn(s"Auto topic creation failed for ${creatableTopics.keys} with version mismatch exception: ${versionException.getMessage}") - cacheTopicCreationErrors(creatableTopics.keys.toSet, versionException.getMessage, timeoutMs) - } else { - response.responseBody() match { - case createTopicsResponse: CreateTopicsResponse => - cacheTopicCreationErrorsFromResponse(createTopicsResponse, timeoutMs) - case _ => - debug(s"Auto topic creation completed for ${creatableTopics.keys} with response ${response.responseBody}.") - } - } - } - } - - val request = requestContext.map { context => - val requestVersion = - channelManager.controllerApiVersions.toScala match { - case None => - // We will rely on the Metadata request to be retried in the case - // that the latest version is not usable by the controller. - ApiKeys.CREATE_TOPICS.latestVersion() - case Some(nodeApiVersions) => - nodeApiVersions.latestUsableVersion(ApiKeys.CREATE_TOPICS) - } - - // Borrow client information such as client id and correlation id from the original request, - // in order to correlate the create request with the original metadata request. - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, - requestVersion, - context.clientId, - context.correlationId) - ForwardingManager.buildEnvelopeRequest(context, - createTopicsRequest.build(requestVersion).serializeWithHeader(requestHeader)) - }.getOrElse(createTopicsRequest) - - channelManager.sendRequest(request, requestCompletionHandler) - - val creatableTopicResponses = creatableTopics.keySet.toSeq.map { topic => - new MetadataResponseTopic() - .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - } - - creatableTopicResponses - } - - private def cacheTopicCreationErrors(topicNames: Set[String], errorMessage: String, ttlMs: Long): Unit = { - topicNames.foreach { topicName => - topicCreationErrorCache.put(topicName, errorMessage, ttlMs) - } - } - - private def cacheTopicCreationErrorsFromResponse(response: CreateTopicsResponse, ttlMs: Long): Unit = { - response.data().topics().forEach { topicResult => - if (topicResult.errorCode() != Errors.NONE.code()) { - val errorMessage = Option(topicResult.errorMessage()) - .filter(_.nonEmpty) - .getOrElse(Errors.forCode(topicResult.errorCode()).message()) - topicCreationErrorCache.put(topicResult.name(), errorMessage, ttlMs) - debug(s"Cached topic creation error for ${topicResult.name()}: $errorMessage") - } - } - } - - override def close(): Unit = { - topicCreationErrorCache.clear() - } -} diff --git a/core/src/main/scala/kafka/server/BrokerServer.scala b/core/src/main/scala/kafka/server/BrokerServer.scala index 7e2c33d6f0891..67409ee777de8 100644 --- a/core/src/main/scala/kafka/server/BrokerServer.scala +++ b/core/src/main/scala/kafka/server/BrokerServer.scala @@ -389,7 +389,9 @@ class BrokerServer( config, clientToControllerChannelManager, () => groupCoordinator.groupMetadataTopicConfigs(), () => transactionCoordinator.transactionTopicConfigs, - () => shareCoordinator.shareGroupStateTopicConfigs() + () => shareCoordinator.shareGroupStateTopicConfigs(), + time, + DefaultAutoTopicCreationManager.DEFAULT_TOPIC_ERROR_CACHE_CAPACITY ) dynamicConfigHandlers = Map[ConfigType, ConfigHandler]( diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index 80cdb8fca2054..7f5790d9aaea6 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -2824,18 +2824,18 @@ class KafkaApis(val requestChannel: RequestChannel, val timeoutMs = heartbeatIntervalMs * 2 autoTopicCreationManager.createStreamsInternalTopics(topicsToCreate, requestContext, timeoutMs) - + // Check for cached topic creation errors only if there's already a MISSING_INTERNAL_TOPICS status val hasMissingInternalTopicsStatus = responseData.status() != null && responseData.status().stream().anyMatch(s => s.statusCode() == StreamsGroupHeartbeatResponse.Status.MISSING_INTERNAL_TOPICS.code()) - + if (hasMissingInternalTopicsStatus) { val currentTimeMs = time.milliseconds() - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(topicsToCreate.keys.toSet, currentTimeMs) - if (cachedErrors.nonEmpty) { + val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(topicsToCreate.keySet(), currentTimeMs) + if (!cachedErrors.isEmpty) { val missingInternalTopicStatus = responseData.status().stream().filter(x => x.statusCode() == StreamsGroupHeartbeatResponse.Status.MISSING_INTERNAL_TOPICS.code()).findFirst() - val creationErrorDetails = cachedErrors.map { case (topic, error) => s"$topic ($error)" }.mkString(", ") + val creationErrorDetails = cachedErrors.entrySet().stream().map(e => s"${e.getKey} (${e.getValue})" ).collect(Collectors.joining(", ")) if (missingInternalTopicStatus.isPresent) { val existingDetail = Option(missingInternalTopicStatus.get().statusDetail()).getOrElse("") missingInternalTopicStatus.get().setStatusDetail( diff --git a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala b/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala deleted file mode 100644 index cd17d7df2b3b2..0000000000000 --- a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala +++ /dev/null @@ -1,587 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.net.InetAddress -import java.nio.ByteBuffer -import java.util -import java.util.concurrent.atomic.AtomicBoolean -import java.util.{Collections, Optional, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.TestUtils -import org.apache.kafka.clients.{ClientResponse, NodeApiVersions, RequestCompletionHandler} -import org.apache.kafka.common.Node -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.{ApiVersionsResponseData, CreateTopicsRequestData} -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.network.{ClientInformation, ListenerName} -import org.apache.kafka.common.protocol.{ApiKeys, ByteBufferAccessor, Errors} -import org.apache.kafka.common.requests._ -import org.apache.kafka.common.security.auth.{KafkaPrincipal, KafkaPrincipalSerde, SecurityProtocol} -import org.apache.kafka.common.utils.{SecurityUtils, Utils} -import org.apache.kafka.server.util.MockTime -import org.apache.kafka.coordinator.group.{GroupCoordinator, GroupCoordinatorConfig} -import org.apache.kafka.coordinator.share.{ShareCoordinator, ShareCoordinatorConfig} -import org.apache.kafka.metadata.MetadataCache -import org.apache.kafka.server.config.ServerConfigs -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.common.{ControllerRequestCompletionHandler, NodeToControllerChannelManager} -import org.apache.kafka.server.quota.ControllerMutationQuota -import org.junit.jupiter.api.Assertions.{assertEquals, assertThrows, assertTrue} -import org.junit.jupiter.api.{BeforeEach, Test} -import org.mockito.ArgumentMatchers.any -import org.mockito.{ArgumentCaptor, ArgumentMatchers, Mockito} -import org.mockito.Mockito.never - -import scala.collection.{Map, Seq} - -class AutoTopicCreationManagerTest { - - private val requestTimeout = 100 - private val testCacheCapacity = 3 - private var config: KafkaConfig = _ - private val metadataCache = Mockito.mock(classOf[MetadataCache]) - private val brokerToController = Mockito.mock(classOf[NodeToControllerChannelManager]) - private val groupCoordinator = Mockito.mock(classOf[GroupCoordinator]) - private val transactionCoordinator = Mockito.mock(classOf[TransactionCoordinator]) - private val shareCoordinator = Mockito.mock(classOf[ShareCoordinator]) - private var autoTopicCreationManager: AutoTopicCreationManager = _ - private val mockTime = new MockTime(0L, 0L) - - private val internalTopicPartitions = 2 - private val internalTopicReplicationFactor: Short = 2 - - @BeforeEach - def setup(): Unit = { - val props = TestUtils.createBrokerConfig(1) - props.setProperty(ServerConfigs.REQUEST_TIMEOUT_MS_CONFIG, requestTimeout.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicPartitions.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicPartitions.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_REPLICATION_FACTOR_CONFIG , internalTopicPartitions.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_NUM_PARTITIONS_CONFIG, internalTopicReplicationFactor.toString) - // Set a short group max session timeout for testing TTL (1 second) - props.setProperty(GroupCoordinatorConfig.GROUP_MAX_SESSION_TIMEOUT_MS_CONFIG, "1000") - - config = KafkaConfig.fromProps(props) - val aliveBrokers = util.List.of(new Node(0, "host0", 0), new Node(1, "host1", 1)) - - Mockito.when(metadataCache.getAliveBrokerNodes(any(classOf[ListenerName]))).thenReturn(aliveBrokers) - } - - @Test - def testCreateOffsetTopic(): Unit = { - Mockito.when(groupCoordinator.groupMetadataTopicConfigs).thenReturn(new Properties) - testCreateTopic(GROUP_METADATA_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateTxnTopic(): Unit = { - Mockito.when(transactionCoordinator.transactionTopicConfigs).thenReturn(new Properties) - testCreateTopic(TRANSACTION_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateShareStateTopic(): Unit = { - Mockito.when(shareCoordinator.shareGroupStateTopicConfigs()).thenReturn(new Properties) - testCreateTopic(SHARE_GROUP_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateNonInternalTopic(): Unit = { - testCreateTopic("topic", isInternal = false) - } - - private def testCreateTopic(topicName: String, - isInternal: Boolean, - numPartitions: Int = 1, - replicationFactor: Short = 1): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - val topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection - topicsCollection.add(getNewTopic(topicName, numPartitions, replicationFactor)) - val requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - - // Calling twice with the same topic will only trigger one forwarding. - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - - Mockito.verify(brokerToController).sendRequest( - ArgumentMatchers.eq(requestBody), - any(classOf[ControllerRequestCompletionHandler])) - } - - @Test - def testTopicCreationWithMetadataContextPassPrincipal(): Unit = { - val topicName = "topic" - - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val serializeIsCalled = new AtomicBoolean(false) - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - assertEquals(principal, userPrincipal) - serializeIsCalled.set(true) - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - - val requestContext = initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - - assertTrue(serializeIsCalled.get()) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - assertEquals(userPrincipal, SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal))) - } - - @Test - def testTopicCreationWithMetadataContextWhenPrincipalSerdeNotDefined(): Unit = { - val topicName = "topic" - - val requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.empty()) - - // Throw upon undefined principal serde when building the forward request - assertThrows(classOf[IllegalArgumentException], () => autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext))) - } - - @Test - def testTopicCreationWithMetadataContextNoRetryUponUnsupportedVersion(): Unit = { - val topicName = "topic" - - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - - val requestContext = initializeRequestContext(KafkaPrincipal.ANONYMOUS, Optional.of(principalSerde)) - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - - // Should only trigger once - val argumentCaptor = ArgumentCaptor.forClass(classOf[ControllerRequestCompletionHandler]) - Mockito.verify(brokerToController).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Complete with unsupported version will not trigger a retry, but cleanup the inflight topics instead - val header = new RequestHeader(ApiKeys.ENVELOPE, 0, "client", 1) - val response = new EnvelopeResponse(ByteBuffer.allocate(0), Errors.UNSUPPORTED_VERSION) - val clientResponse = new ClientResponse(header, null, null, - 0, 0, false, null, null, response) - argumentCaptor.getValue.asInstanceOf[RequestCompletionHandler].onComplete(clientResponse) - Mockito.verify(brokerToController, Mockito.times(1)).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Could do the send again as inflight topics are cleared. - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - Mockito.verify(brokerToController, Mockito.times(2)).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - } - - @Test - def testCreateStreamsInternalTopics(): Unit = { - val topicConfig = new CreatableTopicConfigCollection() - topicConfig.add(new CreatableTopicConfig().setName("cleanup.policy").setValue("compact")) - - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(3).setReplicationFactor(2).setConfigs(topicConfig), - "stream-topic-2" -> new CreatableTopic().setName("stream-topic-2").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - - val requestHeader = new RequestHeader(ApiKeys.CREATE_TOPICS, ApiKeys.CREATE_TOPICS.latestVersion(), "clientId", 0) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - val topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection - topicsCollection.add(getNewTopic("stream-topic-1", 3, 2.toShort).setConfigs(topicConfig)) - topicsCollection.add(getNewTopic("stream-topic-2", 1, 1.toShort)) - val requestBody = new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTopics(topicsCollection) - .setTimeoutMs(requestTimeout)) - .build(ApiKeys.CREATE_TOPICS.latestVersion()) - - val forwardedRequestBuffer = capturedRequest.requestData().duplicate() - assertEquals(requestHeader, RequestHeader.parse(forwardedRequestBuffer)) - assertEquals(requestBody.data(), CreateTopicsRequest.parse(new ByteBufferAccessor(forwardedRequestBuffer), - ApiKeys.CREATE_TOPICS.latestVersion()).data()) - } - - @Test - def testCreateStreamsInternalTopicsWithEmptyTopics(): Unit = { - val topics = Map.empty[String, CreatableTopic] - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - Mockito.verify(brokerToController, never()).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - any(classOf[ControllerRequestCompletionHandler])) - } - - @Test - def testCreateStreamsInternalTopicsPassesPrincipal(): Unit = { - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(-1).setReplicationFactor(-1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]) - Mockito.verify(brokerToController).sendRequest( - argumentCaptor.capture(), - any(classOf[ControllerRequestCompletionHandler])) - val capturedRequest = argumentCaptor.getValue.asInstanceOf[EnvelopeRequest.Builder].build(ApiKeys.ENVELOPE.latestVersion()) - assertEquals(new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user"), SecurityUtils.parseKafkaPrincipal(Utils.utf8(capturedRequest.requestPrincipal))) - } - - private def initializeRequestContextWithUserPrincipal(): RequestContext = { - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - } - - private def initializeRequestContext(kafkaPrincipal: KafkaPrincipal, - principalSerde: Optional[KafkaPrincipalSerde]): RequestContext = { - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - val createTopicApiVersion = new ApiVersionsResponseData.ApiVersion() - .setApiKey(ApiKeys.CREATE_TOPICS.id) - .setMinVersion(ApiKeys.CREATE_TOPICS.oldestVersion()) - .setMaxVersion(ApiKeys.CREATE_TOPICS.latestVersion()) - Mockito.when(brokerToController.controllerApiVersions()) - .thenReturn(Optional.of(NodeApiVersions.create(Collections.singleton(createTopicApiVersion)))) - - val requestHeader = new RequestHeader(ApiKeys.METADATA, ApiKeys.METADATA.latestVersion, - "clientId", 0) - new RequestContext(requestHeader, "1", InetAddress.getLocalHost, Optional.empty(), - kafkaPrincipal, ListenerName.forSecurityProtocol(SecurityProtocol.PLAINTEXT), - SecurityProtocol.PLAINTEXT, ClientInformation.EMPTY, false, principalSerde) - } - - private def createTopicAndVerifyResult(error: Errors, - topicName: String, - isInternal: Boolean, - metadataContext: Option[RequestContext] = None): Unit = { - val topicResponses = autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, metadataContext) - - val expectedResponses = Seq(new MetadataResponseTopic() - .setErrorCode(error.code()) - .setIsInternal(isInternal) - .setName(topicName)) - - assertEquals(expectedResponses, topicResponses) - } - - private def getNewTopic(topicName: String, numPartitions: Int, replicationFactor: Short): CreatableTopic = { - new CreatableTopic() - .setName(topicName) - .setNumPartitions(numPartitions) - .setReplicationFactor(replicationFactor) - } - - @Test - def testTopicCreationErrorCaching(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - val topics = Map( - "test-topic-1" -> new CreatableTopic().setName("test-topic-1").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[ControllerRequestCompletionHandler]) - Mockito.verify(brokerToController).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Simulate a CreateTopicsResponse with errors - val createTopicsResponseData = new org.apache.kafka.common.message.CreateTopicsResponseData() - val topicResult = new org.apache.kafka.common.message.CreateTopicsResponseData.CreatableTopicResult() - .setName("test-topic-1") - .setErrorCode(Errors.TOPIC_ALREADY_EXISTS.code()) - .setErrorMessage("Topic 'test-topic-1' already exists.") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - val header = new RequestHeader(ApiKeys.CREATE_TOPICS, 0, "client", 1) - val clientResponse = new ClientResponse(header, null, null, - 0, 0, false, null, null, createTopicsResponse) - - // Trigger the completion handler - argumentCaptor.getValue.onComplete(clientResponse) - - // Verify that the error was cached - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic-1"), mockTime.milliseconds()) - assertEquals(1, cachedErrors.size) - assertTrue(cachedErrors.contains("test-topic-1")) - assertEquals("Topic 'test-topic-1' already exists.", cachedErrors("test-topic-1")) - } - - @Test - def testGetTopicCreationErrorsWithMultipleTopics(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - val topics = Map( - "success-topic" -> new CreatableTopic().setName("success-topic").setNumPartitions(1).setReplicationFactor(1), - "failed-topic" -> new CreatableTopic().setName("failed-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[ControllerRequestCompletionHandler]) - Mockito.verify(brokerToController).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Simulate mixed response - one success, one failure - val createTopicsResponseData = new org.apache.kafka.common.message.CreateTopicsResponseData() - createTopicsResponseData.topics().add( - new org.apache.kafka.common.message.CreateTopicsResponseData.CreatableTopicResult() - .setName("success-topic") - .setErrorCode(Errors.NONE.code()) - ) - createTopicsResponseData.topics().add( - new org.apache.kafka.common.message.CreateTopicsResponseData.CreatableTopicResult() - .setName("failed-topic") - .setErrorCode(Errors.POLICY_VIOLATION.code()) - .setErrorMessage("Policy violation") - ) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - val header = new RequestHeader(ApiKeys.CREATE_TOPICS, 0, "client", 1) - val clientResponse = new ClientResponse(header, null, null, - 0, 0, false, null, null, createTopicsResponse) - - argumentCaptor.getValue.onComplete(clientResponse) - - // Only the failed topic should be cached - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("success-topic", "failed-topic", "nonexistent-topic"), mockTime.milliseconds()) - assertEquals(1, cachedErrors.size) - assertTrue(cachedErrors.contains("failed-topic")) - assertEquals("Policy violation", cachedErrors("failed-topic")) - } - - @Test - def testErrorCacheTTL(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = testCacheCapacity) - - - // First cache an error by simulating topic creation failure - val topics = Map( - "test-topic" -> new CreatableTopic().setName("test-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - val shortTtlMs = 1000L // Use 1 second TTL for faster testing - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, shortTtlMs) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[ControllerRequestCompletionHandler]) - Mockito.verify(brokerToController).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Simulate a CreateTopicsResponse with error - val createTopicsResponseData = new org.apache.kafka.common.message.CreateTopicsResponseData() - val topicResult = new org.apache.kafka.common.message.CreateTopicsResponseData.CreatableTopicResult() - .setName("test-topic") - .setErrorCode(Errors.INVALID_REPLICATION_FACTOR.code()) - .setErrorMessage("Invalid replication factor") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - val header = new RequestHeader(ApiKeys.CREATE_TOPICS, 0, "client", 1) - val clientResponse = new ClientResponse(header, null, null, - 0, 0, false, null, null, createTopicsResponse) - - // Cache the error at T0 - argumentCaptor.getValue.onComplete(clientResponse) - - // Verify error is cached and accessible within TTL - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic"), mockTime.milliseconds()) - assertEquals(1, cachedErrors.size) - assertEquals("Invalid replication factor", cachedErrors("test-topic")) - - // Advance time beyond TTL - mockTime.sleep(shortTtlMs + 100) // T0 + 1.1 seconds - - // Verify error is now expired and proactively cleaned up - val expiredErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic"), mockTime.milliseconds()) - assertTrue(expiredErrors.isEmpty, "Expired errors should be proactively cleaned up") - } - - @Test - def testErrorCacheExpirationBasedEviction(): Unit = { - // Create manager with small cache size for testing - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - brokerToController, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicErrorCacheCapacity = 3) - - val requestContext = initializeRequestContextWithUserPrincipal() - - // Create 5 topics to exceed the cache size of 3 - val topicNames = (1 to 5).map(i => s"test-topic-$i") - - // Add errors for all 5 topics to the cache - topicNames.zipWithIndex.foreach { case (topicName, idx) => - val topics = Map( - topicName -> new CreatableTopic().setName(topicName).setNumPartitions(1).setReplicationFactor(1) - ) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - val argumentCaptor = ArgumentCaptor.forClass(classOf[ControllerRequestCompletionHandler]) - Mockito.verify(brokerToController, Mockito.atLeastOnce()).sendRequest( - any(classOf[AbstractRequest.Builder[_ <: AbstractRequest]]), - argumentCaptor.capture()) - - // Simulate error response for this topic - val createTopicsResponseData = new org.apache.kafka.common.message.CreateTopicsResponseData() - val topicResult = new org.apache.kafka.common.message.CreateTopicsResponseData.CreatableTopicResult() - .setName(topicName) - .setErrorCode(Errors.TOPIC_ALREADY_EXISTS.code()) - .setErrorMessage(s"Topic '$topicName' already exists.") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - val header = new RequestHeader(ApiKeys.CREATE_TOPICS, 0, "client", 1) - val clientResponse = new ClientResponse(header, null, null, - 0, 0, false, null, null, createTopicsResponse) - - argumentCaptor.getValue.onComplete(clientResponse) - - // Advance time slightly between additions to ensure different timestamps - mockTime.sleep(10) - - } - - // With cache size of 3, topics 1 and 2 should have been evicted - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(topicNames.toSet, mockTime.milliseconds()) - - // Only the last 3 topics should be in the cache (topics 3, 4, 5) - assertEquals(3, cachedErrors.size, "Cache should contain only the most recent 3 entries") - assertTrue(cachedErrors.contains("test-topic-3"), "test-topic-3 should be in cache") - assertTrue(cachedErrors.contains("test-topic-4"), "test-topic-4 should be in cache") - assertTrue(cachedErrors.contains("test-topic-5"), "test-topic-5 should be in cache") - assertTrue(!cachedErrors.contains("test-topic-1"), "test-topic-1 should have been evicted") - assertTrue(!cachedErrors.contains("test-topic-2"), "test-topic-2 should have been evicted") - } -} diff --git a/core/src/test/scala/unit/kafka/server/ExpiringErrorCacheTest.scala b/core/src/test/scala/unit/kafka/server/ExpiringErrorCacheTest.scala deleted file mode 100644 index be02f95374cb3..0000000000000 --- a/core/src/test/scala/unit/kafka/server/ExpiringErrorCacheTest.scala +++ /dev/null @@ -1,400 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import org.apache.kafka.server.util.MockTime -import org.junit.jupiter.api.Assertions._ -import org.junit.jupiter.api.{BeforeEach, Test} -import scala.concurrent.Future -import scala.concurrent.ExecutionContext.Implicits.global -import scala.util.Random -import java.util.concurrent.{CountDownLatch, TimeUnit} - -class ExpiringErrorCacheTest { - - private var mockTime: MockTime = _ - private var cache: ExpiringErrorCache = _ - - @BeforeEach - def setUp(): Unit = { - mockTime = new MockTime() - } - - // Basic Functionality Tests - - @Test - def testPutAndGet(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 1000L) - cache.put("topic2", "error2", 2000L) - - val errors = cache.getErrorsForTopics(Set("topic1", "topic2"), mockTime.milliseconds()) - assertEquals(2, errors.size) - assertEquals("error1", errors("topic1")) - assertEquals("error2", errors("topic2")) - } - - @Test - def testGetNonExistentTopic(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 1000L) - - val errors = cache.getErrorsForTopics(Set("topic1", "topic2"), mockTime.milliseconds()) - assertEquals(1, errors.size) - assertEquals("error1", errors("topic1")) - assertFalse(errors.contains("topic2")) - } - - @Test - def testUpdateExistingEntry(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 1000L) - assertEquals("error1", cache.getErrorsForTopics(Set("topic1"), mockTime.milliseconds())("topic1")) - - // Update with new error - cache.put("topic1", "error2", 2000L) - assertEquals("error2", cache.getErrorsForTopics(Set("topic1"), mockTime.milliseconds())("topic1")) - } - - @Test - def testGetMultipleTopics(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 1000L) - cache.put("topic2", "error2", 1000L) - cache.put("topic3", "error3", 1000L) - - val errors = cache.getErrorsForTopics(Set("topic1", "topic3", "topic4"), mockTime.milliseconds()) - assertEquals(2, errors.size) - assertEquals("error1", errors("topic1")) - assertEquals("error3", errors("topic3")) - assertFalse(errors.contains("topic2")) - assertFalse(errors.contains("topic4")) - } - - // Expiration Tests - - @Test - def testExpiredEntryNotReturned(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 1000L) - - // Entry should be available before expiration - assertEquals(1, cache.getErrorsForTopics(Set("topic1"), mockTime.milliseconds()).size) - - // Advance time past expiration - mockTime.sleep(1001L) - - // Entry should not be returned after expiration - assertTrue(cache.getErrorsForTopics(Set("topic1"), mockTime.milliseconds()).isEmpty) - } - - @Test - def testExpiredEntriesCleanedOnPut(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - // Add entries with different TTLs - cache.put("topic1", "error1", 1000L) - cache.put("topic2", "error2", 2000L) - - // Advance time to expire topic1 but not topic2 - mockTime.sleep(1500L) - - // Add a new entry - this should trigger cleanup - cache.put("topic3", "error3", 1000L) - - // Verify only non-expired entries remain - val errors = cache.getErrorsForTopics(Set("topic1", "topic2", "topic3"), mockTime.milliseconds()) - assertEquals(2, errors.size) - assertFalse(errors.contains("topic1")) - assertEquals("error2", errors("topic2")) - assertEquals("error3", errors("topic3")) - } - - @Test - def testMixedExpiredAndValidEntries(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 500L) - cache.put("topic2", "error2", 1000L) - cache.put("topic3", "error3", 1500L) - - // Advance time to expire only topic1 - mockTime.sleep(600L) - - val errors = cache.getErrorsForTopics(Set("topic1", "topic2", "topic3"), mockTime.milliseconds()) - assertEquals(2, errors.size) - assertFalse(errors.contains("topic1")) - assertTrue(errors.contains("topic2")) - assertTrue(errors.contains("topic3")) - } - - // Capacity Enforcement Tests - - @Test - def testCapacityEnforcement(): Unit = { - cache = new ExpiringErrorCache(3, mockTime) - - // Add 5 entries, exceeding capacity of 3 - for (i <- 1 to 5) { - cache.put(s"topic$i", s"error$i", 1000L) - // Small time advance between entries to ensure different insertion order - mockTime.sleep(10L) - } - - val errors = cache.getErrorsForTopics((1 to 5).map(i => s"topic$i").toSet, mockTime.milliseconds()) - assertEquals(3, errors.size) - - // The cache evicts by earliest expiration time - // Since all have same TTL, earliest inserted (topic1, topic2) should be evicted - assertFalse(errors.contains("topic1")) - assertFalse(errors.contains("topic2")) - assertTrue(errors.contains("topic3")) - assertTrue(errors.contains("topic4")) - assertTrue(errors.contains("topic5")) - } - - @Test - def testEvictionOrder(): Unit = { - cache = new ExpiringErrorCache(3, mockTime) - - // Add entries with different TTLs - cache.put("topic1", "error1", 3000L) // Expires at 3000 - mockTime.sleep(100L) - cache.put("topic2", "error2", 1000L) // Expires at 1100 - mockTime.sleep(100L) - cache.put("topic3", "error3", 2000L) // Expires at 2200 - mockTime.sleep(100L) - cache.put("topic4", "error4", 500L) // Expires at 800 - - // With capacity 3, topic4 (earliest expiration) should be evicted - val errors = cache.getErrorsForTopics(Set("topic1", "topic2", "topic3", "topic4"), mockTime.milliseconds()) - assertEquals(3, errors.size) - assertTrue(errors.contains("topic1")) - assertTrue(errors.contains("topic2")) - assertTrue(errors.contains("topic3")) - assertFalse(errors.contains("topic4")) - } - - @Test - def testCapacityWithDifferentTTLs(): Unit = { - cache = new ExpiringErrorCache(2, mockTime) - - cache.put("topic1", "error1", 5000L) // Long TTL - cache.put("topic2", "error2", 100L) // Short TTL - cache.put("topic3", "error3", 3000L) // Medium TTL - - // topic2 has earliest expiration, so it should be evicted - val errors = cache.getErrorsForTopics(Set("topic1", "topic2", "topic3"), mockTime.milliseconds()) - assertEquals(2, errors.size) - assertTrue(errors.contains("topic1")) - assertFalse(errors.contains("topic2")) - assertTrue(errors.contains("topic3")) - } - - // Update and Stale Entry Tests - - @Test - def testUpdateDoesNotLeaveStaleEntries(): Unit = { - cache = new ExpiringErrorCache(3, mockTime) - - // Fill cache to capacity - cache.put("topic1", "error1", 1000L) - cache.put("topic2", "error2", 1000L) - cache.put("topic3", "error3", 1000L) - - // Update topic2 with longer TTL - cache.put("topic2", "error2_updated", 5000L) - - // Add new entry to trigger eviction - cache.put("topic4", "error4", 1000L) - - // Should evict topic1 or topic3 (earliest expiration), not the updated topic2 - val errors = cache.getErrorsForTopics(Set("topic1", "topic2", "topic3", "topic4"), mockTime.milliseconds()) - assertEquals(3, errors.size) - assertTrue(errors.contains("topic2")) - assertEquals("error2_updated", errors("topic2")) - } - - @Test - def testStaleEntriesInQueueHandledCorrectly(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - // Add and update same topic multiple times - cache.put("topic1", "error1", 1000L) - cache.put("topic1", "error2", 2000L) - cache.put("topic1", "error3", 3000L) - - // Only latest value should be returned - val errors = cache.getErrorsForTopics(Set("topic1"), mockTime.milliseconds()) - assertEquals(1, errors.size) - assertEquals("error3", errors("topic1")) - - // Advance time to expire first two entries - mockTime.sleep(2500L) - - // Force cleanup by adding new entry - cache.put("topic2", "error_new", 1000L) - - // topic1 should still be available with latest value - val errorsAfterCleanup = cache.getErrorsForTopics(Set("topic1"), mockTime.milliseconds()) - assertEquals(1, errorsAfterCleanup.size) - assertEquals("error3", errorsAfterCleanup("topic1")) - } - - // Edge Cases - - @Test - def testEmptyCache(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - val errors = cache.getErrorsForTopics(Set("topic1", "topic2"), mockTime.milliseconds()) - assertTrue(errors.isEmpty) - } - - @Test - def testSingleEntryCache(): Unit = { - cache = new ExpiringErrorCache(1, mockTime) - - cache.put("topic1", "error1", 1000L) - cache.put("topic2", "error2", 1000L) - - // Only most recent should remain - val errors = cache.getErrorsForTopics(Set("topic1", "topic2"), mockTime.milliseconds()) - assertEquals(1, errors.size) - assertFalse(errors.contains("topic1")) - assertTrue(errors.contains("topic2")) - } - - @Test - def testZeroTTL(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 0L) - - // Entry expires immediately - assertTrue(cache.getErrorsForTopics(Set("topic1"), mockTime.milliseconds()).isEmpty) - } - - @Test - def testClearOperation(): Unit = { - cache = new ExpiringErrorCache(10, mockTime) - - cache.put("topic1", "error1", 1000L) - cache.put("topic2", "error2", 1000L) - - assertEquals(2, cache.getErrorsForTopics(Set("topic1", "topic2"), mockTime.milliseconds()).size) - - cache.clear() - - assertTrue(cache.getErrorsForTopics(Set("topic1", "topic2"), mockTime.milliseconds()).isEmpty) - } - - // Concurrent Access Tests - - @Test - def testConcurrentPutOperations(): Unit = { - cache = new ExpiringErrorCache(100, mockTime) - val numThreads = 10 - val numTopicsPerThread = 20 - val latch = new CountDownLatch(numThreads) - - (1 to numThreads).foreach { threadId => - Future { - try { - for (i <- 1 to numTopicsPerThread) { - cache.put(s"topic_${threadId}_$i", s"error_${threadId}_$i", 1000L) - } - } finally { - latch.countDown() - } - } - } - - assertTrue(latch.await(5, TimeUnit.SECONDS)) - - // Verify all entries were added - val allTopics = (1 to numThreads).flatMap { threadId => - (1 to numTopicsPerThread).map(i => s"topic_${threadId}_$i") - }.toSet - - val errors = cache.getErrorsForTopics(allTopics, mockTime.milliseconds()) - assertEquals(100, errors.size) // Limited by cache capacity - } - - @Test - def testConcurrentPutAndGet(): Unit = { - cache = new ExpiringErrorCache(100, mockTime) - val numOperations = 1000 - val random = new Random() - val topics = (1 to 50).map(i => s"topic$i").toArray - - val futures = (1 to numOperations).map { _ => - Future { - if (random.nextBoolean()) { - // Put operation - val topic = topics(random.nextInt(topics.length)) - cache.put(topic, s"error_${random.nextInt()}", 1000L) - } else { - // Get operation - val topicsToGet = Set(topics(random.nextInt(topics.length))) - cache.getErrorsForTopics(topicsToGet, mockTime.milliseconds()) - } - } - } - - // Wait for all operations to complete - Future.sequence(futures).map(_ => ()) - } - - @Test - def testConcurrentUpdates(): Unit = { - cache = new ExpiringErrorCache(50, mockTime) - val numThreads = 10 - val numUpdatesPerThread = 100 - val sharedTopics = (1 to 10).map(i => s"shared_topic$i").toArray - val latch = new CountDownLatch(numThreads) - - (1 to numThreads).foreach { threadId => - Future { - try { - val random = new Random() - for (i <- 1 to numUpdatesPerThread) { - val topic = sharedTopics(random.nextInt(sharedTopics.length)) - cache.put(topic, s"error_thread${threadId}_update$i", 1000L) - } - } finally { - latch.countDown() - } - } - } - - assertTrue(latch.await(5, TimeUnit.SECONDS)) - - // Verify all shared topics have some value - val errors = cache.getErrorsForTopics(sharedTopics.toSet, mockTime.milliseconds()) - sharedTopics.foreach { topic => - assertTrue(errors.contains(topic), s"Topic $topic should have a value") - assertTrue(errors(topic).startsWith("error_thread"), s"Value should be from one of the threads") - } - } -} \ No newline at end of file diff --git a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala index 0994c82933b22..7393a4481349f 100644 --- a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala +++ b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala @@ -10963,8 +10963,8 @@ class KafkaApisTest extends Logging { // Mock AutoTopicCreationManager to return cached errors val mockAutoTopicCreationManager = mock(classOf[AutoTopicCreationManager]) - when(mockAutoTopicCreationManager.getStreamsInternalTopicCreationErrors(ArgumentMatchers.eq(Set("test-topic")), any())) - .thenReturn(Map("test-topic" -> "INVALID_REPLICATION_FACTOR")) + when(mockAutoTopicCreationManager.getStreamsInternalTopicCreationErrors(ArgumentMatchers.eq(util.Set.of("test-topic")), any())) + .thenReturn(util.Map.of("test-topic", "INVALID_REPLICATION_FACTOR")) // Mock the createStreamsInternalTopics method to do nothing (simulate topic creation attempt) doNothing().when(mockAutoTopicCreationManager).createStreamsInternalTopics(any(), any(), anyLong()) @@ -10996,7 +10996,7 @@ class KafkaApisTest extends Logging { // Verify that createStreamsInternalTopics was called verify(mockAutoTopicCreationManager).createStreamsInternalTopics(any(), any(), anyLong()) - verify(mockAutoTopicCreationManager).getStreamsInternalTopicCreationErrors(ArgumentMatchers.eq(Set("test-topic")), any()) + verify(mockAutoTopicCreationManager).getStreamsInternalTopicCreationErrors(ArgumentMatchers.eq(util.Set.of("test-topic")), any()) } @ParameterizedTest diff --git a/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java index ea35b2481efc1..e3cace2e2de64 100644 --- a/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java @@ -58,6 +58,31 @@ default List createInternalTopics(Set topics) { * @param metadataRequestContext defined when creating topics on behalf of the client. The goal here is to preserve * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest * inside Envelope to send to the controller when forwarding is enabled. + * @param timeoutMs the time in milliseconds for which topic creation errors should be cached. This serves as the + * TTL (Time To Live) for error cache entries. If topic creation fails, the error will be cached + * for this duration to avoid repeated failed attempts and provide consistent error responses + * during streams group heartbeat requests. */ - void createStreamsInternalTopics(Map topics, RequestContext metadataRequestContext); + void createStreamsInternalTopics( + Map topics, + RequestContext metadataRequestContext, + long timeoutMs + ); + + /** + * Retrieve cached topic creation errors for the specified streams internal topics. + * This method returns error messages for topics that failed to be created and are still + * within their cache TTL period. Only non-expired error entries are returned. + * + * @param topicNames the set of topic names to check for cached errors + * @param currentTimeMs the current time in milliseconds, used to filter out expired cache entries + * @return a map of topic names to their corresponding error messages for topics that have + * cached errors and are not yet expired. Empty map if no cached errors exist for the topics. + */ + Map getStreamsInternalTopicCreationErrors(Set topicNames, long currentTimeMs); + + /** + * Close the AutoTopicCreationManager and clean up any resources. + */ + void close(); } diff --git a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java index 4c60958396e33..69e6f68b71872 100644 --- a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java @@ -32,6 +32,7 @@ import org.apache.kafka.common.requests.CreateTopicsResponse; import org.apache.kafka.common.requests.RequestContext; import org.apache.kafka.common.requests.RequestHeader; +import org.apache.kafka.common.utils.Time; import org.apache.kafka.coordinator.group.GroupCoordinatorConfig; import org.apache.kafka.coordinator.share.ShareCoordinatorConfig; import org.apache.kafka.coordinator.transaction.TransactionLogConfig; @@ -55,6 +56,7 @@ public class DefaultAutoTopicCreationManager implements AutoTopicCreationManager { + public static final int DEFAULT_TOPIC_ERROR_CACHE_CAPACITY = 1000; private static final Logger LOGGER = LoggerFactory.getLogger(DefaultAutoTopicCreationManager.class); private final AbstractKafkaConfig config; @@ -63,19 +65,23 @@ public class DefaultAutoTopicCreationManager implements AutoTopicCreationManager private final Supplier shareCoordinator; private final Supplier transactionTopicConfigsSupplier; private final Set inflightTopics = ConcurrentHashMap.newKeySet(); + private final ExpiringErrorCache topicCreationErrorCache; public DefaultAutoTopicCreationManager( AbstractKafkaConfig config, NodeToControllerChannelManager channelManager, Supplier groupCoordinatorConfigsSupplier, Supplier transactionTopicConfigsSupplier, - Supplier shareCoordinatorConfigsSupplier + Supplier shareCoordinatorConfigsSupplier, + Time time, + int topicErrorCacheCapacity ) { this.config = config; this.channelManager = channelManager; this.groupCoordinator = groupCoordinatorConfigsSupplier; this.shareCoordinator = shareCoordinatorConfigsSupplier; this.transactionTopicConfigsSupplier = transactionTopicConfigsSupplier; + this.topicCreationErrorCache = new ExpiringErrorCache(topicErrorCacheCapacity, time); } @Override @@ -108,12 +114,17 @@ public List createTopics(Set topics, Optional topics, - RequestContext requestContext + RequestContext requestContext, + long timeoutMs ) { - if (topics.isEmpty()) { - return; + if (!topics.isEmpty()) { + sendCreateTopicRequestWithErrorCaching(topics, Optional.of(requestContext), timeoutMs); } - sendCreateTopicRequest(topics, Optional.of(requestContext)); + } + + @Override + public Map getStreamsInternalTopicCreationErrors(Set topicNames, long currentTimeMs) { + return topicCreationErrorCache.getErrorsForTopics(topicNames, currentTimeMs); } private List sendCreateTopicRequest( @@ -249,4 +260,99 @@ private static boolean isValidTopicName(String topic) { return false; } } + + private List sendCreateTopicRequestWithErrorCaching( + Map creatableTopics, + Optional requestContext, + long timeoutMs + ) { + var topicsToCreate = new CreateTopicsRequestData.CreatableTopicCollection(creatableTopics.size()); + topicsToCreate.addAll(creatableTopics.values()); + + var createTopicsRequest = new CreateTopicsRequest.Builder( + new CreateTopicsRequestData() + .setTimeoutMs(config.requestTimeoutMs()) + .setTopics(topicsToCreate) + ); + + var requestCompletionHandler = new ControllerRequestCompletionHandler() { + @Override + public void onTimeout() { + clearInflightRequests(creatableTopics); + LOGGER.debug("Auto topic creation timed out for {}.", creatableTopics.keySet()); + cacheTopicCreationErrors(creatableTopics.keySet(), "Auto topic creation timed out.", timeoutMs); + } + + @Override + public void onComplete(ClientResponse response) { + clearInflightRequests(creatableTopics); + if (response.authenticationException() != null) { + var authException = response.authenticationException(); + LOGGER.warn("Auto topic creation failed for {} with authentication exception: {}", creatableTopics.keySet(), authException.getMessage()); + cacheTopicCreationErrors(creatableTopics.keySet(), authException.getMessage(), timeoutMs); + } else if (response.versionMismatch() != null) { + var versionException = response.versionMismatch(); + LOGGER.warn("Auto topic creation failed for {} with version mismatch exception: {}", creatableTopics.keySet(), versionException.getMessage()); + cacheTopicCreationErrors(creatableTopics.keySet(), versionException.getMessage(), timeoutMs); + } else { + var body = response.responseBody(); + if (body instanceof CreateTopicsResponse) { + cacheTopicCreationErrorsFromResponse((CreateTopicsResponse) body, timeoutMs); + } else { + LOGGER.debug("Auto topic creation completed for {} with response {}.", creatableTopics.keySet(), response.responseBody()); + } + } + } + }; + + var request = requestContext.>map(context -> { + short requestVersion = channelManager.controllerApiVersions() + .map(nodeApiVersions -> nodeApiVersions.latestUsableVersion(ApiKeys.CREATE_TOPICS)) + // We will rely on the Metadata request to be retried in the case + // that the latest version is not usable by the controller. + .orElseGet(ApiKeys.CREATE_TOPICS::latestVersion); + + // Borrow client information such as client id and correlation id from the original request, + // in order to correlate the create request with the original metadata request. + var requestHeader = new RequestHeader( + ApiKeys.CREATE_TOPICS, + requestVersion, + context.clientId(), + context.correlationId()); + return ForwardingManagerUtils.buildEnvelopeRequest(context, + createTopicsRequest.build(requestVersion).serializeWithHeader(requestHeader)); + }).orElse(createTopicsRequest); + + channelManager.sendRequest(request, requestCompletionHandler); + + return creatableTopics.keySet().stream() + .map(topic -> new MetadataResponseTopic() + .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) + .setName(topic) + .setIsInternal(Topic.isInternal(topic))) + .toList(); + } + + private void cacheTopicCreationErrors(Set topicNames, String errorMessage, long ttlMs) { + for (String topicName : topicNames) { + topicCreationErrorCache.put(topicName, errorMessage, ttlMs); + } + } + + private void cacheTopicCreationErrorsFromResponse(CreateTopicsResponse response, long ttlMs) { + response.data().topics().forEach(topicResult -> { + if (topicResult.errorCode() != Errors.NONE.code()) { + var errorMessage = Optional.ofNullable(topicResult.errorMessage()) + .filter(s -> !s.isEmpty()) + .orElse(Errors.forCode(topicResult.errorCode()).message()); + topicCreationErrorCache.put(topicResult.name(), errorMessage, ttlMs); + LOGGER.debug("Cached topic creation error for {}: {}", topicResult.name(), errorMessage); + } + }); + } + + @Override + public void close() { + topicCreationErrorCache.clear(); + } } diff --git a/server/src/main/java/org/apache/kafka/server/ExpiringErrorCache.java b/server/src/main/java/org/apache/kafka/server/ExpiringErrorCache.java new file mode 100644 index 0000000000000..8fd0e58a86dbc --- /dev/null +++ b/server/src/main/java/org/apache/kafka/server/ExpiringErrorCache.java @@ -0,0 +1,96 @@ +/* + * 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.server; + +import org.apache.kafka.common.utils.Time; + +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; +import java.util.PriorityQueue; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantLock; + +/** + * Thread-safe cache that stores topic creation errors with per-entry expiration. + * - Expiration: maintained by a min-heap (priority queue) on expiration time + * - Capacity: enforced by evicting entries with earliest expiration time (not LRU) + * - Updates: old entries remain in queue but are ignored via reference equality check + */ +class ExpiringErrorCache { + + private record Entry(String topicName, String errorMessage, long expirationTimeMs) { + } + + private final int maxSize; + private final Time time; + private final ConcurrentHashMap byTopic = new ConcurrentHashMap<>(); + private final PriorityQueue expiryQueue = + new PriorityQueue<>(11, Comparator.comparingLong(e -> e.expirationTimeMs)); + private final ReentrantLock lock = new ReentrantLock(); + + ExpiringErrorCache(int maxSize, Time time) { + this.maxSize = maxSize; + this.time = time; + } + + void put(String topicName, String errorMessage, long ttlMs) { + lock.lock(); + try { + var currentTimeMs = time.milliseconds(); + var expirationTimeMs = currentTimeMs + ttlMs; + var entry = new Entry(topicName, errorMessage, expirationTimeMs); + byTopic.put(topicName, entry); + expiryQueue.add(entry); + + // Clean up expired entries and enforce capacity + while (!expiryQueue.isEmpty() && + (expiryQueue.peek().expirationTimeMs <= currentTimeMs || byTopic.size() > maxSize)) { + var evicted = expiryQueue.poll(); + var current = byTopic.get(evicted.topicName); + if (current != null && current.equals(evicted)) { + byTopic.remove(evicted.topicName); + } + } + } finally { + lock.unlock(); + } + } + + Map getErrorsForTopics(Set topicNames, long currentTimeMs) { + var result = new HashMap(); + for (var topicName : topicNames) { + var entry = byTopic.get(topicName); + if (entry != null && entry.expirationTimeMs > currentTimeMs) { + result.put(topicName, entry.errorMessage); + } + } + return result; + } + + void clear() { + lock.lock(); + try { + byTopic.clear(); + expiryQueue.clear(); + } finally { + lock.unlock(); + } + } +} diff --git a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java index 411c69ac5c161..62a5b22e3a887 100644 --- a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java +++ b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java @@ -25,6 +25,7 @@ import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopic; import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopicConfig; import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopicConfigCollection; +import org.apache.kafka.common.message.CreateTopicsResponseData; import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic; import org.apache.kafka.common.network.ClientInformation; import org.apache.kafka.common.network.ListenerName; @@ -33,6 +34,7 @@ import org.apache.kafka.common.protocol.Errors; import org.apache.kafka.common.requests.AbstractRequest; import org.apache.kafka.common.requests.CreateTopicsRequest; +import org.apache.kafka.common.requests.CreateTopicsResponse; import org.apache.kafka.common.requests.EnvelopeRequest; import org.apache.kafka.common.requests.EnvelopeResponse; import org.apache.kafka.common.requests.RequestContext; @@ -51,6 +53,7 @@ import org.apache.kafka.server.config.AbstractKafkaConfig; import org.apache.kafka.server.config.KRaftConfigs; import org.apache.kafka.server.config.ServerConfigs; +import org.apache.kafka.server.util.MockTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -61,6 +64,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.nio.ByteBuffer; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -68,11 +72,13 @@ import java.util.Properties; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.IntStream; import static org.apache.kafka.common.internals.Topic.GROUP_METADATA_TOPIC_NAME; import static org.apache.kafka.common.internals.Topic.SHARE_GROUP_STATE_TOPIC_NAME; import static org.apache.kafka.common.internals.Topic.TRANSACTION_STATE_TOPIC_NAME; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -86,10 +92,12 @@ public class DefaultAutoTopicCreationManagerTest { (Class>) (Class) AbstractRequest.Builder.class; private final int requestTimeout = 100; + private final int testCacheCapacity = 3; private AbstractKafkaConfig config; private final MetadataCache metadataCache = Mockito.mock(MetadataCache.class); private final NodeToControllerChannelManager brokerToController = Mockito.mock(NodeToControllerChannelManager.class); private AutoTopicCreationManager autoTopicCreationManager; + private final MockTime mockTime = new MockTime(0L, 0L); private final int internalTopicPartitions = 2; private final short internalTopicReplicationFactor = 2; @@ -109,6 +117,8 @@ public void setup() { props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, String.valueOf(internalTopicReplicationFactor)); props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_PARTITIONS_CONFIG, String.valueOf(internalTopicReplicationFactor)); props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_NUM_PARTITIONS_CONFIG, String.valueOf(internalTopicReplicationFactor)); + // Set a short group max session timeout for testing TTL (1 second) + props.setProperty(GroupCoordinatorConfig.GROUP_MAX_SESSION_TIMEOUT_MS_CONFIG, "1000"); config = new AbstractKafkaConfig(AbstractKafkaConfig.CONFIG_DEF, props, Map.of(), false) { }; var aliveBrokers = List.of(new Node(0, "host0", 0), new Node(1, "host1", 1)); @@ -146,7 +156,9 @@ private void testCreateTopic( brokerToController, Properties::new, Properties::new, - Properties::new); + Properties::new, + mockTime, + testCacheCapacity); var topicsCollection = new CreateTopicsRequestData.CreatableTopicCollection(); topicsCollection.add(getNewTopic(topicName, numPartitions, replicationFactor)); @@ -269,9 +281,12 @@ public void testCreateStreamsInternalTopics() throws UnknownHostException { brokerToController, Properties::new, Properties::new, - Properties::new); + Properties::new, + mockTime, + testCacheCapacity); - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, + new GroupCoordinatorConfig(config).streamsGroupHeartbeatIntervalMs() * 2L); var argumentCaptor = ArgumentCaptor.forClass(ABSTRACT_REQUEST_BUILDER_CLASS); verify(brokerToController).sendRequest( @@ -305,9 +320,12 @@ public void testCreateStreamsInternalTopicsWithEmptyTopics() throws UnknownHostE brokerToController, Properties::new, Properties::new, - Properties::new); + Properties::new, + mockTime, + testCacheCapacity); - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, + new GroupCoordinatorConfig(config).streamsGroupHeartbeatIntervalMs() * 2L); verify(brokerToController, never()).sendRequest( any(ABSTRACT_REQUEST_BUILDER_CLASS), @@ -329,9 +347,12 @@ public void testCreateStreamsInternalTopicsPassesPrincipal() throws UnknownHostE brokerToController, Properties::new, Properties::new, - Properties::new); + Properties::new, + mockTime, + testCacheCapacity); - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext); + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, + new GroupCoordinatorConfig(config).streamsGroupHeartbeatIntervalMs() * 2L); var argumentCaptor = ArgumentCaptor.forClass(ABSTRACT_REQUEST_BUILDER_CLASS); @@ -369,7 +390,9 @@ private RequestContext initializeRequestContext( brokerToController, Properties::new, Properties::new, - Properties::new); + Properties::new, + mockTime, + testCacheCapacity); var createTopicApiVersion = new ApiVersionsResponseData.ApiVersion() .setApiKey(ApiKeys.CREATE_TOPICS.id) @@ -410,4 +433,228 @@ private static CreatableTopic getNewTopic( .setNumPartitions(numPartitions) .setReplicationFactor(replicationFactor); } + + @Test + public void testTopicCreationErrorCaching() throws UnknownHostException { + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + Properties::new, + Properties::new, + Properties::new, + mockTime, + testCacheCapacity); + + Map topics = new HashMap<>(); + topics.put("test-topic-1", new CreatableTopic() + .setName("test-topic-1") + .setNumPartitions(1) + .setReplicationFactor((short) 1)); + + var requestContext = initializeRequestContextWithUserPrincipal(); + + autoTopicCreationManager.createStreamsInternalTopics( + topics, + requestContext, + new GroupCoordinatorConfig(config).streamsGroupHeartbeatIntervalMs() * 2L); + + var argumentCaptor = ArgumentCaptor.forClass(ControllerRequestCompletionHandler.class); + verify(brokerToController).sendRequest( + any(ABSTRACT_REQUEST_BUILDER_CLASS), + argumentCaptor.capture()); + + // Simulate a CreateTopicsResponse with errors + var createTopicsResponseData = new CreateTopicsResponseData(); + var topicResult = new CreateTopicsResponseData.CreatableTopicResult() + .setName("test-topic-1") + .setErrorCode(Errors.TOPIC_ALREADY_EXISTS.code()) + .setErrorMessage("Topic 'test-topic-1' already exists."); + createTopicsResponseData.topics().add(topicResult); + + var createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData); + var header = new RequestHeader(ApiKeys.CREATE_TOPICS, (short) 0, "client", 1); + var clientResponse = new ClientResponse(header, null, null, + 0, 0, false, null, null, createTopicsResponse); + + // Trigger the completion handler + argumentCaptor.getValue().onComplete(clientResponse); + + // Verify that the error was cached + var cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors( + Set.of("test-topic-1"), mockTime.milliseconds()); + assertEquals(1, cachedErrors.size()); + assertTrue(cachedErrors.containsKey("test-topic-1")); + assertEquals("Topic 'test-topic-1' already exists.", cachedErrors.get("test-topic-1")); + } + + @Test + public void testGetTopicCreationErrorsWithMultipleTopics() throws UnknownHostException { + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + Properties::new, + Properties::new, + Properties::new, + mockTime, + testCacheCapacity); + + var topics = Map.of( + "success-topic", + new CreatableTopic().setName("success-topic").setNumPartitions(1).setReplicationFactor((short) 1), + "failed-topic", + new CreatableTopic().setName("failed-topic").setNumPartitions(1).setReplicationFactor((short) 1)); + + var requestContext = initializeRequestContextWithUserPrincipal(); + autoTopicCreationManager.createStreamsInternalTopics( + topics, requestContext, new GroupCoordinatorConfig(config).streamsGroupHeartbeatIntervalMs() * 2L); + + var argumentCaptor = ArgumentCaptor.forClass(ControllerRequestCompletionHandler.class); + + verify(brokerToController).sendRequest( + any(ABSTRACT_REQUEST_BUILDER_CLASS), + argumentCaptor.capture()); + + // Simulate mixed response - one success, one failure + var createTopicsResponseData = new CreateTopicsResponseData(); + createTopicsResponseData.topics().add( + new CreateTopicsResponseData.CreatableTopicResult() + .setName("success-topic") + .setErrorCode(Errors.NONE.code()) + ); + createTopicsResponseData.topics().add( + new CreateTopicsResponseData.CreatableTopicResult() + .setName("failed-topic") + .setErrorCode(Errors.POLICY_VIOLATION.code()) + .setErrorMessage("Policy violation") + ); + + var createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData); + var header = new RequestHeader(ApiKeys.CREATE_TOPICS, (short) 0, "client", 1); + var clientResponse = new ClientResponse(header, null, null, + 0, 0, false, null, null, createTopicsResponse); + + argumentCaptor.getValue().onComplete(clientResponse); + + // Only the failed topic should be cached + var cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors( + Set.of("success-topic", "failed-topic", "nonexistent-topic"), mockTime.milliseconds()); + assertEquals(1, cachedErrors.size()); + assertTrue(cachedErrors.containsKey("failed-topic")); + assertEquals("Policy violation", cachedErrors.get("failed-topic")); + } + + @Test + public void testErrorCacheTTL() throws UnknownHostException { + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + Properties::new, + Properties::new, + Properties::new, + mockTime, + testCacheCapacity); + + // First cache an error by simulating topic creation failure + var topics = Map.of("test-topic", + new CreatableTopic().setName("test-topic").setNumPartitions(1).setReplicationFactor((short) 1)); + var requestContext = initializeRequestContextWithUserPrincipal(); + long shortTtlMs = 1000L; // Use 1 second TTL for faster testing + autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, shortTtlMs); + + var argumentCaptor = ArgumentCaptor.forClass(ControllerRequestCompletionHandler.class); + verify(brokerToController).sendRequest( + any(ABSTRACT_REQUEST_BUILDER_CLASS), + argumentCaptor.capture()); + + // Simulate a CreateTopicsResponse with error + var createTopicsResponseData = new CreateTopicsResponseData(); + var topicResult = new CreateTopicsResponseData.CreatableTopicResult() + .setName("test-topic") + .setErrorCode(Errors.INVALID_REPLICATION_FACTOR.code()) + .setErrorMessage("Invalid replication factor"); + createTopicsResponseData.topics().add(topicResult); + + var createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData); + var header = new RequestHeader(ApiKeys.CREATE_TOPICS, (short) 0, "client", 1); + var clientResponse = new ClientResponse(header, null, null, + 0, 0, false, null, null, createTopicsResponse); + + // Cache the error at T0 + argumentCaptor.getValue().onComplete(clientResponse); + + // Verify error is cached and accessible within TTL + var cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors( + Set.of("test-topic"), mockTime.milliseconds()); + assertEquals(1, cachedErrors.size()); + assertEquals("Invalid replication factor", cachedErrors.get("test-topic")); + + // Advance time beyond TTL + mockTime.sleep(shortTtlMs + 100); // T0 + 1.1 seconds + + // Verify error is now expired and proactively cleaned up + var expiredErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors( + Set.of("test-topic"), mockTime.milliseconds()); + assertTrue(expiredErrors.isEmpty(), "Expired errors should be proactively cleaned up"); + } + + @Test + public void testErrorCacheExpirationBasedEviction() throws UnknownHostException { + // Create manager with small cache size for testing + autoTopicCreationManager = new DefaultAutoTopicCreationManager( + config, + brokerToController, + Properties::new, + Properties::new, + Properties::new, + mockTime, + 3); + + var requestContext = initializeRequestContextWithUserPrincipal(); + + // Create 5 topics to exceed the cache size of 3 + List topicNames = IntStream.rangeClosed(1, 5).mapToObj(i -> "test-topic-" + i).toList(); + + for (String topicName : topicNames) { + var topics = Map.of(topicName, + new CreatableTopic().setName(topicName).setNumPartitions(1).setReplicationFactor((short) 1)); + + autoTopicCreationManager.createStreamsInternalTopics( + topics, requestContext, new GroupCoordinatorConfig(config).streamsGroupHeartbeatIntervalMs() * 2L); + + var argumentCaptor = ArgumentCaptor.forClass(ControllerRequestCompletionHandler.class); + verify(brokerToController, Mockito.atLeastOnce()).sendRequest( + any(ABSTRACT_REQUEST_BUILDER_CLASS), + argumentCaptor.capture()); + + // Simulate error response for this topic + var createTopicsResponseData = new CreateTopicsResponseData(); + var topicResult = new CreateTopicsResponseData.CreatableTopicResult() + .setName(topicName) + .setErrorCode(Errors.TOPIC_ALREADY_EXISTS.code()) + .setErrorMessage("Topic '" + topicName + "' already exists."); + createTopicsResponseData.topics().add(topicResult); + + var createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData); + var header = new RequestHeader(ApiKeys.CREATE_TOPICS, (short) 0, "client", 1); + var clientResponse = new ClientResponse(header, null, null, + 0, 0, false, null, null, createTopicsResponse); + + argumentCaptor.getValue().onComplete(clientResponse); + + // Advance time slightly between additions to ensure different timestamps + mockTime.sleep(10); + } + + // With cache size of 3, topics 1 and 2 should have been evicted + var cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors( + Set.copyOf(topicNames), mockTime.milliseconds()); + + // Only the last 3 topics should be in the cache (topics 3, 4, 5) + assertEquals(3, cachedErrors.size(), "Cache should contain only the most recent 3 entries"); + assertTrue(cachedErrors.containsKey("test-topic-3"), "test-topic-3 should be in cache"); + assertTrue(cachedErrors.containsKey("test-topic-4"), "test-topic-4 should be in cache"); + assertTrue(cachedErrors.containsKey("test-topic-5"), "test-topic-5 should be in cache"); + assertFalse(cachedErrors.containsKey("test-topic-1"), "test-topic-1 should have been evicted"); + assertFalse(cachedErrors.containsKey("test-topic-2"), "test-topic-2 should have been evicted"); + } } diff --git a/server/src/test/java/org/apache/kafka/server/ExpiringErrorCacheTest.java b/server/src/test/java/org/apache/kafka/server/ExpiringErrorCacheTest.java new file mode 100644 index 0000000000000..ed4f921e819ca --- /dev/null +++ b/server/src/test/java/org/apache/kafka/server/ExpiringErrorCacheTest.java @@ -0,0 +1,412 @@ +/* + * 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.server; + +import org.apache.kafka.server.util.MockTime; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Random; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ExpiringErrorCacheTest { + + private MockTime mockTime; + private ExpiringErrorCache cache; + + @BeforeEach + void setUp() { + mockTime = new MockTime(); + } + + // Basic Functionality Tests + + @Test + void testPutAndGet() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 1000L); + cache.put("topic2", "error2", 2000L); + + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2"), mockTime.milliseconds()); + assertEquals(2, errors.size()); + assertEquals("error1", errors.get("topic1")); + assertEquals("error2", errors.get("topic2")); + } + + @Test + void testGetNonExistentTopic() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 1000L); + + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2"), mockTime.milliseconds()); + assertEquals(1, errors.size()); + assertEquals("error1", errors.get("topic1")); + assertFalse(errors.containsKey("topic2")); + } + + @Test + void testUpdateExistingEntry() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 1000L); + assertEquals("error1", cache.getErrorsForTopics(Set.of("topic1"), mockTime.milliseconds()).get("topic1")); + + // Update with new error + cache.put("topic1", "error2", 2000L); + assertEquals("error2", cache.getErrorsForTopics(Set.of("topic1"), mockTime.milliseconds()).get("topic1")); + } + + @Test + void testGetMultipleTopics() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 1000L); + cache.put("topic2", "error2", 1000L); + cache.put("topic3", "error3", 1000L); + + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic3", "topic4"), mockTime.milliseconds()); + assertEquals(2, errors.size()); + assertEquals("error1", errors.get("topic1")); + assertEquals("error3", errors.get("topic3")); + assertFalse(errors.containsKey("topic2")); + assertFalse(errors.containsKey("topic4")); + } + + // Expiration Tests + + @Test + void testExpiredEntryNotReturned() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 1000L); + + // Entry should be available before expiration + assertEquals(1, cache.getErrorsForTopics(Set.of("topic1"), mockTime.milliseconds()).size()); + + // Advance time past expiration + mockTime.sleep(1001L); + + // Entry should not be returned after expiration + assertTrue(cache.getErrorsForTopics(Set.of("topic1"), mockTime.milliseconds()).isEmpty()); + } + + @Test + void testExpiredEntriesCleanedOnPut() { + cache = new ExpiringErrorCache(10, mockTime); + + // Add entries with different TTLs + cache.put("topic1", "error1", 1000L); + cache.put("topic2", "error2", 2000L); + + // Advance time to expire topic1 but not topic2 + mockTime.sleep(1500L); + + // Add a new entry - this should trigger cleanup + cache.put("topic3", "error3", 1000L); + + // Verify only non-expired entries remain + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2", "topic3"), mockTime.milliseconds()); + assertEquals(2, errors.size()); + assertFalse(errors.containsKey("topic1")); + assertEquals("error2", errors.get("topic2")); + assertEquals("error3", errors.get("topic3")); + } + + @Test + void testMixedExpiredAndValidEntries() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 500L); + cache.put("topic2", "error2", 1000L); + cache.put("topic3", "error3", 1500L); + + // Advance time to expire only topic1 + mockTime.sleep(600L); + + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2", "topic3"), mockTime.milliseconds()); + assertEquals(2, errors.size()); + assertFalse(errors.containsKey("topic1")); + assertTrue(errors.containsKey("topic2")); + assertTrue(errors.containsKey("topic3")); + } + + // Capacity Enforcement Tests + + @Test + void testCapacityEnforcement() { + cache = new ExpiringErrorCache(3, mockTime); + + // Add 5 entries, exceeding capacity of 3 + IntStream.rangeClosed(1, 5).forEach(i -> { + cache.put("topic" + i, "error" + i, 1000L); + // Small time advance between entries to ensure different insertion order + mockTime.sleep(10L); + }); + + var errors = cache.getErrorsForTopics( + IntStream.rangeClosed(1, 5).mapToObj(i -> "topic" + i).collect(Collectors.toSet()), + mockTime.milliseconds()); + assertEquals(3, errors.size()); + + // The cache evicts by earliest expiration time + // Since all have same TTL, earliest inserted (topic1, topic2) should be evicted + assertFalse(errors.containsKey("topic1")); + assertFalse(errors.containsKey("topic2")); + assertTrue(errors.containsKey("topic3")); + assertTrue(errors.containsKey("topic4")); + assertTrue(errors.containsKey("topic5")); + } + + @Test + void testEvictionOrder() { + cache = new ExpiringErrorCache(3, mockTime); + + // Add entries with different TTLs + cache.put("topic1", "error1", 3000L); // Expires at 3000 + mockTime.sleep(100L); + cache.put("topic2", "error2", 1000L); // Expires at 1100 + mockTime.sleep(100L); + cache.put("topic3", "error3", 2000L); // Expires at 2200 + mockTime.sleep(100L); + cache.put("topic4", "error4", 500L); // Expires at 800 + + // With capacity 3, topic4 (earliest expiration) should be evicted + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2", "topic3", "topic4"), mockTime.milliseconds()); + assertEquals(3, errors.size()); + assertTrue(errors.containsKey("topic1")); + assertTrue(errors.containsKey("topic2")); + assertTrue(errors.containsKey("topic3")); + assertFalse(errors.containsKey("topic4")); + } + + @Test + void testCapacityWithDifferentTTLs() { + cache = new ExpiringErrorCache(2, mockTime); + + cache.put("topic1", "error1", 5000L); // Long TTL + cache.put("topic2", "error2", 100L); // Short TTL + cache.put("topic3", "error3", 3000L); // Medium TTL + + // topic2 has earliest expiration, so it should be evicted + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2", "topic3"), mockTime.milliseconds()); + assertEquals(2, errors.size()); + assertTrue(errors.containsKey("topic1")); + assertFalse(errors.containsKey("topic2")); + assertTrue(errors.containsKey("topic3")); + } + + // Update and Stale Entry Tests + + @Test + void testUpdateDoesNotLeaveStaleEntries() { + cache = new ExpiringErrorCache(3, mockTime); + + // Fill cache to capacity + cache.put("topic1", "error1", 1000L); + cache.put("topic2", "error2", 1000L); + cache.put("topic3", "error3", 1000L); + + // Update topic2 with longer TTL + cache.put("topic2", "error2_updated", 5000L); + + // Add new entry to trigger eviction + cache.put("topic4", "error4", 1000L); + + // Should evict topic1 or topic3 (earliest expiration), not the updated topic2 + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2", "topic3", "topic4"), mockTime.milliseconds()); + assertEquals(3, errors.size()); + assertTrue(errors.containsKey("topic2")); + assertEquals("error2_updated", errors.get("topic2")); + } + + @Test + void testStaleEntriesInQueueHandledCorrectly() { + cache = new ExpiringErrorCache(10, mockTime); + + // Add and update same topic multiple times + cache.put("topic1", "error1", 1000L); + cache.put("topic1", "error2", 2000L); + cache.put("topic1", "error3", 3000L); + + // Only latest value should be returned + var errors = cache.getErrorsForTopics(Set.of("topic1"), mockTime.milliseconds()); + assertEquals(1, errors.size()); + assertEquals("error3", errors.get("topic1")); + + // Advance time to expire first two entries + mockTime.sleep(2500L); + + // Force cleanup by adding new entry + cache.put("topic2", "error_new", 1000L); + + // topic1 should still be available with latest value + var errorsAfterCleanup = cache.getErrorsForTopics(Set.of("topic1"), mockTime.milliseconds()); + assertEquals(1, errorsAfterCleanup.size()); + assertEquals("error3", errorsAfterCleanup.get("topic1")); + } + + // Edge Cases + + @Test + void testEmptyCache() { + cache = new ExpiringErrorCache(10, mockTime); + + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2"), mockTime.milliseconds()); + assertTrue(errors.isEmpty()); + } + + @Test + void testSingleEntryCache() { + cache = new ExpiringErrorCache(1, mockTime); + + cache.put("topic1", "error1", 1000L); + cache.put("topic2", "error2", 1000L); + + // Only most recent should remain + var errors = cache.getErrorsForTopics(Set.of("topic1", "topic2"), mockTime.milliseconds()); + assertEquals(1, errors.size()); + assertFalse(errors.containsKey("topic1")); + assertTrue(errors.containsKey("topic2")); + } + + @Test + void testZeroTTL() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 0L); + + // Entry expires immediately + assertTrue(cache.getErrorsForTopics(Set.of("topic1"), mockTime.milliseconds()).isEmpty()); + } + + @Test + void testClearOperation() { + cache = new ExpiringErrorCache(10, mockTime); + + cache.put("topic1", "error1", 1000L); + cache.put("topic2", "error2", 1000L); + + assertEquals(2, cache.getErrorsForTopics(Set.of("topic1", "topic2"), mockTime.milliseconds()).size()); + + cache.clear(); + + assertTrue(cache.getErrorsForTopics(Set.of("topic1", "topic2"), mockTime.milliseconds()).isEmpty()); + } + + // Concurrent Access Tests + + @Test + void testConcurrentPutOperations() { + cache = new ExpiringErrorCache(100, mockTime); + var numThreads = 10; + var numTopicsPerThread = 20; + + var futures = new ArrayList>(); + + IntStream.rangeClosed(1, numThreads).forEach(threadId -> { + final var finalThreadId = threadId; + var future = CompletableFuture.runAsync(() -> + IntStream.rangeClosed(1, numTopicsPerThread).forEach(i -> + cache.put("topic_" + finalThreadId + "_" + i, "error_" + finalThreadId + "_" + i, 1000L)) + ); + futures.add(future); + }); + + assertDoesNotThrow(() -> + CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).get(5, TimeUnit.SECONDS)); + + // Verify all entries were added + var allTopics = new HashSet(); + IntStream.rangeClosed(1, numThreads).forEach(threadId -> + IntStream.rangeClosed(1, numTopicsPerThread).forEach(i -> allTopics.add("topic_" + threadId + "_" + i))); + + var errors = cache.getErrorsForTopics(allTopics, mockTime.milliseconds()); + assertEquals(100, errors.size()); // Limited by cache capacity + } + + @Test + void testConcurrentPutAndGet() { + cache = new ExpiringErrorCache(100, mockTime); + var numOperations = 1000; + var random = new Random(); + var topics = IntStream.rangeClosed(1, 50).mapToObj(i -> "topic" + i).toArray(String[]::new); + + var futures = new ArrayList>(); + IntStream.rangeClosed(1, numOperations).forEach(i -> { + var future = CompletableFuture.runAsync(() -> { + if (random.nextBoolean()) { + // Put operation + var topic = topics[random.nextInt(topics.length)]; + cache.put(topic, "error_" + random.nextInt(), 1000L); + } else { + // Get operation + var topicsToGet = Set.of(topics[random.nextInt(topics.length)]); + cache.getErrorsForTopics(topicsToGet, mockTime.milliseconds()); + } + }); + futures.add(future); + }); + + // Wait for all operations to complete + assertDoesNotThrow(() -> CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join()); + } + + @Test + void testConcurrentUpdates() { + cache = new ExpiringErrorCache(50, mockTime); + var numThreads = 10; + var numUpdatesPerThread = 100; + var sharedTopics = IntStream.rangeClosed(1, 10).mapToObj(i -> "shared_topic" + i).toArray(String[]::new); + + var futures = new ArrayList>(); + IntStream.rangeClosed(1, numThreads).forEach(threadId -> { + var future = CompletableFuture.runAsync(() -> { + var random = new Random(); + IntStream.rangeClosed(1, numUpdatesPerThread).forEach(i -> { + var topic = sharedTopics[random.nextInt(sharedTopics.length)]; + cache.put(topic, "error_thread" + threadId + "_update" + i, 1000L); + }); + }); + futures.add(future); + }); + + assertDoesNotThrow(() -> + CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).get(5, TimeUnit.SECONDS)); + + // Verify all shared topics have some value + var errors = cache.getErrorsForTopics(Set.of(sharedTopics), mockTime.milliseconds()); + for (var topic : sharedTopics) { + assertTrue(errors.containsKey(topic), "Topic " + topic + " should have a value"); + assertTrue(errors.get(topic).startsWith("error_thread"), "Value should be from one of the threads"); + } + } +} From cb0f67484ada8ea0e7d84b36bd6b1c750f4115f1 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Wed, 6 May 2026 00:31:29 +0800 Subject: [PATCH 08/14] fix --- core/src/main/scala/kafka/server/KafkaApis.scala | 2 +- .../org/apache/kafka/server/AutoTopicCreationManager.java | 6 +----- .../kafka/server/DefaultAutoTopicCreationManager.java | 6 +++++- .../kafka/server/DefaultAutoTopicCreationManagerTest.java | 1 - 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala index 10a3afeda6d2b..f9e7669846217 100644 --- a/core/src/main/scala/kafka/server/KafkaApis.scala +++ b/core/src/main/scala/kafka/server/KafkaApis.scala @@ -1285,7 +1285,7 @@ class KafkaApis(val requestChannel: RequestChannel, if (topicMetadata.headOption.isEmpty) { val controllerMutationQuota = quotas.controllerMutation.newPermissiveQuotaFor(request.session, request.header.clientId) - autoTopicCreationManager.createTopics(Seq(internalTopicName).toSet, controllerMutationQuota, None) + autoTopicCreationManager.createTopics(internalTopics, controllerMutationQuota, Optional.empty) (Errors.COORDINATOR_NOT_AVAILABLE, Node.noNode) } else { if (topicMetadata.head.errorCode != Errors.NONE.code) { diff --git a/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java index 2ba9a7954ca38..d12fc48a625d7 100644 --- a/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/AutoTopicCreationManager.java @@ -54,11 +54,7 @@ public interface AutoTopicCreationManager { * for this duration to avoid repeated failed attempts and provide consistent error responses * during streams group heartbeat requests. */ - void createStreamsInternalTopics( - Map topics, - RequestContext metadataRequestContext, - long timeoutMs - ); + void createStreamsInternalTopics(Map topics, RequestContext metadataRequestContext, long timeoutMs); /** * Retrieve cached topic creation errors for the specified streams internal topics. diff --git a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java index 8581cfa3ca50f..f2baf973ba97c 100644 --- a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java @@ -105,7 +105,11 @@ public DefaultAutoTopicCreationManager( } @Override - public List createTopics(Set topics, ControllerMutationQuota controllerMutationQuota, Optional metadataRequestContext) { + public List createTopics( + Set topics, + ControllerMutationQuota controllerMutationQuota, + Optional metadataRequestContext + ) { var creatableTopics = new HashMap(); var uncreatableTopicResponses = new ArrayList(); topics.forEach(topic -> { diff --git a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java index 0f1b40b2b4ab8..a445ee3169b39 100644 --- a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java +++ b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java @@ -97,7 +97,6 @@ public void setup() { props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_PARTITIONS_CONFIG, String.valueOf(internalTopicPartitions)); props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_NUM_PARTITIONS_CONFIG, String.valueOf(internalTopicPartitions)); - // Match Scala TestUtils.createBrokerConfig defaults props.setProperty(ServerLogConfigs.NUM_PARTITIONS_CONFIG, "1"); props.setProperty(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG, "1"); From 11f6ce283d81663990b731cceb3e867b6db228d6 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Wed, 6 May 2026 00:32:34 +0800 Subject: [PATCH 09/14] delete unused files --- .../server/AutoTopicCreationManager.scala | 413 ---------- .../server/AutoTopicCreationManagerTest.scala | 753 ------------------ 2 files changed, 1166 deletions(-) delete mode 100644 core/src/main/scala/kafka/server/AutoTopicCreationManager.scala delete mode 100644 core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala diff --git a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala b/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala deleted file mode 100644 index 2d81c3694bf0d..0000000000000 --- a/core/src/main/scala/kafka/server/AutoTopicCreationManager.scala +++ /dev/null @@ -1,413 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.locks.ReentrantLock -import java.util.{Collections, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.Logging -import org.apache.kafka.common.errors.{AuthenticationException, InvalidTopicException, TimeoutException, UnsupportedVersionException} -import org.apache.kafka.common.internals.Topic -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.CreateTopicsRequestData -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.protocol.Errors -import org.apache.kafka.common.requests.{CreateTopicsRequest, CreateTopicsResponse, RequestContext} -import org.apache.kafka.coordinator.group.GroupCoordinator -import org.apache.kafka.coordinator.share.ShareCoordinator -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.config.{ReplicationConfigs, ServerLogConfigs} -import org.apache.kafka.server.quota.ControllerMutationQuota -import org.apache.kafka.common.utils.Time -import org.apache.kafka.server.TopicCreator - -import scala.collection.{Map, Seq, Set, mutable} -import scala.jdk.CollectionConverters._ - -trait AutoTopicCreationManager { - - def createTopics( - topicNames: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] - - def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext, - timeoutMs: Long - ): Unit - - def getStreamsInternalTopicCreationErrors( - topicNames: Set[String], - currentTimeMs: Long - ): Map[String, String] - - def close(): Unit = {} - -} - -/** - * Thread-safe cache that stores topic creation errors with per-entry expiration. - * - Expiration: maintained by a min-heap (priority queue) on expiration time - * - Capacity: enforced by evicting entries with earliest expiration time (not LRU) - * - Updates: old entries remain in queue but are ignored via reference equality check - */ -private[server] class ExpiringErrorCache(maxSize: Int, time: Time) { - - private case class Entry(topicName: String, errorMessage: String, expirationTimeMs: Long) - - private val byTopic = new ConcurrentHashMap[String, Entry]() - private val expiryQueue = new java.util.PriorityQueue[Entry](11, new java.util.Comparator[Entry] { - override def compare(a: Entry, b: Entry): Int = java.lang.Long.compare(a.expirationTimeMs, b.expirationTimeMs) - }) - private val lock = new ReentrantLock() - - def put(topicName: String, errorMessage: String, ttlMs: Long): Unit = { - lock.lock() - try { - val currentTimeMs = time.milliseconds() - val expirationTimeMs = currentTimeMs + ttlMs - val entry = Entry(topicName, errorMessage, expirationTimeMs) - byTopic.put(topicName, entry) - expiryQueue.add(entry) - - // Clean up expired entries and enforce capacity - while (!expiryQueue.isEmpty && - (expiryQueue.peek().expirationTimeMs <= currentTimeMs || byTopic.size() > maxSize)) { - val evicted = expiryQueue.poll() - val current = byTopic.get(evicted.topicName) - if (current != null && (current eq evicted)) { - byTopic.remove(evicted.topicName) - } - } - } finally { - lock.unlock() - } - } - - def hasError(topicName: String, currentTimeMs: Long): Boolean = { - val entry = byTopic.get(topicName) - entry != null && entry.expirationTimeMs > currentTimeMs - } - - def getErrorsForTopics(topicNames: Set[String], currentTimeMs: Long): Map[String, String] = { - val result = mutable.Map.empty[String, String] - topicNames.foreach { topicName => - val entry = byTopic.get(topicName) - if (entry != null && entry.expirationTimeMs > currentTimeMs) { - result.put(topicName, entry.errorMessage) - } - } - result.toMap - } - - private[server] def clear(): Unit = { - lock.lock() - try { - byTopic.clear() - expiryQueue.clear() - } finally { - lock.unlock() - } - } -} - - -class DefaultAutoTopicCreationManager( - config: KafkaConfig, - groupCoordinator: GroupCoordinator, - txnCoordinator: TransactionCoordinator, - shareCoordinator: ShareCoordinator, - time: Time, - topicCreator: TopicCreator, - topicErrorCacheCapacity: Int = 1000 -) extends AutoTopicCreationManager with Logging { - - private val inflightTopics = Collections.newSetFromMap(new ConcurrentHashMap[String, java.lang.Boolean]()) - - // Hardcoded default capacity; can be overridden in tests via constructor param - private val topicCreationErrorCache = new ExpiringErrorCache(topicErrorCacheCapacity, time) - - /** - * Initiate auto topic creation for the given topics. - * - * @param topics the topics to create - * @param controllerMutationQuota the controller mutation quota for topic creation - * @param metadataRequestContext defined when creating topics on behalf of the client. The goal here is to preserve - * original client principal for auditing, thus needing to wrap a plain CreateTopicsRequest - * inside Envelope to send to the controller when forwarding is enabled. - * @return auto created topic metadata responses - */ - override def createTopics( - topics: Set[String], - controllerMutationQuota: ControllerMutationQuota, - metadataRequestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val (creatableTopics, uncreatableTopicResponses) = filterCreatableTopics(topics) - - val creatableTopicResponses = if (creatableTopics.isEmpty) { - Seq.empty - } else { - sendCreateTopicRequest(creatableTopics, metadataRequestContext) - } - - uncreatableTopicResponses ++ creatableTopicResponses - } - - override def createStreamsInternalTopics( - topics: Map[String, CreatableTopic], - requestContext: RequestContext, - timeoutMs: Long - ): Unit = { - if (topics.isEmpty) { - return - } - - val currentTimeMs = time.milliseconds() - - // Filter out topics that are: - // 1. Already in error cache (back-off period) - // 2. Already in-flight (concurrent request) - val topicsToCreate = topics.filter { case (topicName, _) => - !topicCreationErrorCache.hasError(topicName, currentTimeMs) && - inflightTopics.add(topicName) - } - - if (topicsToCreate.nonEmpty) { - sendCreateTopicRequestWithErrorCaching(topicsToCreate, requestContext, timeoutMs) - } - } - - override def getStreamsInternalTopicCreationErrors( - topicNames: Set[String], - currentTimeMs: Long - ): Map[String, String] = { - topicCreationErrorCache.getErrorsForTopics(topicNames, currentTimeMs) - } - - private def sendCreateTopicRequest( - creatableTopics: Map[String, CreatableTopic], - requestContext: Option[RequestContext] - ): Seq[MetadataResponseTopic] = { - val createTopicsRequest: CreateTopicsRequest.Builder = makeCreateTopicsRequestBuilder(creatableTopics) - - val responseFuture = requestContext match { - case Some(context) => topicCreator.createTopicWithPrincipal(context, createTopicsRequest) - case None => topicCreator.createTopicWithoutPrincipal(createTopicsRequest) - } - - responseFuture.whenComplete { - (response, throwable) => - clearInflightRequests(creatableTopics) - // Log any errors from the topic creation attempt - if (throwable != null) { - logError(creatableTopics, throwable) - } else if (response != null) { - response.data().topics().forEach(topicResult => { - val error = Errors.forCode(topicResult.errorCode) - if (error != Errors.NONE) { - warn(s"Auto topic creation failed for ${topicResult.name} with error '${error.name}': ${topicResult.errorMessage}") - } - }) - } else { - warn("CreateTopicsResponse future completed with null response and no exception") - } - } - - val creatableTopicResponses = creatableTopics.keySet.toSeq.map { topic => - new MetadataResponseTopic() - .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - } - - info(s"Sent auto-creation request for ${creatableTopics.keys} to the active controller.") - creatableTopicResponses - } - - private def clearInflightRequests(creatableTopics: Map[String, CreatableTopic]): Unit = { - creatableTopics.keySet.foreach(inflightTopics.remove) - debug(s"Cleared inflight topic creation state for $creatableTopics") - } - - private def creatableTopic(topic: String): CreatableTopic = { - topic match { - case GROUP_METADATA_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.groupCoordinatorConfig.offsetsTopicPartitions) - .setReplicationFactor(config.groupCoordinatorConfig.offsetsTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections(groupCoordinator.groupMetadataTopicConfigs)) - case TRANSACTION_STATE_TOPIC_NAME => - val transactionLogConfig = new TransactionLogConfig(config) - new CreatableTopic() - .setName(topic) - .setNumPartitions(transactionLogConfig.transactionTopicPartitions) - .setReplicationFactor(transactionLogConfig.transactionTopicReplicationFactor) - .setConfigs(convertToTopicConfigCollections( - txnCoordinator.transactionTopicConfigs)) - case SHARE_GROUP_STATE_TOPIC_NAME => - new CreatableTopic() - .setName(topic) - .setNumPartitions(config.shareCoordinatorConfig.shareCoordinatorStateTopicNumPartitions()) - .setReplicationFactor(config.shareCoordinatorConfig.shareCoordinatorStateTopicReplicationFactor()) - .setConfigs(convertToTopicConfigCollections(shareCoordinator.shareGroupStateTopicConfigs())) - case topicName => - val numPartitions: java.lang.Integer = - if (config.originals.containsKey(ServerLogConfigs.NUM_PARTITIONS_CONFIG)) config.numPartitions - else CreateTopicsRequest.NO_NUM_PARTITIONS - val replicationFactor: java.lang.Short = - if (config.originals.containsKey(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG)) config.defaultReplicationFactor.toShort - else CreateTopicsRequest.NO_REPLICATION_FACTOR - - new CreatableTopic() - .setName(topicName) - .setNumPartitions(numPartitions) - .setReplicationFactor(replicationFactor) - } - } - - private def convertToTopicConfigCollections(config: Properties): CreatableTopicConfigCollection = { - val topicConfigs = new CreatableTopicConfigCollection() - config.forEach { - case (name, value) => - topicConfigs.add(new CreatableTopicConfig() - .setName(name.toString) - .setValue(value.toString)) - } - topicConfigs - } - - private def isValidTopicName(topic: String): Boolean = { - try { - Topic.validate(topic) - true - } catch { - case _: InvalidTopicException => - false - } - } - - private def filterCreatableTopics( - topics: Set[String] - ): (Map[String, CreatableTopic], Seq[MetadataResponseTopic]) = { - - val creatableTopics = mutable.Map.empty[String, CreatableTopic] - val uncreatableTopics = mutable.Buffer.empty[MetadataResponseTopic] - - topics.foreach { topic => - // Attempt basic topic validation before sending any requests to the controller. - val validationError: Option[Errors] = if (!isValidTopicName(topic)) { - Some(Errors.INVALID_TOPIC_EXCEPTION) - } else if (!inflightTopics.add(topic)) { - Some(Errors.UNKNOWN_TOPIC_OR_PARTITION) - } else { - None - } - - validationError match { - case Some(error) => - uncreatableTopics += new MetadataResponseTopic() - .setErrorCode(error.code) - .setName(topic) - .setIsInternal(Topic.isInternal(topic)) - case None => - creatableTopics.put(topic, creatableTopic(topic)) - } - } - - (creatableTopics, uncreatableTopics) - } - - private def sendCreateTopicRequestWithErrorCaching( - creatableTopics: Map[String, CreatableTopic], - requestContext: RequestContext, - timeoutMs: Long - ): Unit = { - val createTopicsRequest: CreateTopicsRequest.Builder = makeCreateTopicsRequestBuilder(creatableTopics) - - val createTopicsResponseFuture = topicCreator.createTopicWithPrincipal(requestContext, createTopicsRequest) - - createTopicsResponseFuture.whenComplete { - (response, throwable) => - clearInflightRequests(creatableTopics) - // Log any errors from the topic creation attempt - if (throwable != null) { - logError(creatableTopics, throwable) - val errorMessage = Option(throwable.getMessage).getOrElse(throwable.toString) - cacheTopicCreationErrors(creatableTopics.keys.toSet, errorMessage, timeoutMs) - } else if (response != null) { - debug(s"Auto topic creation completed for ${creatableTopics.keys} with response $response.") - cacheTopicCreationErrorsFromResponse(response, timeoutMs) - } else { - val ex = new IllegalStateException("CreateTopicsResponse future completed with null response and no exception") - error(s"Auto topic creation failed for ${creatableTopics.keys} due to unexpected future completion state", ex) - cacheTopicCreationErrors(creatableTopics.keys.toSet, ex.getMessage, timeoutMs) - } - } - } - - private def logError(creatableTopics: Map[String, CreatableTopic], throwable: Throwable): Unit = { - throwable match { - case _: TimeoutException => - debug(s"Auto topic creation timed out for ${creatableTopics.keys}.") - case _: AuthenticationException => - warn(s"Auto topic creation failed for ${creatableTopics.keys} with authentication exception") - case _: UnsupportedVersionException => - warn(s"Auto topic creation failed for ${creatableTopics.keys} with invalid version exception") - case other => - warn(s"Auto topic creation failed for ${creatableTopics.keys} with exception", other) - } - } - - private def makeCreateTopicsRequestBuilder(creatableTopics: Map[String, CreatableTopic]): CreateTopicsRequest.Builder = { - val topicsToCreate = new CreateTopicsRequestData.CreatableTopicCollection(creatableTopics.size) - topicsToCreate.addAll(creatableTopics.values.asJavaCollection) - - new CreateTopicsRequest.Builder( - new CreateTopicsRequestData() - .setTimeoutMs(config.requestTimeoutMs) - .setTopics(topicsToCreate) - ) - } - - private def cacheTopicCreationErrors(topicNames: Set[String], errorMessage: String, ttlMs: Long): Unit = { - topicNames.foreach { topicName => - topicCreationErrorCache.put(topicName, errorMessage, ttlMs) - } - } - - private def cacheTopicCreationErrorsFromResponse(response: CreateTopicsResponse, ttlMs: Long): Unit = { - response.data().topics().forEach { topicResult => - if (topicResult.errorCode() != Errors.NONE.code()) { - val errorMessage = Option(topicResult.errorMessage()) - .filter(_.nonEmpty) - .getOrElse(Errors.forCode(topicResult.errorCode()).message()) - topicCreationErrorCache.put(topicResult.name(), errorMessage, ttlMs) - debug(s"Cached topic creation error for ${topicResult.name()}: $errorMessage") - } - } - } - - override def close(): Unit = { - topicCreationErrorCache.clear() - } -} diff --git a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala b/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala deleted file mode 100644 index c90014622f63e..0000000000000 --- a/core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala +++ /dev/null @@ -1,753 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package kafka.server - -import java.net.InetAddress -import java.util -import java.util.concurrent.CompletableFuture -import java.util.{Optional, Properties} -import kafka.coordinator.transaction.TransactionCoordinator -import kafka.utils.TestUtils -import org.apache.kafka.common.Node -import org.apache.kafka.common.internals.Topic.{GROUP_METADATA_TOPIC_NAME, SHARE_GROUP_STATE_TOPIC_NAME, TRANSACTION_STATE_TOPIC_NAME} -import org.apache.kafka.common.message.CreateTopicsRequestData.{CreatableTopic, CreatableTopicConfig, CreatableTopicConfigCollection} -import org.apache.kafka.common.message.CreateTopicsResponseData -import org.apache.kafka.common.message.CreateTopicsResponseData.CreatableTopicResult -import org.apache.kafka.common.message.MetadataResponseData.MetadataResponseTopic -import org.apache.kafka.common.network.{ClientInformation, ListenerName} -import org.apache.kafka.common.protocol.{ApiKeys, Errors} -import org.apache.kafka.common.requests._ -import org.apache.kafka.common.security.auth.{KafkaPrincipal, KafkaPrincipalSerde, SecurityProtocol} -import org.apache.kafka.common.utils.Utils -import org.apache.kafka.common.utils.internals.SecurityUtils -import org.apache.kafka.server.util.MockTime -import org.apache.kafka.coordinator.group.{GroupCoordinator, GroupCoordinatorConfig} -import org.apache.kafka.coordinator.share.{ShareCoordinator, ShareCoordinatorConfig} -import org.apache.kafka.metadata.MetadataCache -import org.apache.kafka.server.config.ServerConfigs -import org.apache.kafka.coordinator.transaction.TransactionLogConfig -import org.apache.kafka.server.TopicCreator -import org.apache.kafka.server.quota.ControllerMutationQuota -import org.junit.jupiter.api.Assertions.{assertEquals, assertTrue} -import org.junit.jupiter.api.{BeforeEach, Test} -import org.mockito.{ArgumentMatchers, Mockito} - -import scala.collection.{Map, Seq} -import scala.jdk.CollectionConverters._ -import scala.collection.mutable.ListBuffer - -/** - * Test implementation of TopicCreator that tracks method calls and allows configuring responses. - */ -class TestTopicCreator extends TopicCreator { - private val withPrincipalCalls = ListBuffer[(RequestContext, CreateTopicsRequest.Builder)]() - private val withoutPrincipalCalls = ListBuffer[CreateTopicsRequest.Builder]() - private var withPrincipalResponse: CompletableFuture[CreateTopicsResponse] = _ - private var withoutPrincipalResponse: CompletableFuture[CreateTopicsResponse] = _ - - override def createTopicWithPrincipal( - requestContext: RequestContext, - request: CreateTopicsRequest.Builder - ): CompletableFuture[CreateTopicsResponse] = { - withPrincipalCalls += ((requestContext, request)) - if (withPrincipalResponse != null) withPrincipalResponse else CompletableFuture.completedFuture(null) - } - - override def createTopicWithoutPrincipal( - request: CreateTopicsRequest.Builder - ): CompletableFuture[CreateTopicsResponse] = { - withoutPrincipalCalls += request - if (withoutPrincipalResponse != null) withoutPrincipalResponse else CompletableFuture.completedFuture(null) - } - - def setResponseForWithPrincipal(response: CreateTopicsResponse): Unit = { - withPrincipalResponse = CompletableFuture.completedFuture(response) - } - - def setResponseForWithoutPrincipal(response: CreateTopicsResponse): Unit = { - withoutPrincipalResponse = CompletableFuture.completedFuture(response) - } - - def setFutureForWithPrincipal(future: CompletableFuture[CreateTopicsResponse]): Unit = { - withPrincipalResponse = future - } - - def setFutureForWithoutPrincipal(future: CompletableFuture[CreateTopicsResponse]): Unit = { - withoutPrincipalResponse = future - } - - def getWithPrincipalCalls: List[(RequestContext, CreateTopicsRequest.Builder)] = withPrincipalCalls.toList - def getWithoutPrincipalCalls: List[CreateTopicsRequest.Builder] = withoutPrincipalCalls.toList - - def withPrincipalCallCount: Int = withPrincipalCalls.size - def withoutPrincipalCallCount: Int = withoutPrincipalCalls.size - - def reset(): Unit = { - withPrincipalCalls.clear() - withoutPrincipalCalls.clear() - withPrincipalResponse = null - withoutPrincipalResponse = null - } -} - -class AutoTopicCreationManagerTest { - - private val requestTimeout = 100 - private val testCacheCapacity = 3 - private var config: KafkaConfig = _ - private val metadataCache = Mockito.mock(classOf[MetadataCache]) - private val topicCreator = new TestTopicCreator() - private val groupCoordinator = Mockito.mock(classOf[GroupCoordinator]) - private val transactionCoordinator = Mockito.mock(classOf[TransactionCoordinator]) - private val shareCoordinator = Mockito.mock(classOf[ShareCoordinator]) - private var autoTopicCreationManager: AutoTopicCreationManager = _ - private val mockTime = new MockTime(0L, 0L) - - private val internalTopicPartitions = 2 - private val internalTopicReplicationFactor: Short = 2 - - @BeforeEach - def setup(): Unit = { - val props = TestUtils.createBrokerConfig(1) - props.setProperty(ServerConfigs.REQUEST_TIMEOUT_MS_CONFIG, requestTimeout.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_REPLICATION_FACTOR_CONFIG, internalTopicReplicationFactor.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_REPLICATION_FACTOR_CONFIG , internalTopicReplicationFactor.toString) - - props.setProperty(GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, internalTopicPartitions.toString) - props.setProperty(TransactionLogConfig.TRANSACTIONS_TOPIC_PARTITIONS_CONFIG, internalTopicPartitions.toString) - props.setProperty(ShareCoordinatorConfig.STATE_TOPIC_NUM_PARTITIONS_CONFIG, internalTopicPartitions.toString) - - config = KafkaConfig.fromProps(props) - val aliveBrokers = util.List.of(new Node(0, "host0", 0), new Node(1, "host1", 1)) - - Mockito.when(metadataCache.getAliveBrokerNodes(ArgumentMatchers.any(classOf[ListenerName]))).thenReturn(aliveBrokers) - topicCreator.reset() - } - - @Test - def testCreateOffsetTopic(): Unit = { - Mockito.when(groupCoordinator.groupMetadataTopicConfigs).thenReturn(new Properties) - testCreateTopic(GROUP_METADATA_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateTxnTopic(): Unit = { - Mockito.when(transactionCoordinator.transactionTopicConfigs).thenReturn(new Properties) - testCreateTopic(TRANSACTION_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateShareStateTopic(): Unit = { - Mockito.when(shareCoordinator.shareGroupStateTopicConfigs()).thenReturn(new Properties) - testCreateTopic(SHARE_GROUP_STATE_TOPIC_NAME, isInternal = true, internalTopicPartitions, internalTopicReplicationFactor) - } - - @Test - def testCreateNonInternalTopic(): Unit = { - testCreateTopic("topic", isInternal = false) - } - - private def testCreateTopic(topicName: String, - isInternal: Boolean, - numPartitions: Int = 1, - replicationFactor: Short = 1): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity) - - // Set up the topicCreator to return a successful response - val createTopicsResponseData = new CreateTopicsResponseData() - val topicResult = new CreatableTopicResult() - .setName(topicName) - .setErrorCode(Errors.NONE.code()) - createTopicsResponseData.topics().add(topicResult) - val response = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithoutPrincipal(response) - - // First call to create topic - should trigger the topic creator - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - - assertEquals(1, topicCreator.withoutPrincipalCallCount, "Should have called createTopicWithoutPrincipal once") - - // Reset the topicCreator to verify the second call - topicCreator.reset() - topicCreator.setResponseForWithoutPrincipal(response) - - // Second call - should also trigger topicCreator because inflight is cleared after first call completes - createTopicAndVerifyResult(Errors.UNKNOWN_TOPIC_OR_PARTITION, topicName, isInternal) - - assertEquals(1, topicCreator.withoutPrincipalCallCount, "Should have called createTopicWithoutPrincipal once more") - - // Verify the request builder matches expected values - val capturedRequest = topicCreator.getWithoutPrincipalCalls.head.build() - assertEquals(requestTimeout, capturedRequest.data().timeoutMs()) - assertEquals(1, capturedRequest.data().topics().size()) - - // Validate request - val topic = capturedRequest.data().topics().iterator().next() - assertEquals(topicName, topic.name()) - assertEquals(numPartitions, topic.numPartitions()) - assertEquals(replicationFactor, topic.replicationFactor()) - } - - @Test - def testTopicCreationWithMetadataContext(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity) - - val topicName = "topic" - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = Utils.utf8(principal.toString) - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - - val requestContext = initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - - val createTopicsResponseData = new CreateTopicsResponseData() - val topicResult = new CreatableTopicResult() - .setName(topicName) - .setErrorCode(Errors.NONE.code()) - createTopicsResponseData.topics().add(topicResult) - val response = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(response) - - autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, Some(requestContext)) - - assertEquals(1, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal once") - val calls = topicCreator.getWithPrincipalCalls - assertEquals(requestContext, calls.head._1) - - val capturedRequest = calls.head._2.build() - assertEquals(1, capturedRequest.data().topics().size()) - assertEquals(topicName, capturedRequest.data().topics().iterator().next().name()) - } - - @Test - def testCreateStreamsInternalTopics(): Unit = { - val topicConfig = new CreatableTopicConfigCollection() - topicConfig.add(new CreatableTopicConfig().setName("cleanup.policy").setValue("compact")) - - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(3).setReplicationFactor(2).setConfigs(topicConfig), - "stream-topic-2" -> new CreatableTopic().setName("stream-topic-2").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity - ) - - val createTopicsResponseData = new CreateTopicsResponseData() - createTopicsResponseData.topics().add( - new CreatableTopicResult() - .setName("stream-topic-1") - .setErrorCode(Errors.NONE.code())) - createTopicsResponseData.topics().add( - new CreatableTopicResult() - .setName("stream-topic-2") - .setErrorCode(Errors.NONE.code())) - val response = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(response) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - assertEquals(1, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal once") - val calls = topicCreator.getWithPrincipalCalls - assertEquals(requestContext, calls.head._1) - - val capturedRequest = calls.head._2.build() - assertEquals(requestTimeout, capturedRequest.data().timeoutMs()) - assertEquals(2, capturedRequest.data().topics().size()) - val topicNames = capturedRequest.data().topics().asScala.map(_.name()).toSet - assertTrue(topicNames.contains("stream-topic-1")) - assertTrue(topicNames.contains("stream-topic-2")) - } - - @Test - def testCreateStreamsInternalTopicsWithEmptyTopics(): Unit = { - val topics = Map.empty[String, CreatableTopic] - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - assertEquals(0, topicCreator.withPrincipalCallCount, "Should not have called createTopicWithPrincipal") - } - - @Test - def testCreateStreamsInternalTopicsPassesRequestContext(): Unit = { - val topics = Map( - "stream-topic-1" -> new CreatableTopic().setName("stream-topic-1").setNumPartitions(-1).setReplicationFactor(-1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity - ) - - val createTopicsResponseData = new CreateTopicsResponseData() - createTopicsResponseData.topics().add( - new CreatableTopicResult() - .setName("stream-topic-1") - .setErrorCode(Errors.NONE.code())) - val response = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(response) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - assertEquals(1, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal once") - val calls = topicCreator.getWithPrincipalCalls - assertEquals(requestContext, calls.head._1) - } - - private def initializeRequestContextWithUserPrincipal(): RequestContext = { - val userPrincipal = new KafkaPrincipal(KafkaPrincipal.USER_TYPE, "user") - val principalSerde = new KafkaPrincipalSerde { - override def serialize(principal: KafkaPrincipal): Array[Byte] = { - Utils.utf8(principal.toString) - } - override def deserialize(bytes: Array[Byte]): KafkaPrincipal = SecurityUtils.parseKafkaPrincipal(Utils.utf8(bytes)) - } - initializeRequestContext(userPrincipal, Optional.of(principalSerde)) - } - - private def initializeRequestContext(kafkaPrincipal: KafkaPrincipal, - principalSerde: Optional[KafkaPrincipalSerde]): RequestContext = { - val requestHeader = new RequestHeader(ApiKeys.METADATA, ApiKeys.METADATA.latestVersion, - "clientId", 0) - new RequestContext(requestHeader, "1", InetAddress.getLocalHost, Optional.empty(), - kafkaPrincipal, ListenerName.forSecurityProtocol(SecurityProtocol.PLAINTEXT), - SecurityProtocol.PLAINTEXT, ClientInformation.EMPTY, false, principalSerde) - } - - private def createTopicAndVerifyResult(error: Errors, - topicName: String, - isInternal: Boolean, - metadataContext: Option[RequestContext] = None): Unit = { - val topicResponses = autoTopicCreationManager.createTopics( - Set(topicName), ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA, metadataContext) - - val expectedResponses = Seq(new MetadataResponseTopic() - .setErrorCode(error.code()) - .setIsInternal(isInternal) - .setName(topicName)) - - assertEquals(expectedResponses, topicResponses) - } - - @Test - def testTopicCreationErrorCaching(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity - ) - - val topics = Map( - "test-topic-1" -> new CreatableTopic().setName("test-topic-1").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - // Simulate a CreateTopicsResponse with errors - val createTopicsResponseData = new CreateTopicsResponseData() - val topicResult = new CreatableTopicResult() - .setName("test-topic-1") - .setErrorCode(Errors.TOPIC_ALREADY_EXISTS.code()) - .setErrorMessage("Topic 'test-topic-1' already exists.") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(createTopicsResponse) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - // Verify that the error was cached - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic-1"), mockTime.milliseconds()) - assertEquals(1, cachedErrors.size) - assertTrue(cachedErrors.contains("test-topic-1")) - assertEquals("Topic 'test-topic-1' already exists.", cachedErrors("test-topic-1")) - } - - @Test - def testGetTopicCreationErrorsWithMultipleTopics(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity - ) - - val topics = Map( - "success-topic" -> new CreatableTopic().setName("success-topic").setNumPartitions(1).setReplicationFactor(1), - "failed-topic" -> new CreatableTopic().setName("failed-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - - // Simulate mixed response - one success, one failure - val createTopicsResponseData = new CreateTopicsResponseData() - createTopicsResponseData.topics().add( - new CreatableTopicResult() - .setName("success-topic") - .setErrorCode(Errors.NONE.code()) - ) - createTopicsResponseData.topics().add( - new CreatableTopicResult() - .setName("failed-topic") - .setErrorCode(Errors.POLICY_VIOLATION.code()) - .setErrorMessage("Policy violation") - ) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(createTopicsResponse) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - // Only the failed topic should be cached - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("success-topic", "failed-topic", "nonexistent-topic"), mockTime.milliseconds()) - assertEquals(1, cachedErrors.size) - assertTrue(cachedErrors.contains("failed-topic")) - assertEquals("Policy violation", cachedErrors("failed-topic")) - } - - @Test - def testErrorCacheTTL(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity - ) - - // First cache an error by simulating topic creation failure - val topics = Map( - "test-topic" -> new CreatableTopic().setName("test-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - val shortTtlMs = 1000L // Use 1 second TTL for faster testing - - // Simulate a CreateTopicsResponse with error - val createTopicsResponseData = new CreateTopicsResponseData() - val topicResult = new CreatableTopicResult() - .setName("test-topic") - .setErrorCode(Errors.INVALID_REPLICATION_FACTOR.code()) - .setErrorMessage("Invalid replication factor") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(createTopicsResponse) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, shortTtlMs) - - // Verify error is cached and accessible within TTL - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic"), mockTime.milliseconds()) - assertEquals(1, cachedErrors.size) - assertEquals("Invalid replication factor", cachedErrors("test-topic")) - - // Advance time beyond TTL - mockTime.sleep(shortTtlMs + 100) // T0 + 1.1 seconds - - // Verify error is now expired and proactively cleaned up - val expiredErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic"), mockTime.milliseconds()) - assertTrue(expiredErrors.isEmpty, "Expired errors should be proactively cleaned up") - } - - @Test - def testErrorCacheExpirationBasedEviction(): Unit = { - // Create manager with small cache size for testing - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = 3) - - val requestContext = initializeRequestContextWithUserPrincipal() - - // Create 5 topics to exceed the cache size of 3 - val topicNames = (1 to 5).map(i => s"test-topic-$i") - - // Add errors for all 5 topics to the cache - topicNames.zipWithIndex.foreach { case (topicName, idx) => - val topics = Map( - topicName -> new CreatableTopic().setName(topicName).setNumPartitions(1).setReplicationFactor(1) - ) - - // Simulate error response for this topic - val createTopicsResponseData = new CreateTopicsResponseData() - val topicResult = new CreatableTopicResult() - .setName(topicName) - .setErrorCode(Errors.TOPIC_ALREADY_EXISTS.code()) - .setErrorMessage(s"Topic '$topicName' already exists.") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(createTopicsResponse) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, config.groupCoordinatorConfig.streamsGroupHeartbeatIntervalMs() * 2) - - // Advance time slightly between additions to ensure different timestamps - mockTime.sleep(10) - } - - // With cache size of 3, topics 1 and 2 should have been evicted - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(topicNames.toSet, mockTime.milliseconds()) - - // Only the last 3 topics should be in the cache (topics 3, 4, 5) - assertEquals(3, cachedErrors.size, "Cache should contain only the most recent 3 entries") - assertTrue(cachedErrors.contains("test-topic-3"), "test-topic-3 should be in cache") - assertTrue(cachedErrors.contains("test-topic-4"), "test-topic-4 should be in cache") - assertTrue(cachedErrors.contains("test-topic-5"), "test-topic-5 should be in cache") - assertTrue(!cachedErrors.contains("test-topic-1"), "test-topic-1 should have been evicted") - assertTrue(!cachedErrors.contains("test-topic-2"), "test-topic-2 should have been evicted") - } - - @Test - def testTopicsInBackoffAreNotRetried(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity) - - val topics = Map( - "test-topic" -> new CreatableTopic().setName("test-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - val timeoutMs = 5000L - - // Simulate error response to cache the error - val createTopicsResponseData = new CreateTopicsResponseData() - val topicResult = new CreatableTopicResult() - .setName("test-topic") - .setErrorCode(Errors.INVALID_REPLICATION_FACTOR.code()) - .setErrorMessage("Invalid replication factor") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(createTopicsResponse) - - // First attempt - trigger topic creation - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, timeoutMs) - - // Verify error is cached - val cachedErrors = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic"), mockTime.milliseconds()) - assertEquals(1, cachedErrors.size) - - // Second attempt - should NOT send request because topic is in back-off - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, timeoutMs) - - // Verify still only one request was sent (not retried during back-off) - assertEquals(1, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal once") - } - - @Test - def testTopicsOutOfBackoffCanBeRetried(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity) - - val topics = Map( - "test-topic" -> new CreatableTopic().setName("test-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - val shortTtlMs = 1000L - - // Simulate error response to cache the error - val createTopicsResponseData = new CreateTopicsResponseData() - val topicResult = new CreatableTopicResult() - .setName("test-topic") - .setErrorCode(Errors.INVALID_REPLICATION_FACTOR.code()) - .setErrorMessage("Invalid replication factor") - createTopicsResponseData.topics().add(topicResult) - - val createTopicsResponse = new CreateTopicsResponse(createTopicsResponseData) - topicCreator.setResponseForWithPrincipal(createTopicsResponse) - - // First attempt - trigger topic creation - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, shortTtlMs) - - // Verify error is cached - val cachedErrors1 = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic"), mockTime.milliseconds()) - assertEquals(1, cachedErrors1.size) - - // Advance time beyond TTL to exit back-off period - mockTime.sleep(shortTtlMs + 100) - - // Verify error is expired - val cachedErrors2 = autoTopicCreationManager.getStreamsInternalTopicCreationErrors(Set("test-topic"), mockTime.milliseconds()) - assertTrue(cachedErrors2.isEmpty, "Error should be expired after TTL") - - // Second attempt - should send request because topic is out of back-off - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, shortTtlMs) - - // Verify a second request was sent (retry allowed after back-off expires) - assertEquals(2, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal twice") - } - - @Test - def testInflightTopicsAreNotRetriedConcurrently(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity) - - val topics = Map( - "test-topic" -> new CreatableTopic().setName("test-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - val timeoutMs = 5000L - - // Use a future that doesn't complete immediately to simulate in-flight state - val future = new CompletableFuture[CreateTopicsResponse]() - topicCreator.setFutureForWithPrincipal(future) - - // First call - should send request and mark topic as in-flight - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, timeoutMs) - - assertEquals(1, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal once") - - // Second concurrent call - should NOT send request because topic is in-flight - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, timeoutMs) - - // Verify still only one request was sent (concurrent request blocked) - assertEquals(1, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal once") - } - - @Test - def testBackoffAndInflightInteraction(): Unit = { - autoTopicCreationManager = new DefaultAutoTopicCreationManager( - config, - groupCoordinator, - transactionCoordinator, - shareCoordinator, - mockTime, - topicCreator, - topicErrorCacheCapacity = testCacheCapacity) - - val topics = Map( - "backoff-topic" -> new CreatableTopic().setName("backoff-topic").setNumPartitions(1).setReplicationFactor(1), - "inflight-topic" -> new CreatableTopic().setName("inflight-topic").setNumPartitions(1).setReplicationFactor(1), - "normal-topic" -> new CreatableTopic().setName("normal-topic").setNumPartitions(1).setReplicationFactor(1) - ) - val requestContext = initializeRequestContextWithUserPrincipal() - val timeoutMs = 5000L - - // Simulate error response for backoff-topic - val backoffResponseData = new CreateTopicsResponseData() - val backoffResult = new CreatableTopicResult() - .setName("backoff-topic") - .setErrorCode(Errors.INVALID_REPLICATION_FACTOR.code()) - .setErrorMessage("Invalid replication factor") - backoffResponseData.topics().add(backoffResult) - val backoffResponse = new CreateTopicsResponse(backoffResponseData) - topicCreator.setResponseForWithPrincipal(backoffResponse) - - // Create error for backoff-topic - val backoffOnly = Map("backoff-topic" -> topics("backoff-topic")) - autoTopicCreationManager.createStreamsInternalTopics(backoffOnly, requestContext, timeoutMs) - - // Make inflight-topic in-flight (without completing the request) - val inflightFuture = new CompletableFuture[CreateTopicsResponse]() - topicCreator.setFutureForWithPrincipal(inflightFuture) - - val inflightOnly = Map("inflight-topic" -> topics("inflight-topic")) - autoTopicCreationManager.createStreamsInternalTopics(inflightOnly, requestContext, timeoutMs) - - // Now attempt to create all three topics together - val normalResponseData = new CreateTopicsResponseData() - val normalResult = new CreatableTopicResult() - .setName("normal-topic") - .setErrorCode(Errors.NONE.code()) - normalResponseData.topics().add(normalResult) - val normalResponse = new CreateTopicsResponse(normalResponseData) - topicCreator.setResponseForWithPrincipal(normalResponse) - - autoTopicCreationManager.createStreamsInternalTopics(topics, requestContext, timeoutMs) - - // Total 3 requests: 1 for backoff-topic, 1 for inflight-topic, 1 for normal-topic only - assertEquals(3, topicCreator.withPrincipalCallCount, "Should have called createTopicWithPrincipal 3 times") - - // Verify that only normal-topic was included in the last request - val calls = topicCreator.getWithPrincipalCalls - val lastRequest = calls(2)._2.build() - val topicNames = lastRequest.data().topics().asScala.map(_.name()).toSet - assertEquals(1, topicNames.size, "Only normal-topic should be created") - assertTrue(topicNames.contains("normal-topic"), "normal-topic should be in the request") - assertTrue(!topicNames.contains("backoff-topic"), "backoff-topic should be filtered (in back-off)") - assertTrue(!topicNames.contains("inflight-topic"), "inflight-topic should be filtered (in-flight)") - } -} From 9011a2437b6001d36088aa057ebd63ccd5734f55 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Wed, 6 May 2026 00:37:32 +0800 Subject: [PATCH 10/14] fix package --- .../kafka/server/DefaultAutoTopicCreationManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java index a445ee3169b39..10c74a7783762 100644 --- a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java +++ b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java @@ -34,8 +34,8 @@ import org.apache.kafka.common.security.auth.KafkaPrincipal; import org.apache.kafka.common.security.auth.KafkaPrincipalSerde; import org.apache.kafka.common.security.auth.SecurityProtocol; -import org.apache.kafka.common.utils.SecurityUtils; import org.apache.kafka.common.utils.Utils; +import org.apache.kafka.common.utils.internals.SecurityUtils; import org.apache.kafka.coordinator.group.GroupCoordinatorConfig; import org.apache.kafka.coordinator.share.ShareCoordinatorConfig; import org.apache.kafka.coordinator.transaction.TransactionLogConfig; From 84eeafa57552720e13bc7e38067972a3951d78ab Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Wed, 6 May 2026 00:42:15 +0800 Subject: [PATCH 11/14] rm unused file --- .../kafka/server/ForwardingManagerUtils.java | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java diff --git a/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java b/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java deleted file mode 100644 index 6fc623a3fbdf1..0000000000000 --- a/server/src/main/java/org/apache/kafka/server/ForwardingManagerUtils.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.kafka.server; - -import org.apache.kafka.common.requests.EnvelopeRequest; -import org.apache.kafka.common.requests.RequestContext; - -import java.nio.ByteBuffer; - -public class ForwardingManagerUtils { - - public static EnvelopeRequest.Builder buildEnvelopeRequest(RequestContext context, ByteBuffer forwardRequestBuffer) { - var principalSerde = context.principalSerde.orElseThrow(() -> new IllegalArgumentException( - String.format("Cannot deserialize principal from request context %s since there is no serde defined", context)) - ); - - var serializedPrincipal = principalSerde.serialize(context.principal); - return new EnvelopeRequest.Builder( - forwardRequestBuffer, - serializedPrincipal, - context.clientAddress.getAddress() - ); - } -} From 5020388073aae5e4c9b8c74031466767289504e8 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Wed, 6 May 2026 00:47:45 +0800 Subject: [PATCH 12/14] adjust method location --- .../apache/kafka/server/config/AbstractKafkaConfig.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java b/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java index a6de3f66174ba..46c07cbdfa456 100644 --- a/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java +++ b/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java @@ -191,10 +191,6 @@ public Map effectiveListenerSecurityProtocolMap( } } - public int defaultReplicationFactor() { - return getInt(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG).shortValue(); - } - public static Map getMap(String propName, String propValue) { try { return Csv.parseCsvMap(propValue); @@ -492,6 +488,11 @@ public long connectionSetupTimeoutMaxMs() { return getLong(ServerConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG); } + // ********* Replication configuration *********** + public int defaultReplicationFactor() { + return getInt(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG).shortValue(); + } + // ********* Log Configuration ********** public boolean autoCreateTopicsEnable() { From 7a61facb90768a093b0f8024ea328f097774b9e0 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Thu, 7 May 2026 23:20:02 +0800 Subject: [PATCH 13/14] remove useless code --- core/src/main/scala/kafka/server/BrokerServer.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/kafka/server/BrokerServer.scala b/core/src/main/scala/kafka/server/BrokerServer.scala index 27c206bd49364..0b0cf2c5f6eb9 100644 --- a/core/src/main/scala/kafka/server/BrokerServer.scala +++ b/core/src/main/scala/kafka/server/BrokerServer.scala @@ -406,9 +406,9 @@ class BrokerServer( val topicCreator = new KRaftTopicCreator(clientToControllerChannelManager) autoTopicCreationManager = new DefaultAutoTopicCreationManager( config, - () => groupCoordinator.groupMetadataTopicConfigs(), + () => groupCoordinator.groupMetadataTopicConfigs, () => transactionCoordinator.transactionTopicConfigs, - () => shareCoordinator.shareGroupStateTopicConfigs(), + () => shareCoordinator.shareGroupStateTopicConfigs, topicCreator, time, ) From 7d7b47e20c05de7c95803de42f1124e186df6fc3 Mon Sep 17 00:00:00 2001 From: Kuan-Po Tseng Date: Thu, 7 May 2026 23:58:58 +0800 Subject: [PATCH 14/14] fix --- .../scala/unit/kafka/server/KafkaApisTest.scala | 17 ++++++++++------- .../server/DefaultAutoTopicCreationManager.java | 9 ++++++--- .../server/config/AbstractKafkaConfig.java | 2 +- .../DefaultAutoTopicCreationManagerTest.java | 7 ++++--- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala index 7c7cb902e7076..dd710d99f4a28 100644 --- a/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala +++ b/core/src/test/scala/unit/kafka/server/KafkaApisTest.scala @@ -792,7 +792,7 @@ class KafkaApisTest extends Logging { when(clientRequestQuotaManager.maybeRecordAndGetThrottleTimeMs(any[RequestChannel.Request](), any[Long])).thenReturn(0) - verifyTopicCreation(topicName, enableAutoTopicCreation = true, isInternal = true, request) + val capturedRequest = verifyTopicCreation(topicName, enableAutoTopicCreation = true, isInternal = true, request) kafkaApis = createKafkaApis(authorizer = Some(authorizer), overrideProperties = topicConfigOverride) kafkaApis.handleFindCoordinatorRequest(request) @@ -805,6 +805,10 @@ class KafkaApisTest extends Logging { assertEquals(key, response.data.coordinators.get(0).key) } else { assertEquals(Errors.COORDINATOR_NOT_AVAILABLE.code, response.data.errorCode) + assertTrue(capturedRequest.getValue.isEmpty) + } + if (checkAutoCreateTopic) { + assertTrue(capturedRequest.getValue.isEmpty) } } @@ -962,7 +966,7 @@ class KafkaApisTest extends Logging { assertEquals(expectedMetadataResponse, response.topicMetadata()) - if (enableAutoTopicCreation && isInternal) { + if (enableAutoTopicCreation) { assertTrue(capturedRequest.getValue.isPresent) assertEquals(request.context, capturedRequest.getValue.get) } @@ -983,12 +987,11 @@ class KafkaApisTest extends Logging { when(autoTopicCreationManager.createTopics( ArgumentMatchers.eq(util.Set.of(topicName)), ArgumentMatchers.eq(ControllerMutationQuota.UNBOUNDED_CONTROLLER_MUTATION_QUOTA), - capturedRequest.capture() - )).thenReturn( + capturedRequest.capture())).thenReturn( util.List.of(new MetadataResponseTopic() - .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) - .setIsInternal(isInternal) - .setName(topicName)) + .setErrorCode(Errors.UNKNOWN_TOPIC_OR_PARTITION.code()) + .setIsInternal(isInternal) + .setName(topicName)) ) } capturedRequest diff --git a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java index f2baf973ba97c..2e5895f7ff178 100644 --- a/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java +++ b/server/src/main/java/org/apache/kafka/server/DefaultAutoTopicCreationManager.java @@ -17,7 +17,10 @@ package org.apache.kafka.server; +import org.apache.kafka.common.errors.AuthenticationException; import org.apache.kafka.common.errors.InvalidTopicException; +import org.apache.kafka.common.errors.TimeoutException; +import org.apache.kafka.common.errors.UnsupportedVersionException; import org.apache.kafka.common.internals.Topic; import org.apache.kafka.common.message.CreateTopicsRequestData; import org.apache.kafka.common.message.CreateTopicsRequestData.CreatableTopic; @@ -222,11 +225,11 @@ private CreateTopicsRequest.Builder makeCreateTopicsRequestBuilder(Map creatableTopics, Throwable throwable) { - if (throwable instanceof org.apache.kafka.common.errors.TimeoutException) { + if (throwable instanceof TimeoutException) { LOGGER.debug("Auto topic creation timed out for {}.", creatableTopics.keySet()); - } else if (throwable instanceof org.apache.kafka.common.errors.AuthenticationException) { + } else if (throwable instanceof AuthenticationException) { LOGGER.warn("Auto topic creation failed for {} with authentication exception.", creatableTopics.keySet()); - } else if (throwable instanceof org.apache.kafka.common.errors.UnsupportedVersionException) { + } else if (throwable instanceof UnsupportedVersionException) { LOGGER.warn("Auto topic creation failed for {} with invalid version exception.", creatableTopics.keySet()); } else { LOGGER.warn("Auto topic creation failed for {} with exception.", creatableTopics.keySet(), throwable); diff --git a/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java b/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java index 46c07cbdfa456..631ae477ac009 100644 --- a/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java +++ b/server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java @@ -490,7 +490,7 @@ public long connectionSetupTimeoutMaxMs() { // ********* Replication configuration *********** public int defaultReplicationFactor() { - return getInt(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG).shortValue(); + return getInt(ReplicationConfigs.DEFAULT_REPLICATION_FACTOR_CONFIG); } // ********* Log Configuration ********** diff --git a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java index 10c74a7783762..582c963d0ab34 100644 --- a/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java +++ b/server/src/test/java/org/apache/kafka/server/DefaultAutoTopicCreationManagerTest.java @@ -469,6 +469,7 @@ public void testErrorCacheExpirationBasedEviction() throws UnknownHostException // Create 5 topics to exceed the cache size of 3 List topicNames = IntStream.rangeClosed(1, 5).mapToObj(i -> "test-topic-" + i).toList(); + // Add errors for all 5 topics to the cache for (String topicName : topicNames) { var topics = Map.of(topicName, new CreatableTopic().setName(topicName).setNumPartitions(1).setReplicationFactor((short) 1)); @@ -636,7 +637,7 @@ public void testBackoffAndInflightInteraction() throws UnknownHostException { var requestContext = initializeRequestContextWithUserPrincipal(); long timeoutMs = 5000L; - // Step 1: Simulate error response for backoff-topic + // Simulate error response for backoff-topic var backoffResponseData = new CreateTopicsResponseData(); backoffResponseData.topics().add(new CreatableTopicResult() .setName("backoff-topic") @@ -649,7 +650,7 @@ public void testBackoffAndInflightInteraction() throws UnknownHostException { new CreatableTopic().setName("backoff-topic").setNumPartitions(1).setReplicationFactor((short) 1)); autoTopicCreationManager.createStreamsInternalTopics(backoffTopics, requestContext, timeoutMs); - // Step 2: Make inflight-topic in-flight (without completing the request) + // Make inflight-topic in-flight (without completing the request) var inflightFuture = new CompletableFuture(); topicCreator.setFutureForWithPrincipal(inflightFuture); @@ -657,7 +658,7 @@ public void testBackoffAndInflightInteraction() throws UnknownHostException { new CreatableTopic().setName("inflight-topic").setNumPartitions(1).setReplicationFactor((short) 1)); autoTopicCreationManager.createStreamsInternalTopics(inflightTopics, requestContext, timeoutMs); - // Step 3: Now attempt to create all three topics together + // Now attempt to create all three topics together var normalResponseData = new CreateTopicsResponseData(); normalResponseData.topics().add(new CreatableTopicResult() .setName("normal-topic")