Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/scala/kafka/api/ApiVersion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ object ApiVersion {
KAFKA_2_7_IV1,
// Introduced AlterIsr (KIP-497)
KAFKA_2_7_IV2,
// Flexible versioning on ListOffsets, WriteTxnMarkers and OffsetsForLeaderEpoch.
// Flexible versioning on ListOffsets, WriteTxnMarkers and OffsetsForLeaderEpoch. Also adds topic IDs (KIP-516)
KAFKA_2_8_IV0,
// Introduced topic IDs to LeaderAndIsr and UpdateMetadata requests/responses (KIP-516)
KAFKA_2_8_IV1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import org.apache.kafka.common.requests._
import org.apache.kafka.common.security.JaasContext
import org.apache.kafka.common.security.auth.SecurityProtocol
import org.apache.kafka.common.utils.{LogContext, Time}
import org.apache.kafka.common.{KafkaException, Node, Reconfigurable, TopicPartition}
import org.apache.kafka.common.{KafkaException, Node, Reconfigurable, TopicPartition, Uuid}

import scala.jdk.CollectionConverters._
import scala.collection.mutable.HashMap
Expand Down Expand Up @@ -486,7 +486,7 @@ abstract class AbstractControllerBrokerRequestBatch(config: KafkaConfig,
val topicIds = leaderAndIsrPartitionStates.keys
.map(_.topic)
.toSet[String]
.map(topic => (topic, controllerContext.topicIds(topic)))
.map(topic => (topic, controllerContext.topicIds.getOrElse(topic, Uuid.ZERO_UUID)))
.toMap
val leaderAndIsrRequestBuilder = new LeaderAndIsrRequest.Builder(leaderAndIsrRequestVersion, controllerId,
controllerEpoch, brokerEpoch, leaderAndIsrPartitionStates.values.toBuffer.asJava, topicIds.asJava, leaders.asJava)
Expand Down
12 changes: 7 additions & 5 deletions core/src/main/scala/kafka/controller/KafkaController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ class KafkaController(val config: KafkaConfig,
(topicPartition -> assignment)

val setDataResponse = zkClient.setTopicAssignmentRaw(topicPartition.topic,
controllerContext.topicIds(topicPartition.topic),
controllerContext.topicIds.get(topicPartition.topic),
topicAssignment, controllerContext.epochZkVersion)
setDataResponse.resultCode match {
case Code.OK =>
Expand Down Expand Up @@ -1658,9 +1658,11 @@ class KafkaController(val config: KafkaConfig,
}

private def processTopicIds(topicIdAssignments: Set[TopicIdReplicaAssignment]): Unit = {
val updated = zkClient.setTopicIds(topicIdAssignments.filter(_.topicId.isEmpty), controllerContext.epochZkVersion)
val allTopicIdAssignments = updated ++ topicIdAssignments.filter(_.topicId.isDefined)
allTopicIdAssignments.foreach(topicIdAssignment => controllerContext.addTopicId(topicIdAssignment.topic, topicIdAssignment.topicId.get))
if (config.usesTopicId) {
val updated = zkClient.setTopicIds(topicIdAssignments.filter(_.topicId.isEmpty), controllerContext.epochZkVersion)
val allTopicIdAssignments = updated ++ topicIdAssignments.filter(_.topicId.isDefined)
allTopicIdAssignments.foreach(topicIdAssignment => controllerContext.addTopicId(topicIdAssignment.topic, topicIdAssignment.topicId.get))
}
}

private def processLogDirEventNotification(): Unit = {
Expand Down Expand Up @@ -1690,7 +1692,7 @@ class KafkaController(val config: KafkaConfig,
}.toMap

zkClient.setTopicAssignment(topic,
controllerContext.topicIds(topic),
controllerContext.topicIds.get(topic),
existingPartitionReplicaAssignment,
controllerContext.epochZkVersion)
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/kafka/server/KafkaApis.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ class KafkaApis(val requestChannel: RequestChannel,
replicationFactor: Int,
properties: util.Properties = new util.Properties()): MetadataResponseTopic = {
try {
adminZkClient.createTopic(topic, numPartitions, replicationFactor, properties, RackAwareMode.Safe)
adminZkClient.createTopic(topic, numPartitions, replicationFactor, properties, RackAwareMode.Safe, config.usesTopicId)
info("Auto creation of topic %s with %d partitions and replication factor %d is successful"
.format(topic, numPartitions, replicationFactor))
metadataResponseTopic(Errors.LEADER_NOT_AVAILABLE, topic, isInternal(topic), util.Collections.emptyList())
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/scala/kafka/server/KafkaConfig.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package kafka.server
import java.util
import java.util.{Collections, Locale, Properties}

import kafka.api.{ApiVersion, ApiVersionValidator, KAFKA_0_10_0_IV1, KAFKA_2_1_IV0, KAFKA_2_7_IV0}
import kafka.api.{ApiVersion, ApiVersionValidator, KAFKA_0_10_0_IV1, KAFKA_2_1_IV0, KAFKA_2_7_IV0, KAFKA_2_8_IV0}
import kafka.cluster.EndPoint
import kafka.coordinator.group.OffsetConfig
import kafka.coordinator.transaction.{TransactionLog, TransactionStateManager}
Expand Down Expand Up @@ -1798,6 +1798,9 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
}
}

def usesTopicId: Boolean =
interBrokerProtocolVersion >= KAFKA_2_8_IV0

validateValues()

private def validateValues(): Unit = {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/kafka/server/ZkAdminManager.scala
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class ZkAdminManager(val config: KafkaConfig,
CreatePartitionsMetadata(topic.name, assignments.keySet)
} else {
controllerMutationQuota.record(assignments.size)
adminZkClient.createTopicWithAssignment(topic.name, configs, assignments, validate = false)
adminZkClient.createTopicWithAssignment(topic.name, configs, assignments, validate = false, config.usesTopicId)
CreatePartitionsMetadata(topic.name, assignments.keySet)
}
} catch {
Expand Down
21 changes: 13 additions & 8 deletions core/src/main/scala/kafka/zk/AdminZkClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,17 @@ class AdminZkClient(zkClient: KafkaZkClient) extends Logging {
* @param replicationFactor Replication factor
* @param topicConfig topic configs
* @param rackAwareMode
* @param usesTopicId Boolean indicating whether the topic ID will be created
*/
def createTopic(topic: String,
partitions: Int,
replicationFactor: Int,
topicConfig: Properties = new Properties,
rackAwareMode: RackAwareMode = RackAwareMode.Enforced): Unit = {
rackAwareMode: RackAwareMode = RackAwareMode.Enforced,
usesTopicId: Boolean = false): Unit = {
val brokerMetadatas = getBrokerMetadatas(rackAwareMode)
val replicaAssignment = AdminUtils.assignReplicasToBrokers(brokerMetadatas, partitions, replicationFactor)
createTopicWithAssignment(topic, topicConfig, replicaAssignment)
createTopicWithAssignment(topic, topicConfig, replicaAssignment, usesTopicId = usesTopicId)
}

/**
Expand Down Expand Up @@ -90,11 +92,13 @@ class AdminZkClient(zkClient: KafkaZkClient) extends Logging {
* @param config The config of the topic
* @param partitionReplicaAssignment The assignments of the topic
* @param validate Boolean indicating if parameters must be validated or not (true by default)
* @param usesTopicId Boolean indicating whether the topic ID will be created
*/
def createTopicWithAssignment(topic: String,
config: Properties,
partitionReplicaAssignment: Map[Int, Seq[Int]],
validate: Boolean = true): Unit = {
validate: Boolean = true,
usesTopicId: Boolean = false): Unit = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if we use TopicCommand with ZookeeperTopicService to create a topic, we will always not set a topicId?

@jolshan jolshan Jan 10, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially yes. The controller will process and add topic IDs to these topics if IBP is at least 2.8 in processTopicChange() using the processTopicIds method.

if (validate)
validateTopicCreate(topic, partitionReplicaAssignment, config)

Expand All @@ -106,7 +110,7 @@ class AdminZkClient(zkClient: KafkaZkClient) extends Logging {

// create the partition assignment
writeTopicPartitionAssignment(topic, partitionReplicaAssignment.map { case (k, v) => k -> ReplicaAssignment(v) },
isUpdate = false)
isUpdate = false, usesTopicId)
}

/**
Expand Down Expand Up @@ -153,16 +157,17 @@ class AdminZkClient(zkClient: KafkaZkClient) extends Logging {
LogConfig.validate(config)
}

private def writeTopicPartitionAssignment(topic: String, replicaAssignment: Map[Int, ReplicaAssignment], isUpdate: Boolean): Unit = {
private def writeTopicPartitionAssignment(topic: String, replicaAssignment: Map[Int, ReplicaAssignment],
isUpdate: Boolean, usesTopicId: Boolean = false): Unit = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you set the default value to false? I don't think we should give it a default value since the caller can decide it by caller.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some places that eventually call this do not have access to IBP. The controller will eventually pick up and assign topic IDs that need them.

try {
val assignment = replicaAssignment.map { case (partitionId, replicas) => (new TopicPartition(topic,partitionId), replicas) }.toMap

if (!isUpdate) {
val topicId = Uuid.randomUuid()
zkClient.createTopicAssignment(topic, topicId, assignment.map { case (k, v) => k -> v.replicas })
val topicIdOpt = if (usesTopicId) Some(Uuid.randomUuid()) else None
zkClient.createTopicAssignment(topic, topicIdOpt, assignment.map { case (k, v) => k -> v.replicas })
} else {
val topicIds = zkClient.getTopicIdsForTopics(Set(topic))
zkClient.setTopicAssignment(topic, topicIds(topic), assignment)
zkClient.setTopicAssignment(topic, topicIds.get(topic), assignment)
}
debug("Updated path %s with %s for replica assignment".format(TopicZNode.path(topic), assignment))
} catch {
Expand Down
22 changes: 10 additions & 12 deletions core/src/main/scala/kafka/zk/KafkaZkClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo
}.toSet

val setDataRequests = updatedAssignments.map { case TopicIdReplicaAssignment(topic, topicIdOpt, assignments) =>
SetDataRequest(TopicZNode.path(topic), TopicZNode.encode(topicIdOpt.get, assignments), ZkVersion.MatchAnyVersion)
SetDataRequest(TopicZNode.path(topic), TopicZNode.encode(topicIdOpt, assignments), ZkVersion.MatchAnyVersion)
}.toSeq

retryRequestsUntilConnected(setDataRequests, expectedControllerEpochZkVersion)
Expand All @@ -507,13 +507,13 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo
/**
* Sets the topic znode with the given assignment.
* @param topic the topic whose assignment is being set.
* @param topicId unique topic ID for the topic
* @param topicId unique topic ID for the topic if the version supports it
* @param assignment the partition to replica mapping to set for the given topic
* @param expectedControllerEpochZkVersion expected controller epoch zkVersion.
* @return SetDataResponse
*/
def setTopicAssignmentRaw(topic: String,
topicId: Uuid,
topicId: Option[Uuid],
assignment: collection.Map[TopicPartition, ReplicaAssignment],
expectedControllerEpochZkVersion: Int): SetDataResponse = {
val setDataRequest = SetDataRequest(TopicZNode.path(topic), TopicZNode.encode(topicId, assignment), ZkVersion.MatchAnyVersion)
Expand All @@ -523,13 +523,13 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo
/**
* Sets the topic znode with the given assignment.
* @param topic the topic whose assignment is being set.
* @param topicId unique topic ID for the topic
* @param topicId unique topic ID for the topic if the version supports it
* @param assignment the partition to replica mapping to set for the given topic
* @param expectedControllerEpochZkVersion expected controller epoch zkVersion.
* @throws KeeperException if there is an error while setting assignment
*/
def setTopicAssignment(topic: String,
topicId: Uuid,
topicId: Option[Uuid],
assignment: Map[TopicPartition, ReplicaAssignment],
expectedControllerEpochZkVersion: Int = ZkVersion.MatchAnyVersion) = {
val setDataResponse = setTopicAssignmentRaw(topic, topicId, assignment, expectedControllerEpochZkVersion)
Expand All @@ -539,11 +539,11 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo
/**
* Create the topic znode with the given assignment.
* @param topic the topic whose assignment is being set.
* @param topicId unique topic ID for the topic
* @param topicId unique topic ID for the topic if the version supports it
* @param assignment the partition to replica mapping to set for the given topic
* @throws KeeperException if there is an error while creating assignment
*/
def createTopicAssignment(topic: String, topicId: Uuid, assignment: Map[TopicPartition, Seq[Int]]): Unit = {
def createTopicAssignment(topic: String, topicId: Option[Uuid], assignment: Map[TopicPartition, Seq[Int]]): Unit = {
val persistedAssignments = assignment.map { case (k, v) => k -> ReplicaAssignment(v) }
createRecursive(TopicZNode.path(topic), TopicZNode.encode(topicId, persistedAssignments))
}
Expand Down Expand Up @@ -609,7 +609,6 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo
* Gets the topic IDs for the given topics.
* @param topics the topics we wish to retrieve the Topic IDs for
* @return the Topic IDs
* @throws IllegalStateException if a topic in topics does not have a Topic ID
*/
def getTopicIdsForTopics(topics: Set[String]): Map[String, Uuid] = {
val getDataRequests = topics.map(topic => GetDataRequest(TopicZNode.path(topic), ctx = Some(topic)))
Expand All @@ -621,10 +620,9 @@ class KafkaZkClient private[zk] (zooKeeperClient: ZooKeeperClient, isSecure: Boo
case Code.NONODE => None
case _ => throw getDataResponse.resultException.get
}
}.map(_.get)
.map(topicIdAssignment => (topicIdAssignment.topic,
topicIdAssignment.topicId.getOrElse(
throw new IllegalStateException("Topic " + topicIdAssignment.topic + " does not have a topic ID."))))
}.filter(_.flatMap(_.topicId).isDefined)
.map(_.get)
.map(topicIdAssignment => (topicIdAssignment.topic, topicIdAssignment.topicId.get))
.toMap
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/scala/kafka/zk/ZkData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ object TopicZNode {
topicId: Option[Uuid],
assignment: Map[TopicPartition, ReplicaAssignment])
def path(topic: String) = s"${TopicsZNode.path}/$topic"
def encode(topicId: Uuid,
def encode(topicId: Option[Uuid],
assignment: collection.Map[TopicPartition, ReplicaAssignment]): Array[Byte] = {
val replicaAssignmentJson = mutable.Map[String, util.List[Int]]()
val addingReplicasAssignmentJson = mutable.Map[String, util.List[Int]]()
Expand All @@ -299,11 +299,11 @@ object TopicZNode {

val topicAssignment = mutable.Map(
"version" -> 3,
"topic_id" -> topicId.toString,
"partitions" -> replicaAssignmentJson.asJava,
"adding_replicas" -> addingReplicasAssignmentJson.asJava,
"removing_replicas" -> removingReplicasAssignmentJson.asJava
)
topicId.foreach(id => topicAssignment += "topic_id" -> id.toString)

Json.encodeAsBytes(topicAssignment.asJava)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class PreferredReplicaLeaderElectionCommandTest extends ZooKeeperTestHarness wit
servers = brokerConfigs.map(b => TestUtils.createServer(KafkaConfig.fromProps(b)))
// create the topic
partitionsAndAssignments.foreach { case (tp, assignment) =>
zkClient.createTopicAssignment(tp.topic, Uuid.randomUuid(),
zkClient.createTopicAssignment(tp.topic, Some(Uuid.randomUuid()),
Map(tp -> assignment))
}
// wait until replica log is created on every broker
Expand Down
Loading