From 9effa4f0056f9a3f749cd425d45e9b206cc07866 Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Sat, 6 Mar 2021 13:07:51 -0800
Subject: [PATCH 1/9] Added partitionMetadata file code to RaftReplicaManager

---
 .../main/scala/kafka/cluster/Partition.scala  | 31 +++++++++----------
 .../main/scala/kafka/server/KafkaConfig.scala |  3 +-
 .../kafka/server/RaftReplicaManager.scala     | 12 +++++++
 .../scala/kafka/server/ReplicaManager.scala   |  2 +-
 .../kafka/server/metadata/MetadataImage.scala | 10 +++++-
 .../server/metadata/MetadataPartitions.scala  | 28 +++++++++++++----
 .../metadata/MetadataPartitionsTest.scala     |  2 +-
 .../kafka/server/RaftReplicaManagerTest.scala | 30 ++++++++++++++++++
 8 files changed, 92 insertions(+), 26 deletions(-)

diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala
index 9d33381815136..8f7a5dd314359 100755
--- a/core/src/main/scala/kafka/cluster/Partition.scala
+++ b/core/src/main/scala/kafka/cluster/Partition.scala
@@ -425,36 +425,35 @@ class Partition(val topicPartition: TopicPartition,
   }
 
   /**
-   * Checks if the topic ID provided in the request is consistent with the topic ID in the log.
+   * Checks if the topic ID received is consistent with the topic ID in the log.
    * If a valid topic ID is provided, and the log exists but has no ID set, set the log ID to be the request ID.
    *
-   * @param requestTopicId the topic ID from the request
+   * @param receivedTopicId the topic ID from the LeaderAndIsr request or from the metadata records
    * @return true if the request topic id is consistent, false otherwise
    */
-  def checkOrSetTopicId(requestTopicId: Uuid): Boolean = {
-    // If the request had an invalid topic ID, then we assume that topic IDs are not supported.
-    // The topic ID was not inconsistent, so return true.
-    // If the log is empty, then we can not say that topic ID is inconsistent, so return true.
-    if (requestTopicId == null || requestTopicId == Uuid.ZERO_UUID)
-      true
-    else {
+  def checkOrSetTopicId(receivedTopicId: Uuid, usingRaft: Boolean): Boolean = {
+    // If the request had an invalid topic ID, then we assume that topic IDs are not supported so ID is consistent.
+    // This is only the case when from LeaderAndIsr Request. Raft code should never have invalid topic IDs.
+    if (receivedTopicId == null || receivedTopicId == Uuid.ZERO_UUID) {
+      if (usingRaft) false else true
+    } else {
       log match {
-        case None => true
+        case None => true // log is empty, so we can not say topic ID is inconsistent
         case Some(log) => {
           // Check if topic ID is in memory, if not, it must be new to the broker and does not have a metadata file.
           // This is because if the broker previously wrote it to file, it would be recovered on restart after failure.
           // Topic ID is consistent since we are just setting it here.
           if (log.topicId == Uuid.ZERO_UUID) {
-            log.partitionMetadataFile.write(requestTopicId)
-            log.topicId = requestTopicId
+            log.partitionMetadataFile.write(receivedTopicId)
+            log.topicId = receivedTopicId
             true
-          } else if (log.topicId != requestTopicId) {
+          } else if (log.topicId != receivedTopicId) {
             stateChangeLogger.error(s"Topic Id in memory: ${log.topicId} does not" +
-              s" match the topic Id for partition $topicPartition provided in the request: " +
-              s"$requestTopicId.")
+              s" match the topic Id for partition $topicPartition provided in the " +
+              s"${if (usingRaft) "metadata records" else "request"}: $receivedTopicId.")
             false
           } else {
-            // topic ID in log exists and matches request topic ID
+            // topic ID in log exists and matches received topic ID
             true
           }
         }
diff --git a/core/src/main/scala/kafka/server/KafkaConfig.scala b/core/src/main/scala/kafka/server/KafkaConfig.scala
index 39a93aa47acf5..3171a10542b03 100755
--- a/core/src/main/scala/kafka/server/KafkaConfig.scala
+++ b/core/src/main/scala/kafka/server/KafkaConfig.scala
@@ -1892,8 +1892,9 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
     }
   }
 
+  // Topic IDs are used with all self-managed quorum clusters and ZK cluster with IBP greater than or equal to 2.8
   def usesTopicId: Boolean =
-    interBrokerProtocolVersion >= KAFKA_2_8_IV0
+    usesSelfManagedQuorum || (requiresZookeeper && interBrokerProtocolVersion >= KAFKA_2_8_IV0)
 
   validateValues()
 
diff --git a/core/src/main/scala/kafka/server/RaftReplicaManager.scala b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
index 143709deefe86..9766f45bcb413 100644
--- a/core/src/main/scala/kafka/server/RaftReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
@@ -251,6 +251,12 @@ class RaftReplicaManager(config: KafkaConfig,
               (Some(Partition(topicPartition, time, configRepository, this)), None)
           }
           partition.foreach { partition =>
+            builder.topicNameToId(partition.topic) match {
+              case Some(id) => partition.checkOrSetTopicId(id, true) // not sure if we should do things to handle case where topic ID is inconsistent
+              case None => throw new IllegalStateException(
+                s"Topic partition ${partition.topicPartition} is missing a topic ID"
+              )
+            }
             val isNew = priorDeferredMetadata match {
               case Some(alreadyDeferred) => alreadyDeferred.isNew
               case _ => prevPartitions.topicPartition(topicPartition.topic(), topicPartition.partition()).isEmpty
@@ -286,6 +292,12 @@ class RaftReplicaManager(config: KafkaConfig,
               Some(partition)
           }
           partition.foreach { partition =>
+            builder.topicNameToId(partition.topic) match {
+              case Some(id) => partition.checkOrSetTopicId(id, true) // not sure if we should do things to handle case where topic ID is inconsistent
+              case None => throw new IllegalStateException(
+                s"Topic partition ${partition.topicPartition} is missing a topic ID"
+              )
+            }
             if (currentState.leaderId == localBrokerId) {
               partitionsToBeLeader.put(partition, currentState)
             } else {
diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala
index 80beadea108aa..bdec6e614e5b6 100644
--- a/core/src/main/scala/kafka/server/ReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/ReplicaManager.scala
@@ -1369,7 +1369,7 @@ class ReplicaManager(val config: KafkaConfig,
               val requestLeaderEpoch = partitionState.leaderEpoch
               val requestTopicId = topicIds.get(topicPartition.topic)
 
-              if (!partition.checkOrSetTopicId(requestTopicId)) {
+              if (!partition.checkOrSetTopicId(requestTopicId, false)) {
                 responseMap.put(topicPartition, Errors.INCONSISTENT_TOPIC_ID)
               } else if (requestLeaderEpoch > currentLeaderEpoch) {
                 // If the leader epoch is valid record the epoch of the controller that made the leadership decision.
diff --git a/core/src/main/scala/kafka/server/metadata/MetadataImage.scala b/core/src/main/scala/kafka/server/metadata/MetadataImage.scala
index f4e5831e8ab1a..d38140714952b 100755
--- a/core/src/main/scala/kafka/server/metadata/MetadataImage.scala
+++ b/core/src/main/scala/kafka/server/metadata/MetadataImage.scala
@@ -47,6 +47,14 @@ case class MetadataImageBuilder(brokerId: Int,
     }
   }
 
+  def topicNameToId(topicName: String): Option[Uuid] = {
+    if (_partitionsBuilder != null) {
+      _partitionsBuilder.topicNameToId(topicName)
+    } else {
+      prevImage.topicNameToId(topicName)
+    }
+  }
+
   def controllerId(controllerId: Option[Int]): Unit = {
     _controllerId = controllerId
   }
@@ -102,7 +110,7 @@ case class MetadataImage(partitions: MetadataPartitions,
                          controllerId: Option[Int],
                          brokers: MetadataBrokers) {
   def this() = {
-    this(MetadataPartitions(Collections.emptyMap(), Collections.emptyMap()),
+    this(MetadataPartitions(Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap()),
       None,
       new MetadataBrokers(Collections.emptyList(), new util.HashMap[Integer, MetadataBroker]()))
   }
diff --git a/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala b/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala
index 96ed8a592fe05..20d88b47643ad 100644
--- a/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala
+++ b/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala
@@ -116,16 +116,21 @@ class MetadataPartitionsBuilder(val brokerId: Int,
                                 val prevPartitions: MetadataPartitions) {
   private var newNameMap = prevPartitions.copyNameMap()
   private var newIdMap = prevPartitions.copyIdMap()
+  private var newReverseIdMap = prevPartitions.copyReverseIdMap()
   private val changed = Collections.newSetFromMap[Any](new util.IdentityHashMap())
   private val _localChanged = new util.HashSet[MetadataPartition]
   private val _localRemoved = new util.HashSet[MetadataPartition]
 
   def topicIdToName(id: Uuid): Option[String] = Option(newIdMap.get(id))
 
+  def topicNameToId(name: String): Option[Uuid] = Option(newReverseIdMap.get(name))
+
   def removeTopicById(id: Uuid): Iterable[MetadataPartition] = {
     Option(newIdMap.remove(id)) match {
       case None => throw new RuntimeException(s"Unable to locate topic with ID $id")
-      case Some(name) => newNameMap.remove(name).values().asScala
+      case Some(name) =>
+        newReverseIdMap.remove(name)
+        newNameMap.remove(name).values().asScala
     }
   }
 
@@ -144,10 +149,14 @@ class MetadataPartitionsBuilder(val brokerId: Int,
 
   def addUuidMapping(name: String, id: Uuid): Unit = {
     newIdMap.put(id, name)
+    newReverseIdMap.put(name, id)
   }
 
   def removeUuidMapping(id: Uuid): Unit = {
-    newIdMap.remove(id)
+    Option(newIdMap.remove(id)) match {
+      case Some(name) => newReverseIdMap.remove(name)
+      case None =>
+    }
   }
 
   def get(topicName: String, partitionId: Int): Option[MetadataPartition] = {
@@ -204,9 +213,10 @@ class MetadataPartitionsBuilder(val brokerId: Int,
   }
 
   def build(): MetadataPartitions = {
-    val result = MetadataPartitions(newNameMap, newIdMap)
+    val result = MetadataPartitions(newNameMap, newIdMap, newReverseIdMap)
     newNameMap = Collections.unmodifiableMap(newNameMap)
     newIdMap = Collections.unmodifiableMap(newIdMap)
+    newReverseIdMap = Collections.unmodifiableMap(newReverseIdMap)
     result
   }
 
@@ -217,9 +227,9 @@ class MetadataPartitionsBuilder(val brokerId: Int,
 
 object MetadataPartitions {
   def apply(nameMap: util.Map[String, util.Map[Int, MetadataPartition]],
-            idMap: util.Map[Uuid, String]): MetadataPartitions = {
-    val reverseMap = idMap.asScala.map(_.swap).toMap.asJava
-    new MetadataPartitions(nameMap, idMap, reverseMap)
+            idMap: util.Map[Uuid, String],
+            reverseIdMap: util.Map[String, Uuid]): MetadataPartitions = {
+    new MetadataPartitions(nameMap, idMap, reverseIdMap)
   }
 }
 
@@ -243,6 +253,12 @@ case class MetadataPartitions(private val nameMap: util.Map[String, util.Map[Int
     copy
   }
 
+  def copyReverseIdMap(): util.Map[String, Uuid] = {
+    val copy = new util.HashMap[String, Uuid](reverseIdMap.size())
+    copy.putAll(reverseIdMap)
+    copy
+  }
+
   def allPartitions(): Iterator[MetadataPartition] = new AllPartitionsIterator(nameMap).asScala
 
   def allTopicNames(): collection.Set[String] = nameMap.keySet().asScala
diff --git a/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala b/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala
index a708d7381d36d..4def28d768377 100644
--- a/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala
+++ b/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala
@@ -33,7 +33,7 @@ import scala.jdk.CollectionConverters._
 @Timeout(value = 120000, unit = TimeUnit.MILLISECONDS)
 class MetadataPartitionsTest {
 
-  private val emptyPartitions = MetadataPartitions(Collections.emptyMap(), Collections.emptyMap())
+  private val emptyPartitions = MetadataPartitions(Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap())
 
   private def newPartition(topicName: String,
                            partitionIndex: Int,
diff --git a/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
index d3d6471712663..9e04c96cc2488 100644
--- a/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
@@ -23,6 +23,7 @@ import java.util.concurrent.atomic.AtomicBoolean
 
 import kafka.cluster.Partition
 import kafka.server.QuotaFactory.QuotaManagers
+import kafka.server.checkpoints.LazyOffsetCheckpoints
 import kafka.server.metadata.{CachedConfigRepository, MetadataBroker, MetadataBrokers, MetadataImage, MetadataImageBuilder, MetadataPartition, RaftMetadataCache}
 import kafka.utils.{MockScheduler, MockTime, TestUtils}
 import org.apache.kafka.common.{TopicPartition, Uuid}
@@ -131,6 +132,11 @@ class RaftReplicaManagerTest {
     val partition0 =  Partition(topicPartition0, time, configRepository, rrm)
     val partition1 =  Partition(topicPartition1, time, configRepository, rrm)
 
+    rrm.createPartition(topicPartition0).createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+    rrm.createPartition(topicPartition1).createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+
     processTopicPartitionMetadata(rrm)
     // verify changes would have been deferred
     val partitionsNewMapCaptor: ArgumentCaptor[mutable.Map[Partition, Boolean]] =
@@ -161,6 +167,9 @@ class RaftReplicaManagerTest {
 
     // leadership change callbacks
     verifyLeadershipChangeCallbacks(List(partition0), List(partition1))
+
+    // partition.metadata file
+    verifyPartitionMetadataFile(rrm, List(topicPartition0, topicPartition1))
   }
 
   @Test
@@ -169,6 +178,12 @@ class RaftReplicaManagerTest {
     rrm.delegate = mockDelegate
     val partition0 = Partition(topicPartition0, time, configRepository, rrm)
     val partition1 = Partition(topicPartition1, time, configRepository, rrm)
+
+    rrm.createPartition(topicPartition0).createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+    rrm.createPartition(topicPartition1).createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+
     rrm.endMetadataChangeDeferral(onLeadershipChange)
 
     // define some return values to avoid NPE
@@ -188,6 +203,9 @@ class RaftReplicaManagerTest {
 
     // leadership change callbacks
     verifyLeadershipChangeCallbacks(List(partition0), List(partition1))
+
+    // partition.metadata file
+    verifyPartitionMetadataFile(rrm, List(topicPartition0, topicPartition1))
   }
 
   private def verifyMakeLeaders(expectedPrevPartitionsAlreadyExisting: Set[MetadataPartition],
@@ -221,6 +239,18 @@ class RaftReplicaManagerTest {
     assertEquals(expectedUpdatedFollowers, updatedFollowersCaptor.getValue.toList)
   }
 
+  private def verifyPartitionMetadataFile(rrm: RaftReplicaManager, topicPartitions: List[TopicPartition]) = {
+    topicPartitions.foreach ( topicPartition => {
+      val log = rrm.getLog(topicPartition).get
+      assertTrue(log.partitionMetadataFile.exists())
+      val partitionMetadata = log.partitionMetadataFile.read()
+
+      // Current version of PartitionMetadataFile is 0.
+      assertEquals(0, partitionMetadata.version)
+      assertEquals(topicId, partitionMetadata.topicId)
+    })
+  }
+
   private def processTopicPartitionMetadata(raftReplicaManager: RaftReplicaManager): Unit = {
     // create brokers
     imageBuilder.brokersBuilder().add(metadataBroker0)

From 63439be7e9d192680e3a43b5ccad014b9cb598f2 Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Mon, 8 Mar 2021 15:06:34 -0800
Subject: [PATCH 2/9] Check for inconsistent topic ID

---
 .../kafka/server/RaftReplicaManager.scala     | 10 ++--
 .../kafka/server/RaftReplicaManagerTest.scala | 51 +++++++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/core/src/main/scala/kafka/server/RaftReplicaManager.scala b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
index 9766f45bcb413..29ddfcb3ebeef 100644
--- a/core/src/main/scala/kafka/server/RaftReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
@@ -27,7 +27,7 @@ import kafka.server.checkpoints.LazyOffsetCheckpoints
 import kafka.server.metadata.{ConfigRepository, MetadataImage, MetadataImageBuilder, MetadataPartition, RaftMetadataCache}
 import kafka.utils.Scheduler
 import org.apache.kafka.common.TopicPartition
-import org.apache.kafka.common.errors.KafkaStorageException
+import org.apache.kafka.common.errors.{InconsistentTopicIdException, KafkaStorageException}
 import org.apache.kafka.common.metrics.Metrics
 import org.apache.kafka.common.utils.Time
 
@@ -252,7 +252,9 @@ class RaftReplicaManager(config: KafkaConfig,
           }
           partition.foreach { partition =>
             builder.topicNameToId(partition.topic) match {
-              case Some(id) => partition.checkOrSetTopicId(id, true) // not sure if we should do things to handle case where topic ID is inconsistent
+              case Some(id) =>
+                if (!partition.checkOrSetTopicId(id, true))
+                  throw new InconsistentTopicIdException(s"Topic partition ${partition.topicPartition} had an inconsistent topic ID." )
               case None => throw new IllegalStateException(
                 s"Topic partition ${partition.topicPartition} is missing a topic ID"
               )
@@ -293,7 +295,9 @@ class RaftReplicaManager(config: KafkaConfig,
           }
           partition.foreach { partition =>
             builder.topicNameToId(partition.topic) match {
-              case Some(id) => partition.checkOrSetTopicId(id, true) // not sure if we should do things to handle case where topic ID is inconsistent
+              case Some(id) =>
+                if (!partition.checkOrSetTopicId(id, true))
+                   throw new InconsistentTopicIdException(s"Topic partition ${partition.topicPartition} had an inconsistent topic ID." )
               case None => throw new IllegalStateException(
                 s"Topic partition ${partition.topicPartition} is missing a topic ID"
               )
diff --git a/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
index 9e04c96cc2488..acb53041a245b 100644
--- a/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
@@ -26,6 +26,7 @@ import kafka.server.QuotaFactory.QuotaManagers
 import kafka.server.checkpoints.LazyOffsetCheckpoints
 import kafka.server.metadata.{CachedConfigRepository, MetadataBroker, MetadataBrokers, MetadataImage, MetadataImageBuilder, MetadataPartition, RaftMetadataCache}
 import kafka.utils.{MockScheduler, MockTime, TestUtils}
+import org.apache.kafka.common.errors.InconsistentTopicIdException
 import org.apache.kafka.common.{TopicPartition, Uuid}
 import org.apache.kafka.common.metadata.PartitionRecord
 import org.apache.kafka.common.metrics.Metrics
@@ -208,6 +209,56 @@ class RaftReplicaManagerTest {
     verifyPartitionMetadataFile(rrm, List(topicPartition0, topicPartition1))
   }
 
+  @Test
+  def testInconsistentTopicIdDefersChanges(): Unit = {
+    val rrm = createRaftReplicaManager()
+    rrm.delegate = mockDelegate
+    val partition0 = rrm.createPartition(topicPartition0)
+    val partition1 = rrm.createPartition(topicPartition1)
+
+    partition0.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+    partition1.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+
+    assertTrue(partition0.log.isDefined)
+    partition0.log.get.topicId = Uuid.randomUuid()
+
+    try {
+      processTopicPartitionMetadata(rrm)
+    } catch {
+      case e: Throwable => assertTrue(e.isInstanceOf[InconsistentTopicIdException])
+    }
+  }
+
+  @Test
+  def testInconsistentTopicIdWhenNotDeferring(): Unit = {
+    val rrm = createRaftReplicaManager()
+    rrm.delegate = mockDelegate
+    val partition0 = rrm.createPartition(topicPartition0)
+    val partition1 = rrm.createPartition(topicPartition1)
+
+    partition0.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+    partition1.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+
+    assertTrue(partition1.log.isDefined)
+    partition1.log.get.topicId = Uuid.randomUuid()
+
+    rrm.endMetadataChangeDeferral(onLeadershipChange)
+
+    // define some return values to avoid NPE
+    when(mockDelegate.makeLeaders(any(), any(), any(), ArgumentMatchers.eq(Some(offset1)))).thenReturn(Set(partition0))
+    when(mockDelegate.makeFollowers(any(), any(), any(), any(), ArgumentMatchers.eq(Some(offset1)))).thenReturn(Set(partition1))
+
+    try {
+      processTopicPartitionMetadata(rrm)
+    } catch {
+      case e: Throwable => assertTrue(e.isInstanceOf[InconsistentTopicIdException])
+    }
+  }
+
   private def verifyMakeLeaders(expectedPrevPartitionsAlreadyExisting: Set[MetadataPartition],
                                 expectedMetadataOffset: Option[Long]): mutable.Map[Partition, MetadataPartition] = {
     val leaderPartitionStatesCaptor: ArgumentCaptor[mutable.Map[Partition, MetadataPartition]] =

From be36e6c9bcc0b4e51d566d3f3d2a1706266361b1 Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Mon, 8 Mar 2021 15:32:31 -0800
Subject: [PATCH 3/9] clean up merge change

---
 .../main/scala/kafka/server/metadata/MetadataImage.scala    | 2 +-
 .../scala/kafka/server/metadata/MetadataPartitions.scala    | 6 +++---
 .../kafka/server/metadata/MetadataPartitionsTest.scala      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/core/src/main/scala/kafka/server/metadata/MetadataImage.scala b/core/src/main/scala/kafka/server/metadata/MetadataImage.scala
index d38140714952b..4687c71922067 100755
--- a/core/src/main/scala/kafka/server/metadata/MetadataImage.scala
+++ b/core/src/main/scala/kafka/server/metadata/MetadataImage.scala
@@ -110,7 +110,7 @@ case class MetadataImage(partitions: MetadataPartitions,
                          controllerId: Option[Int],
                          brokers: MetadataBrokers) {
   def this() = {
-    this(MetadataPartitions(Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap()),
+    this(MetadataPartitions(Collections.emptyMap(), Collections.emptyMap()),
       None,
       new MetadataBrokers(Collections.emptyList(), new util.HashMap[Integer, MetadataBroker]()))
   }
diff --git a/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala b/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala
index 4bdf12c98b64a..d6b3b1bf4fee9 100644
--- a/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala
+++ b/core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala
@@ -264,9 +264,9 @@ class MetadataPartitionsBuilder(val brokerId: Int,
 
 object MetadataPartitions {
   def apply(nameMap: util.Map[String, util.Map[Int, MetadataPartition]],
-            idMap: util.Map[Uuid, String],
-            reverseIdMap: util.Map[String, Uuid]): MetadataPartitions = {
-    new MetadataPartitions(nameMap, idMap, reverseIdMap)
+            idMap: util.Map[Uuid, String]): MetadataPartitions = {
+    val reverseMap = idMap.asScala.map(_.swap).toMap.asJava
+    new MetadataPartitions(nameMap, idMap, reverseMap)
   }
 }
 
diff --git a/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala b/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala
index 2b6bf4b363b94..fcd092597efbb 100644
--- a/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala
+++ b/core/src/test/scala/kafka/server/metadata/MetadataPartitionsTest.scala
@@ -32,7 +32,7 @@ import scala.jdk.CollectionConverters._
 @Timeout(value = 120000, unit = TimeUnit.MILLISECONDS)
 class MetadataPartitionsTest {
 
-  private val emptyPartitions = MetadataPartitions(Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap())
+  private val emptyPartitions = MetadataPartitions(Collections.emptyMap(), Collections.emptyMap())
 
   private def newPartition(topicName: String,
                            partitionIndex: Int,

From 5e3372af52d036b24a9cb599606038ce92809727 Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Tue, 9 Mar 2021 10:30:09 -0800
Subject: [PATCH 4/9] Cleaned style as recommended by review

---
 .../main/scala/kafka/cluster/Partition.scala  |  4 +--
 .../main/scala/kafka/server/KafkaConfig.scala |  2 +-
 .../kafka/server/RaftReplicaManager.scala     | 31 +++++++++----------
 .../scala/kafka/server/ReplicaManager.scala   |  2 +-
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala
index 8f7a5dd314359..5ea5b2c2961c8 100755
--- a/core/src/main/scala/kafka/cluster/Partition.scala
+++ b/core/src/main/scala/kafka/cluster/Partition.scala
@@ -429,13 +429,13 @@ class Partition(val topicPartition: TopicPartition,
    * If a valid topic ID is provided, and the log exists but has no ID set, set the log ID to be the request ID.
    *
    * @param receivedTopicId the topic ID from the LeaderAndIsr request or from the metadata records
-   * @return true if the request topic id is consistent, false otherwise
+   * @return true if the received topic id is consistent, false otherwise
    */
   def checkOrSetTopicId(receivedTopicId: Uuid, usingRaft: Boolean): Boolean = {
     // If the request had an invalid topic ID, then we assume that topic IDs are not supported so ID is consistent.
     // This is only the case when from LeaderAndIsr Request. Raft code should never have invalid topic IDs.
     if (receivedTopicId == null || receivedTopicId == Uuid.ZERO_UUID) {
-      if (usingRaft) false else true
+      !usingRaft // only acceptable when not using Raft
     } else {
       log match {
         case None => true // log is empty, so we can not say topic ID is inconsistent
diff --git a/core/src/main/scala/kafka/server/KafkaConfig.scala b/core/src/main/scala/kafka/server/KafkaConfig.scala
index 3171a10542b03..c9d97912a9360 100755
--- a/core/src/main/scala/kafka/server/KafkaConfig.scala
+++ b/core/src/main/scala/kafka/server/KafkaConfig.scala
@@ -1894,7 +1894,7 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
 
   // Topic IDs are used with all self-managed quorum clusters and ZK cluster with IBP greater than or equal to 2.8
   def usesTopicId: Boolean =
-    usesSelfManagedQuorum || (requiresZookeeper && interBrokerProtocolVersion >= KAFKA_2_8_IV0)
+    usesSelfManagedQuorum || interBrokerProtocolVersion >= KAFKA_2_8_IV0
 
   validateValues()
 
diff --git a/core/src/main/scala/kafka/server/RaftReplicaManager.scala b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
index 29ddfcb3ebeef..c580f96e4e6e5 100644
--- a/core/src/main/scala/kafka/server/RaftReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
@@ -26,7 +26,7 @@ import kafka.server.QuotaFactory.QuotaManagers
 import kafka.server.checkpoints.LazyOffsetCheckpoints
 import kafka.server.metadata.{ConfigRepository, MetadataImage, MetadataImageBuilder, MetadataPartition, RaftMetadataCache}
 import kafka.utils.Scheduler
-import org.apache.kafka.common.TopicPartition
+import org.apache.kafka.common.{TopicPartition, Uuid}
 import org.apache.kafka.common.errors.{InconsistentTopicIdException, KafkaStorageException}
 import org.apache.kafka.common.metrics.Metrics
 import org.apache.kafka.common.utils.Time
@@ -251,14 +251,7 @@ class RaftReplicaManager(config: KafkaConfig,
               (Some(Partition(topicPartition, time, configRepository, this)), None)
           }
           partition.foreach { partition =>
-            builder.topicNameToId(partition.topic) match {
-              case Some(id) =>
-                if (!partition.checkOrSetTopicId(id, true))
-                  throw new InconsistentTopicIdException(s"Topic partition ${partition.topicPartition} had an inconsistent topic ID." )
-              case None => throw new IllegalStateException(
-                s"Topic partition ${partition.topicPartition} is missing a topic ID"
-              )
-            }
+            checkOrSetTopicId(builder.topicNameToId(partition.topic), partition)
             val isNew = priorDeferredMetadata match {
               case Some(alreadyDeferred) => alreadyDeferred.isNew
               case _ => prevPartitions.topicPartition(topicPartition.topic(), topicPartition.partition()).isEmpty
@@ -294,14 +287,7 @@ class RaftReplicaManager(config: KafkaConfig,
               Some(partition)
           }
           partition.foreach { partition =>
-            builder.topicNameToId(partition.topic) match {
-              case Some(id) =>
-                if (!partition.checkOrSetTopicId(id, true))
-                   throw new InconsistentTopicIdException(s"Topic partition ${partition.topicPartition} had an inconsistent topic ID." )
-              case None => throw new IllegalStateException(
-                s"Topic partition ${partition.topicPartition} is missing a topic ID"
-              )
-            }
+            checkOrSetTopicId(builder.topicNameToId(partition.topic), partition)
             if (currentState.leaderId == localBrokerId) {
               partitionsToBeLeader.put(partition, currentState)
             } else {
@@ -392,4 +378,15 @@ class RaftReplicaManager(config: KafkaConfig,
     metadataImage.partitions.topicPartition(partition.topic, partition.partitionId).getOrElse(
       throw new IllegalStateException(s"Partition has metadata changes but does not exist in the metadata cache: ${partition.topicPartition}"))
   }
+
+  private def checkOrSetTopicId(topicIdOpt: Option[Uuid], partition: Partition)= {
+     topicIdOpt match {
+      case Some(id) =>
+        if (!partition.checkOrSetTopicId(id, usingRaft = true))
+          throw new InconsistentTopicIdException(s"Topic partition ${partition.topicPartition} had an inconsistent topic ID." )
+      case None => throw new IllegalStateException(
+        s"Topic partition ${partition.topicPartition} is missing a topic ID"
+      )
+    }
+  }
 }
diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala
index bdec6e614e5b6..94658a4614575 100644
--- a/core/src/main/scala/kafka/server/ReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/ReplicaManager.scala
@@ -1369,7 +1369,7 @@ class ReplicaManager(val config: KafkaConfig,
               val requestLeaderEpoch = partitionState.leaderEpoch
               val requestTopicId = topicIds.get(topicPartition.topic)
 
-              if (!partition.checkOrSetTopicId(requestTopicId, false)) {
+              if (!partition.checkOrSetTopicId(requestTopicId, usingRaft = false)) {
                 responseMap.put(topicPartition, Errors.INCONSISTENT_TOPIC_ID)
               } else if (requestLeaderEpoch > currentLeaderEpoch) {
                 // If the leader epoch is valid record the epoch of the controller that made the leadership decision.

From 51b8f7ed384219d4b882eaf77a69267b20622597 Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Mon, 15 Mar 2021 10:16:28 -0700
Subject: [PATCH 5/9] Fixed bug that prevented writing to partition.metadata
 file on first pass as well as reading from the log on the first pass.

---
 .../main/scala/kafka/cluster/Partition.scala  |  69 ++++------
 core/src/main/scala/kafka/log/Log.scala       |  33 +++--
 .../src/main/scala/kafka/log/LogManager.scala |   7 +-
 .../server/RaftReplicaChangeDelegate.scala    |  14 ++-
 .../kafka/server/RaftReplicaManager.scala     |  46 ++++---
 .../scala/kafka/server/ReplicaManager.scala   |  71 +++++++++--
 .../kafka/cluster/PartitionLockTest.scala     |   4 +-
 .../unit/kafka/cluster/PartitionTest.scala    |  96 +++++++++++++-
 .../test/scala/unit/kafka/log/LogTest.scala   |  37 ++++--
 .../server/BrokerEpochIntegrationTest.scala   |   4 +-
 .../RaftReplicaChangeDelegateTest.scala       |  15 ++-
 .../kafka/server/RaftReplicaManagerTest.scala |  56 ++++-----
 .../kafka/server/ReplicaManagerTest.scala     | 118 +++++++++++++-----
 .../ReplicaFetcherThreadBenchmark.java        |   4 +-
 .../PartitionMakeFollowerBenchmark.java       |   9 +-
 .../UpdateFollowerFetchStateBenchmark.java    |   6 +-
 .../kafka/jmh/server/CheckpointBench.java     |   2 +-
 17 files changed, 418 insertions(+), 173 deletions(-)

diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala
index 5ea5b2c2961c8..2f1d0b521450e 100755
--- a/core/src/main/scala/kafka/cluster/Partition.scala
+++ b/core/src/main/scala/kafka/cluster/Partition.scala
@@ -308,27 +308,29 @@ class Partition(val topicPartition: TopicPartition,
                 s"different from the requested log dir $logDir")
             false
           case None =>
-            createLogIfNotExists(isNew = false, isFutureReplica = true, highWatermarkCheckpoints)
+            // not sure if topic ID should be none here, but not sure if we have access in ReplicaManager where this is called.
+            // could also use topicId method here potentially. This is only used in ReplicaManager (ZK code) so probably ok to set as None.
+            createLogIfNotExists(isNew = false, isFutureReplica = true, highWatermarkCheckpoints, None)
             true
         }
       }
     }
   }
 
-  def createLogIfNotExists(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints): Unit = {
+  def createLogIfNotExists(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Unit = {
     isFutureReplica match {
       case true if futureLog.isEmpty =>
-        val log = createLog(isNew, isFutureReplica, offsetCheckpoints)
+        val log = createLog(isNew, isFutureReplica, offsetCheckpoints, topicId)
         this.futureLog = Option(log)
       case false if log.isEmpty =>
-        val log = createLog(isNew, isFutureReplica, offsetCheckpoints)
+        val log = createLog(isNew, isFutureReplica, offsetCheckpoints, topicId)
         this.log = Option(log)
       case _ => trace(s"${if (isFutureReplica) "Future Log" else "Log"} already exists.")
     }
   }
 
   // Visible for testing
-  private[cluster] def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints): Log = {
+  private[cluster] def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Log = {
     def updateHighWatermark(log: Log) = {
       val checkpointHighWatermark = offsetCheckpoints.fetch(log.parentDir, topicPartition).getOrElse {
         info(s"No checkpointed highwatermark is found for partition $topicPartition")
@@ -341,7 +343,7 @@ class Partition(val topicPartition: TopicPartition,
     logManager.initializingLog(topicPartition)
     var maybeLog: Option[Log] = None
     try {
-      val log = logManager.getOrCreateLog(topicPartition, isNew, isFutureReplica)
+      val log = logManager.getOrCreateLog(topicPartition, isNew, isFutureReplica, topicId)
       maybeLog = Some(log)
       updateHighWatermark(log)
       log
@@ -425,40 +427,19 @@ class Partition(val topicPartition: TopicPartition,
   }
 
   /**
-   * Checks if the topic ID received is consistent with the topic ID in the log.
-   * If a valid topic ID is provided, and the log exists but has no ID set, set the log ID to be the request ID.
-   *
-   * @param receivedTopicId the topic ID from the LeaderAndIsr request or from the metadata records
-   * @return true if the received topic id is consistent, false otherwise
+   * @return the topic ID for the log or None if the log or the topic ID does not exist.
    */
-  def checkOrSetTopicId(receivedTopicId: Uuid, usingRaft: Boolean): Boolean = {
-    // If the request had an invalid topic ID, then we assume that topic IDs are not supported so ID is consistent.
-    // This is only the case when from LeaderAndIsr Request. Raft code should never have invalid topic IDs.
-    if (receivedTopicId == null || receivedTopicId == Uuid.ZERO_UUID) {
-      !usingRaft // only acceptable when not using Raft
-    } else {
-      log match {
-        case None => true // log is empty, so we can not say topic ID is inconsistent
-        case Some(log) => {
-          // Check if topic ID is in memory, if not, it must be new to the broker and does not have a metadata file.
-          // This is because if the broker previously wrote it to file, it would be recovered on restart after failure.
-          // Topic ID is consistent since we are just setting it here.
-          if (log.topicId == Uuid.ZERO_UUID) {
-            log.partitionMetadataFile.write(receivedTopicId)
-            log.topicId = receivedTopicId
-            true
-          } else if (log.topicId != receivedTopicId) {
-            stateChangeLogger.error(s"Topic Id in memory: ${log.topicId} does not" +
-              s" match the topic Id for partition $topicPartition provided in the " +
-              s"${if (usingRaft) "metadata records" else "request"}: $receivedTopicId.")
-            false
-          } else {
-            // topic ID in log exists and matches received topic ID
-            true
-          }
-        }
-      }
-    }
+  def topicId: Option[Uuid] = {
+    val log = this.log.orElse(logManager.getLog(topicPartition))
+    log.flatMap(_.topicId)
+  }
+
+  /**
+   * @return the topic ID and the partition's log. Return None for values that do not exist
+   */
+  def topicIdAndLog: (Option[Uuid], Option[Log]) = {
+    val log = this.log.orElse(logManager.getLog(topicPartition))
+    (log.flatMap(_.topicId), log)
   }
 
   // remoteReplicas will be called in the hot path, and must be inexpensive
@@ -539,7 +520,8 @@ class Partition(val topicPartition: TopicPartition,
    * If the leader replica id does not change, return false to indicate the replica manager.
    */
   def makeLeader(partitionState: LeaderAndIsrPartitionState,
-                 highWatermarkCheckpoints: OffsetCheckpoints): Boolean = {
+                 highWatermarkCheckpoints: OffsetCheckpoints,
+                 topicId: Option[Uuid] = None): Boolean = {
     val (leaderHWIncremented, isNewLeader) = inWriteLock(leaderIsrUpdateLock) {
       // record the epoch of the controller that made the leadership decision. This is useful while updating the isr
       // to maintain the decision maker controller's epoch in the zookeeper path
@@ -556,7 +538,7 @@ class Partition(val topicPartition: TopicPartition,
         removingReplicas = removingReplicas
       )
       try {
-        createLogIfNotExists(partitionState.isNew, isFutureReplica = false, highWatermarkCheckpoints)
+        createLogIfNotExists(partitionState.isNew, isFutureReplica = false, highWatermarkCheckpoints, topicId)
       } catch {
         case e: ZooKeeperClientException =>
           stateChangeLogger.error(s"A ZooKeeper client exception has occurred and makeLeader will be skipping the " +
@@ -623,7 +605,8 @@ class Partition(val topicPartition: TopicPartition,
    * replica manager that state is already correct and the become-follower steps can be skipped
    */
   def makeFollower(partitionState: LeaderAndIsrPartitionState,
-                   highWatermarkCheckpoints: OffsetCheckpoints): Boolean = {
+                   highWatermarkCheckpoints: OffsetCheckpoints,
+                   topicId: Option[Uuid] = None): Boolean = {
     inWriteLock(leaderIsrUpdateLock) {
       val newLeaderBrokerId = partitionState.leader
       val oldLeaderEpoch = leaderEpoch
@@ -638,7 +621,7 @@ class Partition(val topicPartition: TopicPartition,
         removingReplicas = partitionState.removingReplicas.asScala.map(_.toInt)
       )
       try {
-        createLogIfNotExists(partitionState.isNew, isFutureReplica = false, highWatermarkCheckpoints)
+        createLogIfNotExists(partitionState.isNew, isFutureReplica = false, highWatermarkCheckpoints, topicId)
       } catch {
         case e: ZooKeeperClientException =>
           stateChangeLogger.error(s"A ZooKeeper client exception has occurred. makeFollower will be skipping the " +
diff --git a/core/src/main/scala/kafka/log/Log.scala b/core/src/main/scala/kafka/log/Log.scala
index 0ad82f46b9d4b..7d85a8deb4c53 100644
--- a/core/src/main/scala/kafka/log/Log.scala
+++ b/core/src/main/scala/kafka/log/Log.scala
@@ -241,12 +241,16 @@ case object SnapshotGenerated extends LogStartOffsetIncrementReason {
  * @param producerIdExpirationCheckIntervalMs How often to check for producer ids which need to be expired
  * @param hadCleanShutdown boolean flag to indicate if the Log had a clean/graceful shutdown last time. true means
  *                         clean shutdown whereas false means a crash.
+ * @param topicId optional Uuid to specify the topic ID for the topic if it exists. Should only be specified when
+ *                first creating the log through Partition.makeLeader or Partition.makeFollower. When reloading a log,
+ *                this field will be populated by reading the topic ID value from partition.metadata if it exists.
  * @param keepPartitionMetadataFile boolean flag to indicate whether the partition.metadata file should be kept in the
- *                                  log directory. A partition.metadata file is only created when the controller's
- *                                  inter-broker protocol version is at least 2.8. This file will persist the topic ID on
- *                                  the broker. If inter-broker protocol is downgraded below 2.8, a topic ID may be lost
- *                                  and a new ID generated upon re-upgrade. If the inter-broker protocol version is below
- *                                  2.8, partition.metadata will be deleted to avoid ID conflicts upon re-upgrade.
+ *                                  log directory. A partition.metadata file is only created when the raft controller is used
+ *                                  or the ZK controller's inter-broker protocol version is at least 2.8.
+ *                                  This file will persist the topic ID on the broker. If inter-broker protocol for a ZK controller
+ *                                  is downgraded below 2.8, a topic ID may be lost and a new ID generated upon re-upgrade.
+ *                                  If the inter-broker protocol version on a ZK cluster is below 2.8, partition.metadata
+ *                                  will be deleted to avoid ID conflicts upon re-upgrade.
  */
 @threadsafe
 class Log(@volatile private var _dir: File,
@@ -262,6 +266,7 @@ class Log(@volatile private var _dir: File,
           val producerStateManager: ProducerStateManager,
           logDirFailureChannel: LogDirFailureChannel,
           private val hadCleanShutdown: Boolean = true,
+          @volatile var topicId : Option[Uuid] = None,
           val keepPartitionMetadataFile: Boolean = true) extends Logging with KafkaMetricsGroup {
 
   import kafka.log.Log._
@@ -315,8 +320,6 @@ class Log(@volatile private var _dir: File,
 
   @volatile var partitionMetadataFile : PartitionMetadataFile = null
 
-  @volatile var topicId : Uuid = Uuid.ZERO_UUID
-
   locally {
     // create the log directory if it doesn't exist
     Files.createDirectories(dir.toPath)
@@ -349,11 +352,20 @@ class Log(@volatile private var _dir: File,
 
     // Delete partition metadata file if the version does not support topic IDs.
     // Recover topic ID if present and topic IDs are supported
+    // If we were provided a topic ID when creating the log, partition metadata files are supported, and one does not yet exist
+    // write to the partition metadata file.
+    // Ensure we do not try to assign a provided topicId that is inconsistent with the ID on file.
     if (partitionMetadataFile.exists()) {
         if (!keepPartitionMetadataFile)
           partitionMetadataFile.delete()
-        else
-          topicId = partitionMetadataFile.read().topicId
+        else {
+          val fileTopicId = partitionMetadataFile.read().topicId
+          if (topicId.isDefined && fileTopicId != topicId.get)
+            throw new IllegalStateException(s"Tried to assign topic ID $topicId to log, but log already contained topicId $fileTopicId")
+          topicId = Some(fileTopicId)
+        }
+    } else if (topicId.isDefined && keepPartitionMetadataFile) {
+      partitionMetadataFile.write(topicId.get)
     }
   }
 
@@ -2600,11 +2612,12 @@ object Log {
             producerIdExpirationCheckIntervalMs: Int,
             logDirFailureChannel: LogDirFailureChannel,
             lastShutdownClean: Boolean = true,
+            topicId: Option[Uuid] = None,
             keepPartitionMetadataFile: Boolean = true): Log = {
     val topicPartition = Log.parseTopicPartitionName(dir)
     val producerStateManager = new ProducerStateManager(topicPartition, dir, maxProducerIdExpirationMs)
     new Log(dir, config, logStartOffset, recoveryPoint, scheduler, brokerTopicStats, time, maxProducerIdExpirationMs,
-      producerIdExpirationCheckIntervalMs, topicPartition, producerStateManager, logDirFailureChannel, lastShutdownClean, keepPartitionMetadataFile)
+      producerIdExpirationCheckIntervalMs, topicPartition, producerStateManager, logDirFailureChannel, lastShutdownClean, topicId, keepPartitionMetadataFile)
   }
 
   /**
diff --git a/core/src/main/scala/kafka/log/LogManager.scala b/core/src/main/scala/kafka/log/LogManager.scala
index 1ca4d7ec33f2c..6b1a17ff4f950 100755
--- a/core/src/main/scala/kafka/log/LogManager.scala
+++ b/core/src/main/scala/kafka/log/LogManager.scala
@@ -27,7 +27,7 @@ import kafka.server.checkpoints.OffsetCheckpointFile
 import kafka.server.metadata.ConfigRepository
 import kafka.server._
 import kafka.utils._
-import org.apache.kafka.common.{KafkaException, TopicPartition}
+import org.apache.kafka.common.{KafkaException, TopicPartition, Uuid}
 import org.apache.kafka.common.utils.Time
 import org.apache.kafka.common.errors.{KafkaStorageException, LogDirNotFoundException}
 
@@ -773,12 +773,12 @@ class LogManager(logDirs: Seq[File],
    * Otherwise throw KafkaStorageException
    *
    * @param topicPartition The partition whose log needs to be returned or created
-   * @param loadConfig A function to retrieve the log config, this is only called if the log is created
    * @param isNew Whether the replica should have existed on the broker or not
    * @param isFuture True if the future log of the specified partition should be returned or created
+   * @param topicId The topic ID of the topic used in the case of log creation.
    * @throws KafkaStorageException if isNew=false, log is not found in the cache and there is offline log directory on the broker
    */
-  def getOrCreateLog(topicPartition: TopicPartition, isNew: Boolean = false, isFuture: Boolean = false): Log = {
+  def getOrCreateLog(topicPartition: TopicPartition, isNew: Boolean = false, isFuture: Boolean = false, topicId: Option[Uuid] = None): Log = {
     logCreationOrDeletionLock synchronized {
       getLog(topicPartition, isFuture).getOrElse {
         // create the log if it has not already been created in another thread
@@ -827,6 +827,7 @@ class LogManager(logDirs: Seq[File],
           time = time,
           brokerTopicStats = brokerTopicStats,
           logDirFailureChannel = logDirFailureChannel,
+          topicId = topicId,
           keepPartitionMetadataFile = keepPartitionMetadataFile)
 
         if (isFuture)
diff --git a/core/src/main/scala/kafka/server/RaftReplicaChangeDelegate.scala b/core/src/main/scala/kafka/server/RaftReplicaChangeDelegate.scala
index 1bf21e074069d..beb4eb52c3d4a 100644
--- a/core/src/main/scala/kafka/server/RaftReplicaChangeDelegate.scala
+++ b/core/src/main/scala/kafka/server/RaftReplicaChangeDelegate.scala
@@ -22,7 +22,7 @@ import kafka.log.Log
 import kafka.server.checkpoints.OffsetCheckpoints
 import kafka.server.metadata.{MetadataBrokers, MetadataPartition}
 import kafka.utils.Implicits.MapExtensionMethods
-import org.apache.kafka.common.TopicPartition
+import org.apache.kafka.common.{TopicPartition, Uuid}
 import org.apache.kafka.common.errors.KafkaStorageException
 
 import scala.collection.{Map, Set, mutable}
@@ -72,7 +72,8 @@ class RaftReplicaChangeDelegate(helper: RaftReplicaChangeDelegateHelper) {
   def makeLeaders(prevPartitionsAlreadyExisting: Set[MetadataPartition],
                   partitionStates: Map[Partition, MetadataPartition],
                   highWatermarkCheckpoints: OffsetCheckpoints,
-                  metadataOffset: Option[Long]): Set[Partition] = {
+                  metadataOffset: Option[Long],
+                  topicIds: String => Option[Uuid]): Set[Partition] = {
     val partitionsMadeLeaders = mutable.Set[Partition]()
     val traceLoggingEnabled = helper.stateChangeLogger.isTraceEnabled
     val deferredBatches = metadataOffset.isEmpty
@@ -94,7 +95,7 @@ class RaftReplicaChangeDelegate(helper: RaftReplicaChangeDelegateHelper) {
         try {
           val isrState = state.toLeaderAndIsrPartitionState(
             !prevPartitionsAlreadyExisting(state))
-          if (partition.makeLeader(isrState, highWatermarkCheckpoints)) {
+          if (partition.makeLeader(isrState, highWatermarkCheckpoints, topicIds(partition.topic))) {
             partitionsMadeLeaders += partition
             if (traceLoggingEnabled) {
               helper.stateChangeLogger.trace(s"$partitionLogMsgPrefix: completed the become-leader state change.")
@@ -125,7 +126,8 @@ class RaftReplicaChangeDelegate(helper: RaftReplicaChangeDelegateHelper) {
                     currentBrokers: MetadataBrokers,
                     partitionStates: Map[Partition, MetadataPartition],
                     highWatermarkCheckpoints: OffsetCheckpoints,
-                    metadataOffset: Option[Long]): Set[Partition] = {
+                    metadataOffset: Option[Long],
+                    topicIds: String => Option[Uuid]): Set[Partition] = {
     val traceLoggingEnabled = helper.stateChangeLogger.isTraceEnabled
     val deferredBatches = metadataOffset.isEmpty
     val topLevelLogPrefix = if (deferredBatches)
@@ -164,10 +166,10 @@ class RaftReplicaChangeDelegate(helper: RaftReplicaChangeDelegateHelper) {
               s"since the new leader ${state.leaderId} is unavailable.")
             // Create the local replica even if the leader is unavailable. This is required to ensure that we include
             // the partition's high watermark in the checkpoint file (see KAFKA-1647)
-            partition.createLogIfNotExists(isNew, isFutureReplica = false, highWatermarkCheckpoints)
+            partition.createLogIfNotExists(isNew, isFutureReplica = false, highWatermarkCheckpoints, topicIds(partition.topic))
           } else {
             val isrState = state.toLeaderAndIsrPartitionState(isNew)
-            if (partition.makeFollower(isrState, highWatermarkCheckpoints)) {
+            if (partition.makeFollower(isrState, highWatermarkCheckpoints, topicIds(partition.topic))) {
               partitionsMadeFollower += partition
               if (traceLoggingEnabled) {
                 helper.stateChangeLogger.trace(s"$partitionLogMsgPrefix: completed the " +
diff --git a/core/src/main/scala/kafka/server/RaftReplicaManager.scala b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
index c580f96e4e6e5..d1d9eeb8d0610 100644
--- a/core/src/main/scala/kafka/server/RaftReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
@@ -157,11 +157,11 @@ class RaftReplicaManager(config: KafkaConfig,
         }
 
         val partitionsMadeLeader = if (leaderPartitionStates.nonEmpty)
-          delegate.makeLeaders(partitionsAlreadyExisting, leaderPartitionStates, highWatermarkCheckpoints, None)
+          delegate.makeLeaders(partitionsAlreadyExisting, leaderPartitionStates, highWatermarkCheckpoints, None, metadataImage.topicNameToId)
         else
           Set.empty[Partition]
         val partitionsMadeFollower = if (followerPartitionStates.nonEmpty)
-          delegate.makeFollowers(partitionsAlreadyExisting, brokers, followerPartitionStates, highWatermarkCheckpoints, None)
+          delegate.makeFollowers(partitionsAlreadyExisting, brokers, followerPartitionStates, highWatermarkCheckpoints, None, metadataImage.topicNameToId)
         else
           Set.empty[Partition]
 
@@ -173,7 +173,7 @@ class RaftReplicaManager(config: KafkaConfig,
 
         updateLeaderAndFollowerMetrics(partitionsMadeFollower.map(_.topic).toSet)
 
-        maybeAddLogDirFetchers(partitionsMadeFollower, highWatermarkCheckpoints)
+        maybeAddLogDirFetchers(partitionsMadeFollower, highWatermarkCheckpoints, metadataImage.topicNameToId)
 
         replicaFetcherManager.shutdownIdleFetcherThreads()
         replicaAlterLogDirsManager.shutdownIdleFetcherThreads()
@@ -251,7 +251,7 @@ class RaftReplicaManager(config: KafkaConfig,
               (Some(Partition(topicPartition, time, configRepository, this)), None)
           }
           partition.foreach { partition =>
-            checkOrSetTopicId(builder.topicNameToId(partition.topic), partition)
+            checkTopicId(builder.topicNameToId(partition.topic), partition.topicId, partition.topicPartition)
             val isNew = priorDeferredMetadata match {
               case Some(alreadyDeferred) => alreadyDeferred.isNew
               case _ => prevPartitions.topicPartition(topicPartition.topic(), topicPartition.partition()).isEmpty
@@ -287,7 +287,7 @@ class RaftReplicaManager(config: KafkaConfig,
               Some(partition)
           }
           partition.foreach { partition =>
-            checkOrSetTopicId(builder.topicNameToId(partition.topic), partition)
+            checkTopicId(builder.topicNameToId(partition.topic), partition.topicId, partition.topicPartition)
             if (currentState.leaderId == localBrokerId) {
               partitionsToBeLeader.put(partition, currentState)
             } else {
@@ -305,12 +305,12 @@ class RaftReplicaManager(config: KafkaConfig,
         val highWatermarkCheckpoints = new LazyOffsetCheckpoints(this.highWatermarkCheckpoints)
         val partitionsBecomeLeader = if (partitionsToBeLeader.nonEmpty)
           delegate.makeLeaders(changedPartitionsPreviouslyExisting, partitionsToBeLeader, highWatermarkCheckpoints,
-            Some(metadataOffset))
+            Some(metadataOffset), builder.topicNameToId)
         else
           Set.empty[Partition]
         val partitionsBecomeFollower = if (partitionsToBeFollower.nonEmpty)
           delegate.makeFollowers(changedPartitionsPreviouslyExisting, nextBrokers, partitionsToBeFollower, highWatermarkCheckpoints,
-            Some(metadataOffset))
+            Some(metadataOffset), builder.topicNameToId)
         else
           Set.empty[Partition]
         updateLeaderAndFollowerMetrics(partitionsBecomeFollower.map(_.topic).toSet)
@@ -328,7 +328,7 @@ class RaftReplicaManager(config: KafkaConfig,
           }
         }
 
-        maybeAddLogDirFetchers(partitionsBecomeFollower, highWatermarkCheckpoints)
+        maybeAddLogDirFetchers(partitionsBecomeFollower, highWatermarkCheckpoints, builder.topicNameToId)
 
         replicaFetcherManager.shutdownIdleFetcherThreads()
         replicaAlterLogDirsManager.shutdownIdleFetcherThreads()
@@ -379,14 +379,30 @@ class RaftReplicaManager(config: KafkaConfig,
       throw new IllegalStateException(s"Partition has metadata changes but does not exist in the metadata cache: ${partition.topicPartition}"))
   }
 
-  private def checkOrSetTopicId(topicIdOpt: Option[Uuid], partition: Partition)= {
-     topicIdOpt match {
-      case Some(id) =>
-        if (!partition.checkOrSetTopicId(id, usingRaft = true))
-          throw new InconsistentTopicIdException(s"Topic partition ${partition.topicPartition} had an inconsistent topic ID." )
+  /**
+   * Checks if the topic ID received from the MetadataPartitionsBuilder is consistent with the topic ID in the log.
+   * If the log does not exist, logTopicIdOpt will be None. In this case, the ID is not inconsistent.
+   *
+   * @param receivedTopicIdOpt the topic ID received from the MetadataRecords if it exists
+   * @param logTopicIdOpt the topic ID in the log if the log exists
+   * @param topicPartition the topicPartition for the Partition being checked
+   * @throws InconsistentTopicIdException if the topic ids are not consistent
+   * @throws IllegalArgumentException if the MetadataPartitionsBuilder did not have a topic ID associated with the topic
+   */
+  private def checkTopicId(receivedTopicIdOpt: Option[Uuid], logTopicIdOpt: Option[Uuid], topicPartition: TopicPartition): Unit = {
+    receivedTopicIdOpt match {
+      case Some(receivedTopicId) =>
+        logTopicIdOpt.foreach(logTopicId => {
+          if (receivedTopicId != logTopicId) {
+            // not sure if we need both the logger and the error thrown
+            stateChangeLogger.error(s"Topic Id in memory: $logTopicId does not" +
+              s" match the topic Id for partition $topicPartition received: " +
+              s"$receivedTopicId.")
+            throw new InconsistentTopicIdException(s"Topic partition $topicPartition had an inconsistent topic ID.")
+          }
+        })
       case None => throw new IllegalStateException(
-        s"Topic partition ${partition.topicPartition} is missing a topic ID"
-      )
+        s"Topic partition $topicPartition is missing a topic ID")
     }
   }
 }
diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala
index 94658a4614575..a4b0133af45fe 100644
--- a/core/src/main/scala/kafka/server/ReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/ReplicaManager.scala
@@ -37,7 +37,7 @@ import kafka.server.metadata.ConfigRepository
 import kafka.utils._
 import kafka.utils.Implicits._
 import kafka.zk.KafkaZkClient
-import org.apache.kafka.common.{ElectionType, IsolationLevel, Node, TopicPartition}
+import org.apache.kafka.common.{ElectionType, IsolationLevel, Node, TopicPartition, Uuid}
 import org.apache.kafka.common.errors._
 import org.apache.kafka.common.internals.Topic
 import org.apache.kafka.common.message.LeaderAndIsrRequestData.LeaderAndIsrPartitionState
@@ -1326,6 +1326,14 @@ class ReplicaManager(val config: KafkaConfig,
             s"epoch ${leaderAndIsrRequest.controllerEpoch}")
         }
       val topicIds = leaderAndIsrRequest.topicIds()
+      def getTopicId (topicName: String): Option[Uuid] = {
+        val topicId = topicIds.get(topicName)
+        // if invalid topic ID return None
+        if (topicId == null || topicId == Uuid.ZERO_UUID)
+          None
+        else
+          Some(topicId)
+      }
 
       val response = {
         if (leaderAndIsrRequest.controllerEpoch < controllerEpoch) {
@@ -1369,7 +1377,11 @@ class ReplicaManager(val config: KafkaConfig,
               val requestLeaderEpoch = partitionState.leaderEpoch
               val requestTopicId = topicIds.get(topicPartition.topic)
 
-              if (!partition.checkOrSetTopicId(requestTopicId, usingRaft = false)) {
+              val (topicIdOpt, partitionLogOpt) = partition.topicIdAndLog
+              if (!checkTopicId(requestTopicId, topicIdOpt, partitionLogOpt)) {
+                stateChangeLogger.error(s"Topic Id in memory: ${partition.topicId.get} does not" +
+                  s" match the topic Id for partition $topicPartition received: " +
+                  s"$requestTopicId.")
                 responseMap.put(topicPartition, Errors.INCONSISTENT_TOPIC_ID)
               } else if (requestLeaderEpoch > currentLeaderEpoch) {
                 // If the leader epoch is valid record the epoch of the controller that made the leadership decision.
@@ -1407,12 +1419,12 @@ class ReplicaManager(val config: KafkaConfig,
           val highWatermarkCheckpoints = new LazyOffsetCheckpoints(this.highWatermarkCheckpoints)
           val partitionsBecomeLeader = if (partitionsToBeLeader.nonEmpty)
             makeLeaders(controllerId, controllerEpoch, partitionsToBeLeader, correlationId, responseMap,
-              highWatermarkCheckpoints)
+              highWatermarkCheckpoints, getTopicId)
           else
             Set.empty[Partition]
           val partitionsBecomeFollower = if (partitionsToBeFollower.nonEmpty)
             makeFollowers(controllerId, controllerEpoch, partitionsToBeFollower, correlationId, responseMap,
-              highWatermarkCheckpoints)
+              highWatermarkCheckpoints, getTopicId)
           else
             Set.empty[Partition]
 
@@ -1435,7 +1447,7 @@ class ReplicaManager(val config: KafkaConfig,
           // have been completely populated before starting the checkpointing there by avoiding weird race conditions
           startHighWatermarkCheckPointThread()
 
-          maybeAddLogDirFetchers(partitionStates.keySet, highWatermarkCheckpoints)
+          maybeAddLogDirFetchers(partitionStates.keySet, highWatermarkCheckpoints, getTopicId)
 
           replicaFetcherManager.shutdownIdleFetcherThreads()
           replicaAlterLogDirsManager.shutdownIdleFetcherThreads()
@@ -1473,6 +1485,37 @@ class ReplicaManager(val config: KafkaConfig,
     }
   }
 
+  /**
+   * Checks if the topic ID provided in the LeaderAndIsr request is consistent with the topic ID in the log.
+   *
+   * If the request had an invalid topic ID (null or zero), then we assume that topic IDs are not supported.
+   * The topic ID was not inconsistent, so return true.
+   * If the log does not exist or the topic ID is not yet set, logTopicIdOpt will be None.
+   * In both cases, the ID is not inconsistent so return true.
+   * If a valid topic ID is provided, and the log exists but has no ID set, set the log ID to be the request ID.
+   *
+   * @param requestTopicId the topic ID from the LeaderAndIsr request
+   * @param logTopicIdOpt the topic ID in the log if the log and the topic ID exists
+   * @param partitionLog the log for the partition being checked if it exists
+   * @return true if the request topic id is consistent, false otherwise
+   */
+  private def checkTopicId(requestTopicId: Uuid, logTopicIdOpt: Option[Uuid], partitionLog: Option[Log]): Boolean = {
+    if (requestTopicId == null || requestTopicId == Uuid.ZERO_UUID) {
+      true
+    } else
+      logTopicIdOpt match {
+        case None =>
+          // if we have valid request ID, log exists, but log topic ID is not yet assigned, write to log
+          partitionLog.foreach(log => {
+            log.partitionMetadataFile.write(requestTopicId)
+            log.topicId = Some(requestTopicId)
+          })
+          true
+        case Some(logTopicId)  =>
+           requestTopicId == logTopicId
+    }
+  }
+
   /**
    * KAFKA-8392
    * For topic partitions of which the broker is no longer a leader, delete metrics related to
@@ -1488,7 +1531,8 @@ class ReplicaManager(val config: KafkaConfig,
   }
 
   protected def maybeAddLogDirFetchers(partitions: Set[Partition],
-                                     offsetCheckpoints: OffsetCheckpoints): Unit = {
+                                       offsetCheckpoints: OffsetCheckpoints,
+                                       topicIds: String => Option[Uuid]): Unit = {
     val futureReplicasAndInitialOffset = new mutable.HashMap[TopicPartition, InitialFetchState]
     for (partition <- partitions) {
       val topicPartition = partition.topicPartition
@@ -1500,7 +1544,8 @@ class ReplicaManager(val config: KafkaConfig,
           partition.createLogIfNotExists(
             isNew = false,
             isFutureReplica = true,
-            offsetCheckpoints)
+            offsetCheckpoints,
+            topicIds(partition.topic))
 
           // pause cleaning for partitions that are being moved and start ReplicaAlterDirThread to move
           // replica from source dir to destination dir
@@ -1534,7 +1579,8 @@ class ReplicaManager(val config: KafkaConfig,
                           partitionStates: Map[Partition, LeaderAndIsrPartitionState],
                           correlationId: Int,
                           responseMap: mutable.Map[TopicPartition, Errors],
-                          highWatermarkCheckpoints: OffsetCheckpoints): Set[Partition] = {
+                          highWatermarkCheckpoints: OffsetCheckpoints,
+                          topicIds: String => Option[Uuid]): Set[Partition] = {
     val traceEnabled = stateChangeLogger.isTraceEnabled
     partitionStates.keys.foreach { partition =>
       if (traceEnabled)
@@ -1555,7 +1601,7 @@ class ReplicaManager(val config: KafkaConfig,
       // Update the partition information to be the leader
       partitionStates.forKeyValue { (partition, partitionState) =>
         try {
-          if (partition.makeLeader(partitionState, highWatermarkCheckpoints))
+          if (partition.makeLeader(partitionState, highWatermarkCheckpoints, topicIds(partitionState.topicName)))
             partitionsToMakeLeaders += partition
           else
             stateChangeLogger.info(s"Skipped the become-leader state change after marking its " +
@@ -1616,7 +1662,8 @@ class ReplicaManager(val config: KafkaConfig,
                             partitionStates: Map[Partition, LeaderAndIsrPartitionState],
                             correlationId: Int,
                             responseMap: mutable.Map[TopicPartition, Errors],
-                            highWatermarkCheckpoints: OffsetCheckpoints) : Set[Partition] = {
+                            highWatermarkCheckpoints: OffsetCheckpoints,
+                            topicIds: String => Option[Uuid]) : Set[Partition] = {
     val traceLoggingEnabled = stateChangeLogger.isTraceEnabled
     partitionStates.forKeyValue { (partition, partitionState) =>
       if (traceLoggingEnabled)
@@ -1635,7 +1682,7 @@ class ReplicaManager(val config: KafkaConfig,
           metadataCache.getAliveBrokers.find(_.id == newLeaderBrokerId) match {
             // Only change partition state when the leader is available
             case Some(_) =>
-              if (partition.makeFollower(partitionState, highWatermarkCheckpoints))
+              if (partition.makeFollower(partitionState, highWatermarkCheckpoints, topicIds(partitionState.topicName)))
                 partitionsToMakeFollower += partition
               else
                 stateChangeLogger.info(s"Skipped the become-follower state change after marking its partition as " +
@@ -1653,7 +1700,7 @@ class ReplicaManager(val config: KafkaConfig,
               // Create the local replica even if the leader is unavailable. This is required to ensure that we include
               // the partition's high watermark in the checkpoint file (see KAFKA-1647)
               partition.createLogIfNotExists(isNew = partitionState.isNew, isFutureReplica = false,
-                highWatermarkCheckpoints)
+                highWatermarkCheckpoints, topicIds(partitionState.topicName))
           }
         } catch {
           case e: KafkaStorageException =>
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
index b105df3110102..7d9ca1fbecf1d 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
@@ -27,7 +27,7 @@ import kafka.server._
 import kafka.server.checkpoints.OffsetCheckpoints
 import kafka.utils._
 import org.apache.kafka.common.message.LeaderAndIsrRequestData.LeaderAndIsrPartitionState
-import org.apache.kafka.common.TopicPartition
+import org.apache.kafka.common.{TopicPartition, Uuid}
 import org.apache.kafka.common.record.{MemoryRecords, SimpleRecord}
 import org.apache.kafka.common.utils.Utils
 import org.junit.jupiter.api.Assertions.{assertEquals, assertFalse, assertTrue}
@@ -278,7 +278,7 @@ class PartitionLockTest extends Logging {
         }
       }
 
-      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints): Log = {
+      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Log = {
         val log = super.createLog(isNew, isFutureReplica, offsetCheckpoints)
         new SlowLog(log, mockTime, appendSemaphore)
       }
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
index 9e66969602645..3b97b3dae2543 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
@@ -34,7 +34,7 @@ import org.apache.kafka.common.record._
 import org.apache.kafka.common.requests.ListOffsetsRequest
 import org.apache.kafka.common.requests.OffsetsForLeaderEpochResponse.{UNDEFINED_EPOCH, UNDEFINED_EPOCH_OFFSET}
 import org.apache.kafka.common.utils.SystemTime
-import org.apache.kafka.common.{IsolationLevel, TopicPartition}
+import org.apache.kafka.common.{IsolationLevel, TopicPartition, Uuid}
 import org.junit.jupiter.api.Assertions._
 import org.junit.jupiter.api.Test
 import org.mockito.ArgumentMatchers
@@ -230,7 +230,7 @@ class PartitionTest extends AbstractPartitionTest {
       logManager,
       alterIsrManager) {
 
-      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints): Log = {
+      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Log = {
         val log = super.createLog(isNew, isFutureReplica, offsetCheckpoints)
         new SlowLog(log, mockTime, appendSemaphore)
       }
@@ -1637,6 +1637,98 @@ class PartitionTest extends AbstractPartitionTest {
     assertEquals(4, partition.localLogOrException.highWatermark)
   }
 
+  @Test
+  def testTopicIdAndPartitionMetadataFileForLeader(): Unit = {
+    val controllerEpoch = 3
+    val leaderEpoch = 5
+    val topicId = Uuid.randomUuid()
+    val replicas = List[Integer](brokerId, brokerId + 1).asJava
+    val leaderState = new LeaderAndIsrPartitionState()
+      .setControllerEpoch(controllerEpoch)
+      .setLeader(brokerId)
+      .setLeaderEpoch(leaderEpoch)
+      .setIsr(replicas)
+      .setZkVersion(1)
+      .setReplicas(replicas)
+      .setIsNew(false)
+    partition.makeLeader(leaderState, offsetCheckpoints, Some(topicId))
+
+    checkTopicId(topicId, partition)
+
+    // Create new Partition object for same topicPartition
+    val partition2 = new Partition(topicPartition,
+      replicaLagTimeMaxMs = Defaults.ReplicaLagTimeMaxMs,
+      interBrokerProtocolVersion = ApiVersion.latestVersion,
+      localBrokerId = brokerId,
+      time,
+      isrChangeListener,
+      delayedOperations,
+      metadataCache,
+      logManager,
+      alterIsrManager)
+
+    // partition2 should not yet be associated with the log, but should be able to get ID
+    assertTrue(partition2.topicId.isDefined)
+    assertEquals(topicId, partition2.topicId.get)
+    assertFalse(partition2.log.isDefined)
+
+    // Calling makeLeader with a new topic ID should not overwrite the old topic ID. We should get the same log.
+    // This scenario should not occur, since the topic ID check will fail, but it is good to check we grab the old log.
+    partition2.makeLeader(leaderState, offsetCheckpoints, Some(Uuid.randomUuid()))
+    checkTopicId(topicId, partition2)
+  }
+
+  @Test
+  def testTopicIdAndPartitionMetadataFileForFollower(): Unit = {
+    val controllerEpoch = 3
+    val leaderEpoch = 5
+    val topicId = Uuid.randomUuid()
+    val replicas = List[Integer](brokerId, brokerId + 1).asJava
+    val leaderState = new LeaderAndIsrPartitionState()
+      .setControllerEpoch(controllerEpoch)
+      .setLeader(brokerId)
+      .setLeaderEpoch(leaderEpoch)
+      .setIsr(replicas)
+      .setZkVersion(1)
+      .setReplicas(replicas)
+      .setIsNew(false)
+    partition.makeFollower(leaderState, offsetCheckpoints, Some(topicId))
+
+    checkTopicId(topicId, partition)
+
+    // Create new Partition object for same topicPartition
+    val partition2 = new Partition(topicPartition,
+      replicaLagTimeMaxMs = Defaults.ReplicaLagTimeMaxMs,
+      interBrokerProtocolVersion = ApiVersion.latestVersion,
+      localBrokerId = brokerId,
+      time,
+      isrChangeListener,
+      delayedOperations,
+      metadataCache,
+      logManager,
+      alterIsrManager)
+
+    // partition2 should not yet be associated with the log, but should be able to get ID
+    assertTrue(partition2.topicId.isDefined)
+    assertEquals(topicId, partition2.topicId.get)
+    assertFalse(partition2.log.isDefined)
+
+    // Calling makeFollower with a new topic ID should not overwrite the old topic ID. We should get the same log.
+    // This scenario should not occur, since the topic ID check will fail, but it is good to check we grab the old log.
+    partition2.makeFollower(leaderState, offsetCheckpoints, Some(Uuid.randomUuid()))
+    checkTopicId(topicId, partition2)
+  }
+
+  def checkTopicId(expectedTopicId: Uuid, partition: Partition): Unit = {
+    assertTrue(partition.topicId.isDefined)
+    assertEquals(expectedTopicId, partition.topicId.get)
+    assertTrue(partition.log.isDefined)
+    val log = partition.log.get
+    assertEquals(expectedTopicId, log.topicId.get)
+    assertTrue(log.partitionMetadataFile.exists())
+    assertEquals(expectedTopicId, log.partitionMetadataFile.read().topicId)
+  }
+
   @Test
   def testAddAndRemoveMetrics(): Unit = {
     val metricsToCheck = List(
diff --git a/core/src/test/scala/unit/kafka/log/LogTest.scala b/core/src/test/scala/unit/kafka/log/LogTest.scala
index 1e5257df49853..fbed2aedda50b 100755
--- a/core/src/test/scala/unit/kafka/log/LogTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogTest.scala
@@ -2559,10 +2559,29 @@ class LogTest {
 
     // test recovery case
     log = createLog(logDir, logConfig)
-    assertTrue(log.topicId == topicId)
+    assertTrue(log.topicId.isDefined)
+    assertTrue(log.topicId.get == topicId)
     log.close()
   }
 
+  @Test
+  def testLogFailsWhenInconsistentTopicIdSet(): Unit = {
+    val logConfig = LogTest.createLogConfig()
+    var log = createLog(logDir, logConfig)
+
+    val topicId = Uuid.randomUuid()
+    log.partitionMetadataFile.write(topicId)
+    log.close()
+
+    // test creating a log with a new ID
+    try {
+      log = createLog(logDir, logConfig, topicId = Some(Uuid.randomUuid()))
+      log.close()
+    } catch {
+      case e: Throwable => assertTrue(e.isInstanceOf[IllegalStateException])
+    }
+  }
+
   /**
    * Test building the time index on the follower by setting assignOffsets to false.
    */
@@ -3123,7 +3142,7 @@ class LogTest {
 
     // Write a topic ID to the partition metadata file to ensure it is transferred correctly.
     val id = Uuid.randomUuid()
-    log.topicId = id
+    log.topicId = Some(id)
     log.partitionMetadataFile.write(id)
 
     log.appendAsLeader(TestUtils.records(List(new SimpleRecord("foo".getBytes()))), leaderEpoch = 5)
@@ -3138,7 +3157,8 @@ class LogTest {
     assertFalse(PartitionMetadataFile.newFile(this.logDir).exists())
 
     // Check the topic ID remains in memory and was copied correctly.
-    assertEquals(id, log.topicId)
+    assertTrue(log.topicId.isDefined)
+    assertEquals(id, log.topicId.get)
     assertEquals(id, log.partitionMetadataFile.read().topicId)
   }
 
@@ -4853,9 +4873,10 @@ class LogTest {
                         time: Time = mockTime,
                         maxProducerIdExpirationMs: Int = 60 * 60 * 1000,
                         producerIdExpirationCheckIntervalMs: Int = LogManager.ProducerIdExpirationCheckIntervalMs,
-                        lastShutdownClean: Boolean = true): Log = {
+                        lastShutdownClean: Boolean = true,
+                        topicId: Option[Uuid] = None): Log = {
     LogTest.createLog(dir, config, brokerTopicStats, scheduler, time, logStartOffset, recoveryPoint,
-      maxProducerIdExpirationMs, producerIdExpirationCheckIntervalMs, lastShutdownClean)
+      maxProducerIdExpirationMs, producerIdExpirationCheckIntervalMs, lastShutdownClean, topicId = topicId)
   }
 
   private def createLogWithOffsetOverflow(logConfig: LogConfig): (Log, LogSegment) = {
@@ -4921,7 +4942,8 @@ object LogTest {
                 recoveryPoint: Long = 0L,
                 maxProducerIdExpirationMs: Int = 60 * 60 * 1000,
                 producerIdExpirationCheckIntervalMs: Int = LogManager.ProducerIdExpirationCheckIntervalMs,
-                lastShutdownClean: Boolean = true): Log = {
+                lastShutdownClean: Boolean = true,
+                topicId: Option[Uuid] = None): Log = {
     Log(dir = dir,
       config = config,
       logStartOffset = logStartOffset,
@@ -4932,7 +4954,8 @@ object LogTest {
       maxProducerIdExpirationMs = maxProducerIdExpirationMs,
       producerIdExpirationCheckIntervalMs = producerIdExpirationCheckIntervalMs,
       logDirFailureChannel = new LogDirFailureChannel(10),
-      lastShutdownClean = lastShutdownClean)
+      lastShutdownClean = lastShutdownClean,
+      topicId = topicId)
   }
 
   /**
diff --git a/core/src/test/scala/unit/kafka/server/BrokerEpochIntegrationTest.scala b/core/src/test/scala/unit/kafka/server/BrokerEpochIntegrationTest.scala
index cbb6471fe2628..46be884349197 100755
--- a/core/src/test/scala/unit/kafka/server/BrokerEpochIntegrationTest.scala
+++ b/core/src/test/scala/unit/kafka/server/BrokerEpochIntegrationTest.scala
@@ -25,7 +25,7 @@ import kafka.controller.{ControllerChannelManager, ControllerContext, StateChang
 import kafka.utils.TestUtils
 import kafka.utils.TestUtils.createTopic
 import kafka.zk.ZooKeeperTestHarness
-import org.apache.kafka.common.{TopicPartition, Uuid}
+import org.apache.kafka.common.TopicPartition
 import org.apache.kafka.common.message.LeaderAndIsrRequestData.LeaderAndIsrPartitionState
 import org.apache.kafka.common.message.StopReplicaRequestData.{StopReplicaPartitionState, StopReplicaTopicState}
 import org.apache.kafka.common.message.UpdateMetadataRequestData.{UpdateMetadataBroker, UpdateMetadataEndpoint, UpdateMetadataPartitionState}
@@ -112,10 +112,10 @@ class BrokerEpochIntegrationTest extends ZooKeeperTestHarness {
 
   private def testControlRequestWithBrokerEpoch(epochInRequestDiffFromCurrentEpoch: Long): Unit = {
     val tp = new TopicPartition("new-topic", 0)
-    val topicIds = Collections.singletonMap("new-topic", Uuid.randomUuid)
 
     // create topic with 1 partition, 2 replicas, one on each broker
     createTopic(zkClient, tp.topic(), partitionReplicaAssignment = Map(0 -> Seq(brokerId1, brokerId2)), servers = servers)
+    val topicIds = getController.kafkaController.controllerContext.topicIds.toMap.asJava
 
     val controllerId = 2
     val controllerEpoch = zkClient.getControllerEpoch.get._1
diff --git a/core/src/test/scala/unit/kafka/server/RaftReplicaChangeDelegateTest.scala b/core/src/test/scala/unit/kafka/server/RaftReplicaChangeDelegateTest.scala
index 609f1cf3b3625..ad35452a3b0e0 100644
--- a/core/src/test/scala/unit/kafka/server/RaftReplicaChangeDelegateTest.scala
+++ b/core/src/test/scala/unit/kafka/server/RaftReplicaChangeDelegateTest.scala
@@ -24,7 +24,7 @@ import kafka.server.checkpoints.OffsetCheckpoints
 import kafka.server.metadata.{MetadataBroker, MetadataBrokers, MetadataPartition}
 import org.apache.kafka.common.message.LeaderAndIsrRequestData
 import org.apache.kafka.common.network.ListenerName
-import org.apache.kafka.common.{Node, TopicPartition}
+import org.apache.kafka.common.{Node, TopicPartition, Uuid}
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.ValueSource
@@ -43,10 +43,13 @@ class RaftReplicaChangeDelegateTest {
     val leaderId = 0
     val topicPartition = new TopicPartition("foo", 5)
     val replicas = Seq(0, 1, 2).map(Int.box).asJava
+    val topicId = Uuid.randomUuid()
+    val topicIds = (topicName: String) => if (topicName == "foo") Some(topicId) else None
 
     val helper = mockedHelper()
     val partition = mock(classOf[Partition])
     when(partition.topicPartition).thenReturn(topicPartition)
+    when(partition.topic).thenReturn(topicPartition.topic)
 
     val highWatermarkCheckpoints = mock(classOf[OffsetCheckpoints])
     when(highWatermarkCheckpoints.fetch(
@@ -81,16 +84,17 @@ class RaftReplicaChangeDelegateTest {
 
     val delegate = new RaftReplicaChangeDelegate(helper)
     val updatedPartitions = if (isLeader) {
-      when(partition.makeLeader(expectedLeaderAndIsr, highWatermarkCheckpoints))
+      when(partition.makeLeader(expectedLeaderAndIsr, highWatermarkCheckpoints, Some(topicId)))
         .thenReturn(true)
       delegate.makeLeaders(
         prevPartitionsAlreadyExisting = Set.empty,
         partitionStates = Map(partition -> metadataPartition),
         highWatermarkCheckpoints,
-        metadataOffset = Some(500)
+        metadataOffset = Some(500),
+        topicIds
       )
     } else {
-      when(partition.makeFollower(expectedLeaderAndIsr, highWatermarkCheckpoints))
+      when(partition.makeFollower(expectedLeaderAndIsr, highWatermarkCheckpoints, Some(topicId)))
         .thenReturn(true)
       when(partition.leaderReplicaIdOpt).thenReturn(Some(leaderId))
       delegate.makeFollowers(
@@ -98,7 +102,8 @@ class RaftReplicaChangeDelegateTest {
         currentBrokers = aliveBrokers(replicas),
         partitionStates = Map(partition -> metadataPartition),
         highWatermarkCheckpoints,
-        metadataOffset = Some(500)
+        metadataOffset = Some(500),
+        topicIds
       )
     }
 
diff --git a/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
index acb53041a245b..a37214b605fc7 100644
--- a/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/server/RaftReplicaManagerTest.scala
@@ -133,11 +133,6 @@ class RaftReplicaManagerTest {
     val partition0 =  Partition(topicPartition0, time, configRepository, rrm)
     val partition1 =  Partition(topicPartition1, time, configRepository, rrm)
 
-    rrm.createPartition(topicPartition0).createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
-    rrm.createPartition(topicPartition1).createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
-
     processTopicPartitionMetadata(rrm)
     // verify changes would have been deferred
     val partitionsNewMapCaptor: ArgumentCaptor[mutable.Map[Partition, Boolean]] =
@@ -145,7 +140,7 @@ class RaftReplicaManagerTest {
     verify(mockDelegate).makeDeferred(partitionsNewMapCaptor.capture(), ArgumentMatchers.eq(offset1))
     val partitionsDeferredMap = partitionsNewMapCaptor.getValue
     assertEquals(Map(partition0 -> true, partition1 -> true), partitionsDeferredMap)
-    verify(mockDelegate, never()).makeFollowers(any(), any(), any(), any(), any())
+    verify(mockDelegate, never()).makeFollowers(any(), any(), any(), any(), any(), any())
 
     // now mark those topic partitions as being deferred so we can later apply the changes
     rrm.markPartitionDeferred(partition0, isNew = true)
@@ -153,8 +148,15 @@ class RaftReplicaManagerTest {
 
     // apply the changes
     // define some return values to avoid NPE
-    when(mockDelegate.makeLeaders(any(), any(), any(), any())).thenReturn(Set(partition0))
-    when(mockDelegate.makeFollowers(any(), any(), any(), any(), any())).thenReturn(Set(partition1))
+    when(mockDelegate.makeLeaders(any(), any(), any(), any(), any())).thenReturn(Set(partition0))
+    when(mockDelegate.makeFollowers(any(), any(), any(), any(), any(), any())).thenReturn(Set(partition1))
+
+    // Simulate creation of logs in makeLeaders/makeFollowers
+    partition0.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(topicId))
+    partition1.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(topicId))
+
     rrm.endMetadataChangeDeferral(onLeadershipChange)
     // verify that the deferred changes would have been applied
 
@@ -180,16 +182,18 @@ class RaftReplicaManagerTest {
     val partition0 = Partition(topicPartition0, time, configRepository, rrm)
     val partition1 = Partition(topicPartition1, time, configRepository, rrm)
 
-    rrm.createPartition(topicPartition0).createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
-    rrm.createPartition(topicPartition1).createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
-
     rrm.endMetadataChangeDeferral(onLeadershipChange)
 
     // define some return values to avoid NPE
-    when(mockDelegate.makeLeaders(any(), any(), any(), ArgumentMatchers.eq(Some(offset1)))).thenReturn(Set(partition0))
-    when(mockDelegate.makeFollowers(any(), any(), any(), any(), ArgumentMatchers.eq(Some(offset1)))).thenReturn(Set(partition1))
+    when(mockDelegate.makeLeaders(any(), any(), any(), ArgumentMatchers.eq(Some(offset1)), any())).thenReturn(Set(partition0))
+    when(mockDelegate.makeFollowers(any(), any(), any(), any(), ArgumentMatchers.eq(Some(offset1)), any())).thenReturn(Set(partition1))
+
+    // Simulate creation of logs in makeLeaders/makeFollowers
+    partition0.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(topicId))
+    partition1.createLogIfNotExists(isNew = false, isFutureReplica = false,
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(topicId))
+
     // process the changes
     processTopicPartitionMetadata(rrm)
     // verify that the changes would have been applied
@@ -217,12 +221,9 @@ class RaftReplicaManagerTest {
     val partition1 = rrm.createPartition(topicPartition1)
 
     partition0.createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+     new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(Uuid.randomUuid()))
     partition1.createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
-
-    assertTrue(partition0.log.isDefined)
-    partition0.log.get.topicId = Uuid.randomUuid()
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(Uuid.randomUuid()))
 
     try {
       processTopicPartitionMetadata(rrm)
@@ -239,18 +240,15 @@ class RaftReplicaManagerTest {
     val partition1 = rrm.createPartition(topicPartition1)
 
     partition0.createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(Uuid.randomUuid()))
     partition1.createLogIfNotExists(isNew = false, isFutureReplica = false,
-      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints))
-
-    assertTrue(partition1.log.isDefined)
-    partition1.log.get.topicId = Uuid.randomUuid()
+      new LazyOffsetCheckpoints(rrm.highWatermarkCheckpoints), Some(Uuid.randomUuid()))
 
     rrm.endMetadataChangeDeferral(onLeadershipChange)
 
     // define some return values to avoid NPE
-    when(mockDelegate.makeLeaders(any(), any(), any(), ArgumentMatchers.eq(Some(offset1)))).thenReturn(Set(partition0))
-    when(mockDelegate.makeFollowers(any(), any(), any(), any(), ArgumentMatchers.eq(Some(offset1)))).thenReturn(Set(partition1))
+    when(mockDelegate.makeLeaders(any(), any(), any(), ArgumentMatchers.eq(Some(offset1)), any())).thenReturn(Set(partition0))
+    when(mockDelegate.makeFollowers(any(), any(), any(), any(), ArgumentMatchers.eq(Some(offset1)), any())).thenReturn(Set(partition1))
 
     try {
       processTopicPartitionMetadata(rrm)
@@ -264,7 +262,7 @@ class RaftReplicaManagerTest {
     val leaderPartitionStatesCaptor: ArgumentCaptor[mutable.Map[Partition, MetadataPartition]] =
       ArgumentCaptor.forClass(classOf[mutable.Map[Partition, MetadataPartition]])
     verify(mockDelegate).makeLeaders(ArgumentMatchers.eq(expectedPrevPartitionsAlreadyExisting),
-      leaderPartitionStatesCaptor.capture(), any(), ArgumentMatchers.eq(expectedMetadataOffset))
+      leaderPartitionStatesCaptor.capture(), any(), ArgumentMatchers.eq(expectedMetadataOffset), any())
     leaderPartitionStatesCaptor.getValue
   }
 
@@ -275,7 +273,7 @@ class RaftReplicaManagerTest {
       ArgumentCaptor.forClass(classOf[mutable.Map[Partition, MetadataPartition]])
     val brokersCaptor: ArgumentCaptor[MetadataBrokers] = ArgumentCaptor.forClass(classOf[MetadataBrokers])
     verify(mockDelegate).makeFollowers(ArgumentMatchers.eq(expectedPrevPartitionsAlreadyExisting), brokersCaptor.capture(),
-      followerPartitionStatesCaptor.capture(), any(), ArgumentMatchers.eq(expectedMetadataOffset))
+      followerPartitionStatesCaptor.capture(), any(), ArgumentMatchers.eq(expectedMetadataOffset), any())
     val brokers = brokersCaptor.getValue
     assertEquals(expectedBrokers.size, brokers.size())
     expectedBrokers.foreach(brokerId => assertTrue(brokers.aliveBroker(brokerId).isDefined))
diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
index 76ce6c7a5f145..038a63890992b 100644
--- a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
@@ -809,6 +809,7 @@ class ReplicaManagerTest {
   private def verifyBecomeFollowerWhenLeaderIsUnchangedButMissedLeaderUpdate(extraProps: Properties,
                                                                              expectTruncation: Boolean): Unit = {
     val topicPartition = 0
+    val topicId = Uuid.randomUuid()
     val followerBrokerId = 0
     val leaderBrokerId = 1
     val controllerId = 0
@@ -821,7 +822,7 @@ class ReplicaManagerTest {
     // Prepare the mocked components for the test
     val (replicaManager, mockLogMgr) = prepareReplicaManagerAndLogManager(new MockTimer(time),
       topicPartition, leaderEpoch + leaderEpochIncrement, followerBrokerId, leaderBrokerId, countDownLatch,
-      expectTruncation = expectTruncation, localLogOffset = Some(10), extraProps = extraProps)
+      expectTruncation = expectTruncation, localLogOffset = Some(10), extraProps = extraProps, topicId = Some(topicId))
 
     // Initialize partition state to follower, with leader = 1, leaderEpoch = 1
     val tp = new TopicPartition(topic, topicPartition)
@@ -838,7 +839,7 @@ class ReplicaManagerTest {
     val leaderAndIsrRequest0 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion,
       controllerId, controllerEpoch, brokerEpoch,
       Seq(leaderAndIsrPartitionState(tp, leaderEpoch, leaderBrokerId, aliveBrokerIds)).asJava,
-      Collections.singletonMap(topic, Uuid.randomUuid()),
+      Collections.singletonMap(topic, topicId),
       Set(new Node(followerBrokerId, "host1", 0),
         new Node(leaderBrokerId, "host2", 1)).asJava).build()
     replicaManager.becomeLeaderOrFollower(correlationId, leaderAndIsrRequest0,
@@ -886,6 +887,7 @@ class ReplicaManagerTest {
   @Test
   def testPreferredReplicaAsFollower(): Unit = {
     val topicPartition = 0
+    val topicId = Uuid.randomUuid()
     val followerBrokerId = 0
     val leaderBrokerId = 1
     val leaderEpoch = 1
@@ -895,13 +897,13 @@ class ReplicaManagerTest {
     // Prepare the mocked components for the test
     val (replicaManager, _) = prepareReplicaManagerAndLogManager(new MockTimer(time),
       topicPartition, leaderEpoch + leaderEpochIncrement, followerBrokerId,
-      leaderBrokerId, countDownLatch, expectTruncation = true)
+      leaderBrokerId, countDownLatch, expectTruncation = true, topicId = Some(topicId))
 
     val brokerList = Seq[Integer](0, 1).asJava
 
     val tp0 = new TopicPartition(topic, 0)
 
-    replicaManager.createPartition(new TopicPartition(topic, 0))
+    initializeLogAndTopicId(replicaManager, tp0, topicId)
 
     // Make this replica the follower
     val leaderAndIsrRequest2 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -915,7 +917,7 @@ class ReplicaManagerTest {
         .setZkVersion(0)
         .setReplicas(brokerList)
         .setIsNew(false)).asJava,
-      Collections.singletonMap(topic, Uuid.randomUuid()),
+      Collections.singletonMap(topic, topicId),
       Set(new Node(0, "host1", 0), new Node(1, "host2", 1)).asJava).build()
     replicaManager.becomeLeaderOrFollower(1, leaderAndIsrRequest2, (_, _) => ())
 
@@ -936,6 +938,7 @@ class ReplicaManagerTest {
   @Test
   def testPreferredReplicaAsLeader(): Unit = {
     val topicPartition = 0
+    val topicId = Uuid.randomUuid()
     val followerBrokerId = 0
     val leaderBrokerId = 1
     val leaderEpoch = 1
@@ -945,13 +948,13 @@ class ReplicaManagerTest {
     // Prepare the mocked components for the test
     val (replicaManager, _) = prepareReplicaManagerAndLogManager(new MockTimer(time),
       topicPartition, leaderEpoch + leaderEpochIncrement, followerBrokerId,
-      leaderBrokerId, countDownLatch, expectTruncation = true)
+      leaderBrokerId, countDownLatch, expectTruncation = true, topicId = Some(topicId))
 
     val brokerList = Seq[Integer](0, 1).asJava
 
     val tp0 = new TopicPartition(topic, 0)
 
-    replicaManager.createPartition(new TopicPartition(topic, 0))
+    initializeLogAndTopicId(replicaManager, tp0, topicId)
 
     // Make this replica the follower
     val leaderAndIsrRequest2 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -965,7 +968,7 @@ class ReplicaManagerTest {
         .setZkVersion(0)
         .setReplicas(brokerList)
         .setIsNew(false)).asJava,
-      Collections.singletonMap(topic, Uuid.randomUuid()),
+      Collections.singletonMap(topic, topicId),
       Set(new Node(0, "host1", 0), new Node(1, "host2", 1)).asJava).build()
     replicaManager.becomeLeaderOrFollower(1, leaderAndIsrRequest2, (_, _) => ())
 
@@ -986,6 +989,7 @@ class ReplicaManagerTest {
   @Test
   def testFollowerFetchWithDefaultSelectorNoForcedHwPropagation(): Unit = {
     val topicPartition = 0
+    val topicId = Uuid.randomUuid()
     val followerBrokerId = 0
     val leaderBrokerId = 1
     val leaderEpoch = 1
@@ -996,13 +1000,13 @@ class ReplicaManagerTest {
     // Prepare the mocked components for the test
     val (replicaManager, _) = prepareReplicaManagerAndLogManager(timer,
       topicPartition, leaderEpoch + leaderEpochIncrement, followerBrokerId,
-      leaderBrokerId, countDownLatch, expectTruncation = true)
+      leaderBrokerId, countDownLatch, expectTruncation = true, topicId = Some(topicId))
 
     val brokerList = Seq[Integer](0, 1).asJava
 
     val tp0 = new TopicPartition(topic, 0)
 
-    replicaManager.createPartition(new TopicPartition(topic, 0))
+    initializeLogAndTopicId(replicaManager, tp0, topicId)
 
     // Make this replica the follower
     val leaderAndIsrRequest2 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -1016,7 +1020,7 @@ class ReplicaManagerTest {
         .setZkVersion(0)
         .setReplicas(brokerList)
         .setIsNew(false)).asJava,
-      Collections.singletonMap(topic, Uuid.randomUuid()),
+      Collections.singletonMap(topic, topicId),
       Set(new Node(0, "host1", 0), new Node(1, "host2", 1)).asJava).build()
     replicaManager.becomeLeaderOrFollower(1, leaderAndIsrRequest2, (_, _) => ())
 
@@ -1064,6 +1068,16 @@ class ReplicaManagerTest {
       leaderBrokerId, countDownLatch, expectTruncation = true, extraProps = props))
   }
 
+  // Due to some limitations to EasyMock, we need to create the log so that the Partition.topicId does not call
+  // LogManager.getLog with a default argument
+  // TODO: convert tests to using Mockito to avoid this issue.
+  private def initializeLogAndTopicId(replicaManager: ReplicaManager, topicPartition: TopicPartition, topicId: Uuid): Unit = {
+    val partition = replicaManager.createPartition(new TopicPartition(topic, 0))
+    val log = replicaManager.logManager.getOrCreateLog(topicPartition, false, false, Some(topicId))
+    log.topicId = Some(topicId)
+    partition.log = Some(log)
+  }
+
   @Test
   def testDefaultReplicaSelector(): Unit = {
     val topicPartition = 0
@@ -1456,7 +1470,8 @@ class ReplicaManagerTest {
                                                  localLogOffset: Option[Long] = None,
                                                  offsetFromLeader: Long = 5,
                                                  leaderEpochFromLeader: Int = 3,
-                                                 extraProps: Properties = new Properties()) : (ReplicaManager, LogManager) = {
+                                                 extraProps: Properties = new Properties(),
+                                                 topicId: Option[Uuid] = None) : (ReplicaManager, LogManager) = {
     val props = TestUtils.createBrokerConfig(0, TestUtils.MockZkConnect)
     props.put("log.dir", TestUtils.tempRelativeDir("data").getAbsolutePath)
     props.asScala ++= extraProps.asScala
@@ -1478,7 +1493,8 @@ class ReplicaManagerTest {
       topicPartition = new TopicPartition(topic, topicPartition),
       producerStateManager = new ProducerStateManager(new TopicPartition(topic, topicPartition),
         new File(new File(config.logDirs.head), s"$topic-$topicPartition"), 30000),
-      logDirFailureChannel = mockLogDirFailureChannel) {
+      logDirFailureChannel = mockLogDirFailureChannel,
+      topicId = topicId) {
 
       override def endOffsetForEpoch(leaderEpoch: Int): Option[OffsetAndEpoch] = {
         assertEquals(leaderEpoch, leaderEpochFromLeader)
@@ -1500,7 +1516,7 @@ class ReplicaManagerTest {
     val mockLogMgr: LogManager = EasyMock.createMock(classOf[LogManager])
     EasyMock.expect(mockLogMgr.liveLogDirs).andReturn(config.logDirs.map(new File(_).getAbsoluteFile)).anyTimes
     EasyMock.expect(mockLogMgr.getOrCreateLog(EasyMock.eq(topicPartitionObj),
-      isNew = EasyMock.eq(false), isFuture = EasyMock.eq(false))).andReturn(mockLog).anyTimes
+      isNew = EasyMock.eq(false), isFuture = EasyMock.eq(false), EasyMock.anyObject())).andReturn(mockLog).anyTimes
     if (expectTruncation) {
       EasyMock.expect(mockLogMgr.truncateTo(Map(topicPartitionObj -> offsetFromLeader),
         isFuture = false)).once
@@ -2215,14 +2231,57 @@ class ReplicaManagerTest {
   }
 
   @Test
-  def testPartitionMetadataFile() = {
+  def testPartitionMetadataFile(): Unit = {
     val replicaManager = setupReplicaManagerWithMockedPurgatories(new MockTimer(time))
     try {
       val brokerList = Seq[Integer](0, 1).asJava
       val topicPartition = new TopicPartition(topic, 0)
-      replicaManager.createPartition(topicPartition)
-        .createLogIfNotExists(isNew = false, isFutureReplica = false,
-          new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints))
+      val topicIds = Collections.singletonMap(topic, Uuid.randomUuid())
+      val topicNames = topicIds.asScala.map(_.swap).asJava
+
+      def leaderAndIsrRequest(epoch: Int, topicIds: java.util.Map[String, Uuid]): LeaderAndIsrRequest =
+        new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
+          Seq(new LeaderAndIsrPartitionState()
+            .setTopicName(topic)
+            .setPartitionIndex(0)
+            .setControllerEpoch(0)
+            .setLeader(0)
+            .setLeaderEpoch(epoch)
+            .setIsr(brokerList)
+            .setZkVersion(0)
+            .setReplicas(brokerList)
+            .setIsNew(true)).asJava,
+          topicIds,
+          Set(new Node(0, "host1", 0), new Node(1, "host2", 1)).asJava).build()
+
+      val response = replicaManager.becomeLeaderOrFollower(0, leaderAndIsrRequest(0, topicIds), (_, _) => ())
+      assertEquals(Errors.NONE, response.partitionErrors(topicNames).get(topicPartition))
+      assertFalse(replicaManager.localLog(topicPartition).isEmpty)
+      val id = topicIds.get(topicPartition.topic())
+      val log = replicaManager.localLog(topicPartition).get
+      assertTrue(log.partitionMetadataFile.exists())
+      val partitionMetadata = log.partitionMetadataFile.read()
+
+      // Current version of PartitionMetadataFile is 0.
+      assertEquals(0, partitionMetadata.version)
+      assertEquals(id, partitionMetadata.topicId)
+    } finally replicaManager.shutdown(checkpointHW = false)
+  }
+
+  @Test
+  def testPartitionMetadataFileCreatedWithExistingLog(): Unit = {
+    val replicaManager = setupReplicaManagerWithMockedPurgatories(new MockTimer(time))
+    try {
+      val brokerList = Seq[Integer](0, 1).asJava
+      val topicPartition = new TopicPartition(topic, 0)
+
+      replicaManager.logManager.getOrCreateLog(topicPartition, isNew = true)
+
+      assertTrue(replicaManager.getLog(topicPartition).isDefined)
+      var log = replicaManager.getLog(topicPartition).get
+      assertEquals(None, log.topicId)
+      assertFalse(log.partitionMetadataFile.exists())
+
       val topicIds = Collections.singletonMap(topic, Uuid.randomUuid())
       val topicNames = topicIds.asScala.map(_.swap).asJava
 
@@ -2244,7 +2303,7 @@ class ReplicaManagerTest {
       assertEquals(Errors.NONE, response.partitionErrors(topicNames).get(topicPartition))
       assertFalse(replicaManager.localLog(topicPartition).isEmpty)
       val id = topicIds.get(topicPartition.topic())
-      val log = replicaManager.localLog(topicPartition).get
+      log = replicaManager.localLog(topicPartition).get
       assertTrue(log.partitionMetadataFile.exists())
       val partitionMetadata = log.partitionMetadataFile.read()
 
@@ -2255,14 +2314,11 @@ class ReplicaManagerTest {
   }
 
   @Test
-  def testInvalidIdReturnsError() = {
+  def testInvalidIdReturnsError(): Unit = {
     val replicaManager = setupReplicaManagerWithMockedPurgatories(new MockTimer(time))
     try {
       val brokerList = Seq[Integer](0, 1).asJava
       val topicPartition = new TopicPartition(topic, 0)
-      replicaManager.createPartition(topicPartition)
-        .createLogIfNotExists(isNew = false, isFutureReplica = false,
-          new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints))
       val topicIds = Collections.singletonMap(topic, Uuid.randomUuid())
       val topicNames = topicIds.asScala.map(_.swap).asJava
 
@@ -2300,15 +2356,13 @@ class ReplicaManagerTest {
   }
 
   @Test
-  def testPartitionMetadataFileNotCreated() = {
+  def testPartitionMetadataFileNotCreated(): Unit = {
     val replicaManager = setupReplicaManagerWithMockedPurgatories(new MockTimer(time))
     try {
       val brokerList = Seq[Integer](0, 1).asJava
       val topicPartition = new TopicPartition(topic, 0)
       val topicPartitionFoo = new TopicPartition("foo", 0)
-      replicaManager.createPartition(topicPartition)
-        .createLogIfNotExists(isNew = false, isFutureReplica = false,
-          new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints))
+      val topicPartitionFake = new TopicPartition("fakeTopic", 0)
       val topicIds = Map(topic -> Uuid.ZERO_UUID, "foo" -> Uuid.randomUuid()).asJava
       val topicNames = topicIds.asScala.map(_.swap).asJava
 
@@ -2329,28 +2383,28 @@ class ReplicaManagerTest {
 
       // There is no file if the topic does not have an associated topic ID.
       val response = replicaManager.becomeLeaderOrFollower(0, leaderAndIsrRequest(0, "fakeTopic", ApiKeys.LEADER_AND_ISR.latestVersion), (_, _) => ())
-      assertFalse(replicaManager.localLog(topicPartition).isEmpty)
-      val log = replicaManager.localLog(topicPartition).get
+      assertTrue(replicaManager.localLog(topicPartitionFake).isDefined)
+      val log = replicaManager.localLog(topicPartitionFake).get
       assertFalse(log.partitionMetadataFile.exists())
       assertEquals(Errors.NONE, response.partitionErrors(topicNames).get(topicPartition))
 
       // There is no file if the topic has the default UUID.
       val response2 = replicaManager.becomeLeaderOrFollower(0, leaderAndIsrRequest(0, topic, ApiKeys.LEADER_AND_ISR.latestVersion), (_, _) => ())
-      assertFalse(replicaManager.localLog(topicPartition).isEmpty)
+      assertTrue(replicaManager.localLog(topicPartition).isDefined)
       val log2 = replicaManager.localLog(topicPartition).get
       assertFalse(log2.partitionMetadataFile.exists())
       assertEquals(Errors.NONE, response2.partitionErrors(topicNames).get(topicPartition))
 
       // There is no file if the request an older version
       val response3 = replicaManager.becomeLeaderOrFollower(0, leaderAndIsrRequest(0, "foo", 0), (_, _) => ())
-      assertFalse(replicaManager.localLog(topicPartitionFoo).isEmpty)
+      assertTrue(replicaManager.localLog(topicPartitionFoo).isDefined)
       val log3 = replicaManager.localLog(topicPartitionFoo).get
       assertFalse(log3.partitionMetadataFile.exists())
       assertEquals(Errors.NONE, response3.partitionErrors(topicNames).get(topicPartitionFoo))
 
       // There is no file if the request is an older version
       val response4 = replicaManager.becomeLeaderOrFollower(0, leaderAndIsrRequest(1, "foo", 4), (_, _) => ())
-      assertFalse(replicaManager.localLog(topicPartitionFoo).isEmpty)
+      assertTrue(replicaManager.localLog(topicPartitionFoo).isDefined)
       val log4 = replicaManager.localLog(topicPartitionFoo).get
       assertFalse(log4.partitionMetadataFile.exists())
       assertEquals(Errors.NONE, response4.partitionErrors(topicNames).get(topicPartitionFoo))
diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java
index 453fa40096b2f..05e5f84649a74 100644
--- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java
+++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java
@@ -44,6 +44,7 @@
 import kafka.utils.KafkaScheduler;
 import kafka.utils.Pool;
 import org.apache.kafka.common.TopicPartition;
+import org.apache.kafka.common.Uuid;
 import org.apache.kafka.common.message.FetchResponseData;
 import org.apache.kafka.common.message.LeaderAndIsrRequestData;
 import org.apache.kafka.common.message.OffsetForLeaderEpochRequestData.OffsetForLeaderPartition;
@@ -104,6 +105,7 @@ public class ReplicaFetcherThreadBenchmark {
     private File logDir = new File(System.getProperty("java.io.tmpdir"), UUID.randomUUID().toString());
     private KafkaScheduler scheduler = new KafkaScheduler(1, "scheduler", true);
     private Pool<TopicPartition, Partition> pool = new Pool<TopicPartition, Partition>(Option.empty());
+    private Option<Uuid> topicId = OptionConverters.toScala(Optional.of(Uuid.randomUuid()));
 
     @Setup(Level.Trial)
     public void setup() throws IOException {
@@ -159,7 +161,7 @@ public void setup() throws IOException {
                     0, Time.SYSTEM, isrChangeListener, new DelayedOperationsMock(tp),
                     Mockito.mock(MetadataCache.class), logManager, isrChannelManager);
 
-            partition.makeFollower(partitionState, offsetCheckpoints);
+            partition.makeFollower(partitionState, offsetCheckpoints, topicId);
             pool.put(tp, partition);
             initialFetchStates.put(tp, new InitialFetchState(new BrokerEndPoint(3, "host", 3000), 0, 0));
             BaseRecords fetched = new BaseRecords() {
diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/PartitionMakeFollowerBenchmark.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/PartitionMakeFollowerBenchmark.java
index ece6f86a0fed8..81cb1fa9af105 100644
--- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/PartitionMakeFollowerBenchmark.java
+++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/PartitionMakeFollowerBenchmark.java
@@ -33,6 +33,7 @@
 import kafka.server.metadata.CachedConfigRepository;
 import kafka.utils.KafkaScheduler;
 import org.apache.kafka.common.TopicPartition;
+import org.apache.kafka.common.Uuid;
 import org.apache.kafka.common.message.LeaderAndIsrRequestData;
 import org.apache.kafka.common.record.CompressionType;
 import org.apache.kafka.common.record.MemoryRecords;
@@ -54,6 +55,7 @@
 import org.openjdk.jmh.annotations.Warmup;
 import scala.Option;
 import scala.collection.JavaConverters;
+import scala.compat.java8.OptionConverters;
 
 import java.io.File;
 import java.io.IOException;
@@ -62,6 +64,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.Properties;
 import java.util.UUID;
 import java.util.concurrent.ExecutorService;
@@ -84,6 +87,7 @@ public class PartitionMakeFollowerBenchmark {
     private OffsetCheckpoints offsetCheckpoints = Mockito.mock(OffsetCheckpoints.class);
     private DelayedOperations delayedOperations  = Mockito.mock(DelayedOperations.class);
     private ExecutorService executorService = Executors.newSingleThreadExecutor();
+    private Option<Uuid> topicId;
 
     @Setup(Level.Trial)
     public void setup() throws IOException {
@@ -114,6 +118,7 @@ public void setup() throws IOException {
             true);
 
         TopicPartition tp = new TopicPartition("topic", 0);
+        topicId = OptionConverters.toScala(Optional.of(Uuid.randomUuid()));
 
         Mockito.when(offsetCheckpoints.fetch(logDir.getAbsolutePath(), tp)).thenReturn(Option.apply(0L));
         IsrChangeListener isrChangeListener = Mockito.mock(IsrChangeListener.class);
@@ -122,7 +127,7 @@ public void setup() throws IOException {
             ApiVersion$.MODULE$.latestVersion(), 0, Time.SYSTEM,
             isrChangeListener, delayedOperations,
             Mockito.mock(MetadataCache.class), logManager, alterIsrManager);
-        partition.createLogIfNotExists(true, false, offsetCheckpoints);
+        partition.createLogIfNotExists(true, false, offsetCheckpoints, topicId);
         executorService.submit((Runnable) () -> {
             SimpleRecord[] simpleRecords = new SimpleRecord[] {
                 new SimpleRecord(1L, "foo".getBytes(StandardCharsets.UTF_8), "1".getBytes(StandardCharsets.UTF_8)),
@@ -155,7 +160,7 @@ public boolean testMakeFollower() {
             .setZkVersion(1)
             .setReplicas(replicas)
             .setIsNew(true);
-        return partition.makeFollower(partitionState, offsetCheckpoints);
+        return partition.makeFollower(partitionState, offsetCheckpoints, topicId);
     }
 
     private static LogConfig createLogConfig() {
diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/UpdateFollowerFetchStateBenchmark.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/UpdateFollowerFetchStateBenchmark.java
index a82c6a0e7744f..e62191b45f409 100644
--- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/UpdateFollowerFetchStateBenchmark.java
+++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/partition/UpdateFollowerFetchStateBenchmark.java
@@ -34,6 +34,7 @@
 import kafka.server.metadata.CachedConfigRepository;
 import kafka.utils.KafkaScheduler;
 import org.apache.kafka.common.TopicPartition;
+import org.apache.kafka.common.Uuid;
 import org.apache.kafka.common.message.LeaderAndIsrRequestData.LeaderAndIsrPartitionState;
 import org.apache.kafka.common.utils.Time;
 import org.mockito.Mockito;
@@ -51,11 +52,13 @@
 import org.openjdk.jmh.annotations.Warmup;
 import scala.Option;
 import scala.collection.JavaConverters;
+import scala.compat.java8.OptionConverters;
 
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.Properties;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
@@ -68,6 +71,7 @@
 @OutputTimeUnit(TimeUnit.NANOSECONDS)
 public class UpdateFollowerFetchStateBenchmark {
     private TopicPartition topicPartition = new TopicPartition(UUID.randomUUID().toString(), 0);
+    private Option<Uuid> topicId = OptionConverters.toScala(Optional.of(Uuid.randomUuid()));
     private File logDir = new File(System.getProperty("java.io.tmpdir"), topicPartition.toString());
     private KafkaScheduler scheduler = new KafkaScheduler(1, "scheduler", true);
     private BrokerTopicStats brokerTopicStats = new BrokerTopicStats();
@@ -120,7 +124,7 @@ public void setUp() {
                 ApiVersion$.MODULE$.latestVersion(), 0, Time.SYSTEM,
                 isrChangeListener, delayedOperations,
                 Mockito.mock(MetadataCache.class), logManager, alterIsrManager);
-        partition.makeLeader(partitionState, offsetCheckpoints);
+        partition.makeLeader(partitionState, offsetCheckpoints, topicId);
     }
 
     // avoid mocked DelayedOperations to avoid mocked class affecting benchmark results
diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java
index 45d2cb1d3406f..649835950c7ec 100644
--- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java
+++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java
@@ -145,7 +145,7 @@ public void setup() {
         OffsetCheckpoints checkpoints = (logDir, topicPartition) -> Option.apply(0L);
         for (TopicPartition topicPartition : topicPartitions) {
             final Partition partition = this.replicaManager.createPartition(topicPartition);
-            partition.createLogIfNotExists(true, false, checkpoints);
+            partition.createLogIfNotExists(true, false, checkpoints, Option.empty());
         }
 
         replicaManager.checkpointHighWatermarks();

From 2893d15dae149f498b2d85d0f598416bfb6f613f Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Wed, 17 Mar 2021 17:58:26 -0700
Subject: [PATCH 6/9] Removed optional argument and placement for
 partition.metadata file writing when log already exists

---
 .../main/scala/kafka/cluster/Partition.scala  |  25 ++--
 .../src/main/scala/kafka/log/LogManager.scala |   2 +-
 .../scala/kafka/server/ReplicaManager.scala   |  14 +-
 .../kafka/cluster/AssignmentStateTest.scala   |   2 +-
 .../kafka/cluster/PartitionLockTest.scala     |  12 +-
 .../unit/kafka/cluster/PartitionTest.scala    | 126 +++++++++---------
 .../scala/unit/kafka/log/LogManagerTest.scala |  36 ++---
 .../test/scala/unit/kafka/log/LogTest.scala   |   2 +-
 .../server/HighwatermarkPersistenceTest.scala |   6 +-
 .../unit/kafka/server/LogOffsetTest.scala     |   4 +-
 .../kafka/server/ReplicaManagerTest.scala     |  55 ++++----
 11 files changed, 136 insertions(+), 148 deletions(-)

diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala
index 2f1d0b521450e..ed8b3533df33f 100755
--- a/core/src/main/scala/kafka/cluster/Partition.scala
+++ b/core/src/main/scala/kafka/cluster/Partition.scala
@@ -308,16 +308,14 @@ class Partition(val topicPartition: TopicPartition,
                 s"different from the requested log dir $logDir")
             false
           case None =>
-            // not sure if topic ID should be none here, but not sure if we have access in ReplicaManager where this is called.
-            // could also use topicId method here potentially. This is only used in ReplicaManager (ZK code) so probably ok to set as None.
-            createLogIfNotExists(isNew = false, isFutureReplica = true, highWatermarkCheckpoints, None)
+            createLogIfNotExists(isNew = false, isFutureReplica = true, highWatermarkCheckpoints, topicId)
             true
         }
       }
     }
   }
 
-  def createLogIfNotExists(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Unit = {
+  def createLogIfNotExists(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid]): Unit = {
     isFutureReplica match {
       case true if futureLog.isEmpty =>
         val log = createLog(isNew, isFutureReplica, offsetCheckpoints, topicId)
@@ -330,7 +328,7 @@ class Partition(val topicPartition: TopicPartition,
   }
 
   // Visible for testing
-  private[cluster] def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Log = {
+  private[cluster] def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid]): Log = {
     def updateHighWatermark(log: Log) = {
       val checkpointHighWatermark = offsetCheckpoints.fetch(log.parentDir, topicPartition).getOrElse {
         info(s"No checkpointed highwatermark is found for partition $topicPartition")
@@ -346,6 +344,11 @@ class Partition(val topicPartition: TopicPartition,
       val log = logManager.getOrCreateLog(topicPartition, isNew, isFutureReplica, topicId)
       maybeLog = Some(log)
       updateHighWatermark(log)
+      // When running a ZK controller, we may get a log that does not have a topic ID. Assign it here.
+      if (log.topicId == None && topicId.isDefined) {
+        log.partitionMetadataFile.write(topicId.get)
+        log.topicId = Some(topicId.get)
+      }
       log
     } finally {
       logManager.finishedInitializingLog(topicPartition, maybeLog)
@@ -434,14 +437,6 @@ class Partition(val topicPartition: TopicPartition,
     log.flatMap(_.topicId)
   }
 
-  /**
-   * @return the topic ID and the partition's log. Return None for values that do not exist
-   */
-  def topicIdAndLog: (Option[Uuid], Option[Log]) = {
-    val log = this.log.orElse(logManager.getLog(topicPartition))
-    (log.flatMap(_.topicId), log)
-  }
-
   // remoteReplicas will be called in the hot path, and must be inexpensive
   def remoteReplicas: Iterable[Replica] =
     remoteReplicasMap.values
@@ -521,7 +516,7 @@ class Partition(val topicPartition: TopicPartition,
    */
   def makeLeader(partitionState: LeaderAndIsrPartitionState,
                  highWatermarkCheckpoints: OffsetCheckpoints,
-                 topicId: Option[Uuid] = None): Boolean = {
+                 topicId: Option[Uuid]): Boolean = {
     val (leaderHWIncremented, isNewLeader) = inWriteLock(leaderIsrUpdateLock) {
       // record the epoch of the controller that made the leadership decision. This is useful while updating the isr
       // to maintain the decision maker controller's epoch in the zookeeper path
@@ -606,7 +601,7 @@ class Partition(val topicPartition: TopicPartition,
    */
   def makeFollower(partitionState: LeaderAndIsrPartitionState,
                    highWatermarkCheckpoints: OffsetCheckpoints,
-                   topicId: Option[Uuid] = None): Boolean = {
+                   topicId: Option[Uuid]): Boolean = {
     inWriteLock(leaderIsrUpdateLock) {
       val newLeaderBrokerId = partitionState.leader
       val oldLeaderEpoch = leaderEpoch
diff --git a/core/src/main/scala/kafka/log/LogManager.scala b/core/src/main/scala/kafka/log/LogManager.scala
index 6b1a17ff4f950..0720af92ca1e8 100755
--- a/core/src/main/scala/kafka/log/LogManager.scala
+++ b/core/src/main/scala/kafka/log/LogManager.scala
@@ -778,7 +778,7 @@ class LogManager(logDirs: Seq[File],
    * @param topicId The topic ID of the topic used in the case of log creation.
    * @throws KafkaStorageException if isNew=false, log is not found in the cache and there is offline log directory on the broker
    */
-  def getOrCreateLog(topicPartition: TopicPartition, isNew: Boolean = false, isFuture: Boolean = false, topicId: Option[Uuid] = None): Log = {
+  def getOrCreateLog(topicPartition: TopicPartition, isNew: Boolean = false, isFuture: Boolean = false, topicId: Option[Uuid]): Log = {
     logCreationOrDeletionLock synchronized {
       getLog(topicPartition, isFuture).getOrElse {
         // create the log if it has not already been created in another thread
diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala
index a4b0133af45fe..3ca0a22800d09 100644
--- a/core/src/main/scala/kafka/server/ReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/ReplicaManager.scala
@@ -1377,8 +1377,7 @@ class ReplicaManager(val config: KafkaConfig,
               val requestLeaderEpoch = partitionState.leaderEpoch
               val requestTopicId = topicIds.get(topicPartition.topic)
 
-              val (topicIdOpt, partitionLogOpt) = partition.topicIdAndLog
-              if (!checkTopicId(requestTopicId, topicIdOpt, partitionLogOpt)) {
+              if (!checkTopicId(requestTopicId, partition.topicId)) {
                 stateChangeLogger.error(s"Topic Id in memory: ${partition.topicId.get} does not" +
                   s" match the topic Id for partition $topicPartition received: " +
                   s"$requestTopicId.")
@@ -1492,24 +1491,17 @@ class ReplicaManager(val config: KafkaConfig,
    * The topic ID was not inconsistent, so return true.
    * If the log does not exist or the topic ID is not yet set, logTopicIdOpt will be None.
    * In both cases, the ID is not inconsistent so return true.
-   * If a valid topic ID is provided, and the log exists but has no ID set, set the log ID to be the request ID.
    *
    * @param requestTopicId the topic ID from the LeaderAndIsr request
-   * @param logTopicIdOpt the topic ID in the log if the log and the topic ID exists
-   * @param partitionLog the log for the partition being checked if it exists
+   * @param logTopicIdOpt the topic ID in the log if the log and the topic ID exist
    * @return true if the request topic id is consistent, false otherwise
    */
-  private def checkTopicId(requestTopicId: Uuid, logTopicIdOpt: Option[Uuid], partitionLog: Option[Log]): Boolean = {
+  private def checkTopicId(requestTopicId: Uuid, logTopicIdOpt: Option[Uuid]): Boolean = {
     if (requestTopicId == null || requestTopicId == Uuid.ZERO_UUID) {
       true
     } else
       logTopicIdOpt match {
         case None =>
-          // if we have valid request ID, log exists, but log topic ID is not yet assigned, write to log
-          partitionLog.foreach(log => {
-            log.partitionMetadataFile.write(requestTopicId)
-            log.topicId = Some(requestTopicId)
-          })
           true
         case Some(logTopicId)  =>
            requestTopicId == logTopicId
diff --git a/core/src/test/scala/unit/kafka/cluster/AssignmentStateTest.scala b/core/src/test/scala/unit/kafka/cluster/AssignmentStateTest.scala
index 5e087c5e407fd..a618825e8a5f1 100644
--- a/core/src/test/scala/unit/kafka/cluster/AssignmentStateTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/AssignmentStateTest.scala
@@ -108,7 +108,7 @@ class AssignmentStateTest extends AbstractPartitionTest {
     if (original.nonEmpty)
       partition.assignmentState = SimpleAssignmentState(original)
     // do the test
-    partition.makeLeader(leaderState, offsetCheckpoints)
+    partition.makeLeader(leaderState, offsetCheckpoints, None)
     assertEquals(isReassigning, partition.isReassigning)
     if (adding.nonEmpty)
       adding.foreach(r => assertTrue(partition.isAddingReplica(r)))
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
index 7d9ca1fbecf1d..a65b6fa4c7c09 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
@@ -142,7 +142,7 @@ class PartitionLockTest extends Logging {
       .setIsNew(true)
     val offsetCheckpoints: OffsetCheckpoints = mock(classOf[OffsetCheckpoints])
     // Update replica set synchronously first to avoid race conditions
-    partition.makeLeader(partitionState(secondReplicaSet), offsetCheckpoints)
+    partition.makeLeader(partitionState(secondReplicaSet), offsetCheckpoints, None)
     assertTrue(partition.getReplica(replicaToCheck).isDefined, s"Expected replica $replicaToCheck to be defined")
 
     val future = executorService.submit((() => {
@@ -155,7 +155,7 @@ class PartitionLockTest extends Logging {
           secondReplicaSet
         }
 
-        partition.makeLeader(partitionState(replicas), offsetCheckpoints)
+        partition.makeLeader(partitionState(replicas), offsetCheckpoints, None)
 
         i += 1
         Thread.sleep(1) // just to avoid tight loop
@@ -278,8 +278,8 @@ class PartitionLockTest extends Logging {
         }
       }
 
-      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Log = {
-        val log = super.createLog(isNew, isFutureReplica, offsetCheckpoints)
+      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid]): Log = {
+        val log = super.createLog(isNew, isFutureReplica, offsetCheckpoints, None)
         new SlowLog(log, mockTime, appendSemaphore)
       }
     }
@@ -288,7 +288,7 @@ class PartitionLockTest extends Logging {
     when(alterIsrManager.submit(ArgumentMatchers.any[AlterIsrItem]))
       .thenReturn(true)
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
 
     val controllerEpoch = 0
     val replicas = (0 to numReplicaFetchers).map(i => Integer.valueOf(brokerId + i)).toList.asJava
@@ -301,7 +301,7 @@ class PartitionLockTest extends Logging {
       .setIsr(isr)
       .setZkVersion(1)
       .setReplicas(replicas)
-      .setIsNew(true), offsetCheckpoints), "Expected become leader transition to succeed")
+      .setIsNew(true), offsetCheckpoints, None), "Expected become leader transition to succeed")
 
     partition
   }
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
index 3b97b3dae2543..6c0a7f40c2f7f 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
@@ -51,7 +51,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testLastFetchedOffsetValidation(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     def append(leaderEpoch: Int, count: Int): Unit = {
       val recordArray = (1 to count).map { i =>
         new SimpleRecord(s"$i".getBytes)
@@ -130,7 +130,7 @@ class PartitionTest extends AbstractPartitionTest {
   def testMakeLeaderUpdatesEpochCache(): Unit = {
     val leaderEpoch = 8
 
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     log.appendAsLeader(MemoryRecords.withRecords(0L, CompressionType.NONE, 0,
       new SimpleRecord("k1".getBytes, "v1".getBytes),
       new SimpleRecord("k2".getBytes, "v2".getBytes)
@@ -156,7 +156,7 @@ class PartitionTest extends AbstractPartitionTest {
 
     configRepository.setTopicConfig(topicPartition.topic,
       LogConfig.MessageFormatVersionProp, kafka.api.KAFKA_0_10_2_IV0.shortVersion)
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     log.appendAsLeader(TestUtils.records(List(
       new SimpleRecord("k1".getBytes, "v1".getBytes),
       new SimpleRecord("k2".getBytes, "v2".getBytes)),
@@ -185,7 +185,7 @@ class PartitionTest extends AbstractPartitionTest {
     val latch = new CountDownLatch(1)
 
     logManager.maybeUpdatePreferredLogDir(topicPartition, logDir1.getAbsolutePath)
-    partition.createLogIfNotExists(isNew = true, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = true, isFutureReplica = false, offsetCheckpoints, None)
     logManager.maybeUpdatePreferredLogDir(topicPartition, logDir2.getAbsolutePath)
     partition.maybeCreateFutureReplica(logDir2.getAbsolutePath, offsetCheckpoints)
 
@@ -230,13 +230,13 @@ class PartitionTest extends AbstractPartitionTest {
       logManager,
       alterIsrManager) {
 
-      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid] = None): Log = {
-        val log = super.createLog(isNew, isFutureReplica, offsetCheckpoints)
+      override def createLog(isNew: Boolean, isFutureReplica: Boolean, offsetCheckpoints: OffsetCheckpoints, topicId: Option[Uuid]): Log = {
+        val log = super.createLog(isNew, isFutureReplica, offsetCheckpoints, None)
         new SlowLog(log, mockTime, appendSemaphore)
       }
     }
 
-    partition.createLogIfNotExists(isNew = true, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = true, isFutureReplica = false, offsetCheckpoints, None)
 
     val appendThread = new Thread {
       override def run(): Unit = {
@@ -257,7 +257,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(List[Integer](0, 1, 2, brokerId).asJava)
       .setIsNew(false)
-    assertTrue(partition.makeFollower(partitionState, offsetCheckpoints))
+    assertTrue(partition.makeFollower(partitionState, offsetCheckpoints, None))
 
     appendSemaphore.release()
     appendThread.join()
@@ -271,7 +271,7 @@ class PartitionTest extends AbstractPartitionTest {
   // active segment
   def testMaybeReplaceCurrentWithFutureReplicaDifferentBaseOffsets(): Unit = {
     logManager.maybeUpdatePreferredLogDir(topicPartition, logDir1.getAbsolutePath)
-    partition.createLogIfNotExists(isNew = true, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = true, isFutureReplica = false, offsetCheckpoints, None)
     logManager.maybeUpdatePreferredLogDir(topicPartition, logDir2.getAbsolutePath)
     partition.maybeCreateFutureReplica(logDir2.getAbsolutePath, offsetCheckpoints)
 
@@ -567,7 +567,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setReplicas(replicas.map(Int.box).asJava)
       .setIsNew(true)
 
-    assertTrue(partition.makeLeader(leaderState, offsetCheckpoints), "Expected first makeLeader() to return 'leader changed'")
+    assertTrue(partition.makeLeader(leaderState, offsetCheckpoints, None), "Expected first makeLeader() to return 'leader changed'")
     assertEquals(leaderEpoch, partition.getLeaderEpoch, "Current leader epoch")
     assertEquals(Set[Integer](leader, follower2), partition.isrState.isr, "ISR")
 
@@ -640,7 +640,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setReplicas(replicas.map(Int.box).asJava)
       .setIsNew(false)
 
-    assertTrue(partition.makeFollower(followerState, offsetCheckpoints))
+    assertTrue(partition.makeFollower(followerState, offsetCheckpoints, None))
 
     // Back to leader, this resets the startLogOffset for this epoch (to 2), we're now in the fault condition
     val newLeaderState = new LeaderAndIsrPartitionState()
@@ -652,7 +652,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setReplicas(replicas.map(Int.box).asJava)
       .setIsNew(false)
 
-    assertTrue(partition.makeLeader(newLeaderState, offsetCheckpoints))
+    assertTrue(partition.makeLeader(newLeaderState, offsetCheckpoints, None))
 
     // Try to get offsets as a client
     fetchOffsetsForTimestamp(ListOffsetsRequest.LATEST_TIMESTAMP, Some(IsolationLevel.READ_UNCOMMITTED)) match {
@@ -713,8 +713,8 @@ class PartitionTest extends AbstractPartitionTest {
 
   private def setupPartitionWithMocks(leaderEpoch: Int,
                                       isLeader: Boolean,
-                                      log: Log = logManager.getOrCreateLog(topicPartition)): Partition = {
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+                                      log: Log = logManager.getOrCreateLog(topicPartition, topicId = None)): Partition = {
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
 
     val controllerEpoch = 0
     val replicas = List[Integer](brokerId, brokerId + 1).asJava
@@ -728,7 +728,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setIsr(isr)
           .setZkVersion(1)
           .setReplicas(replicas)
-          .setIsNew(true), offsetCheckpoints), "Expected become leader transition to succeed")
+          .setIsNew(true), offsetCheckpoints, None), "Expected become leader transition to succeed")
       assertEquals(leaderEpoch, partition.getLeaderEpoch)
     } else {
       assertTrue(partition.makeFollower(new LeaderAndIsrPartitionState()
@@ -738,7 +738,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setIsr(isr)
           .setZkVersion(1)
           .setReplicas(replicas)
-          .setIsNew(true), offsetCheckpoints), "Expected become follower transition to succeed")
+          .setIsNew(true), offsetCheckpoints, None), "Expected become follower transition to succeed")
       assertEquals(leaderEpoch, partition.getLeaderEpoch)
       assertEquals(None, partition.leaderLogIfLocal)
     }
@@ -748,7 +748,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testAppendRecordsAsFollowerBelowLogStartOffset(): Unit = {
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val log = partition.localLogOrException
 
     val initialLogStartOffset = 5L
@@ -802,7 +802,7 @@ class PartitionTest extends AbstractPartitionTest {
     val replicas = List[Integer](brokerId, brokerId + 1).asJava
     val isr = replicas
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
 
     assertTrue(partition.makeLeader(new LeaderAndIsrPartitionState()
         .setControllerEpoch(controllerEpoch)
@@ -811,7 +811,7 @@ class PartitionTest extends AbstractPartitionTest {
         .setIsr(isr)
         .setZkVersion(1)
         .setReplicas(replicas)
-        .setIsNew(true), offsetCheckpoints), "Expected become leader transition to succeed")
+        .setIsNew(true), offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(leaderEpoch, partition.getLeaderEpoch)
 
     val records = createTransactionalRecords(List(
@@ -881,7 +881,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(List[Integer](0, 1, 2, brokerId).asJava)
       .setIsNew(false)
-    partition.makeFollower(partitionState, offsetCheckpoints)
+    partition.makeFollower(partitionState, offsetCheckpoints, None)
 
     // Request with same leader and epoch increases by only 1, do become-follower steps
     partitionState = new LeaderAndIsrPartitionState()
@@ -892,7 +892,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(List[Integer](0, 1, 2, brokerId).asJava)
       .setIsNew(false)
-    assertTrue(partition.makeFollower(partitionState, offsetCheckpoints))
+    assertTrue(partition.makeFollower(partitionState, offsetCheckpoints, None))
 
     // Request with same leader and same epoch, skip become-follower steps
     partitionState = new LeaderAndIsrPartitionState()
@@ -902,7 +902,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setIsr(List[Integer](0, 1, 2, brokerId).asJava)
       .setZkVersion(1)
       .setReplicas(List[Integer](0, 1, 2, brokerId).asJava)
-    assertFalse(partition.makeFollower(partitionState, offsetCheckpoints))
+    assertFalse(partition.makeFollower(partitionState, offsetCheckpoints, None))
   }
 
   @Test
@@ -930,7 +930,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(replicas)
       .setIsNew(true)
-    assertTrue(partition.makeLeader(leaderState, offsetCheckpoints), "Expected first makeLeader() to return 'leader changed'")
+    assertTrue(partition.makeLeader(leaderState, offsetCheckpoints, None), "Expected first makeLeader() to return 'leader changed'")
     assertEquals(leaderEpoch, partition.getLeaderEpoch, "Current leader epoch")
     assertEquals(Set[Integer](leader, follower2), partition.isrState.isr, "ISR")
 
@@ -964,7 +964,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(replicas)
       .setIsNew(false)
-    partition.makeFollower(followerState, offsetCheckpoints)
+    partition.makeFollower(followerState, offsetCheckpoints, None)
 
     val newLeaderState = new LeaderAndIsrPartitionState()
       .setControllerEpoch(controllerEpoch)
@@ -974,7 +974,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(replicas)
       .setIsNew(false)
-    assertTrue(partition.makeLeader(newLeaderState, offsetCheckpoints),
+    assertTrue(partition.makeLeader(newLeaderState, offsetCheckpoints, None),
       "Expected makeLeader() to return 'leader changed' after makeFollower()")
     val currentLeaderEpochStartOffset = partition.localLogOrException.logEndOffset
 
@@ -1043,13 +1043,13 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(replicas)
       .setIsNew(true)
-    partition.makeLeader(leaderState, offsetCheckpoints)
+    partition.makeLeader(leaderState, offsetCheckpoints, None)
     assertTrue(partition.isAtMinIsr)
   }
 
   @Test
   def testUpdateFollowerFetchState(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 6, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1058,7 +1058,7 @@ class PartitionTest extends AbstractPartitionTest {
     val replicas = List[Integer](brokerId, remoteBrokerId).asJava
     val isr = replicas
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
 
     val initializeTimeMs = time.milliseconds()
     assertTrue(partition.makeLeader(
@@ -1070,7 +1070,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
 
     val remoteReplica = partition.getReplica(remoteBrokerId).get
     assertEquals(initializeTimeMs, remoteReplica.lastCaughtUpTimeMs)
@@ -1105,7 +1105,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testIsrExpansion(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1114,7 +1114,7 @@ class PartitionTest extends AbstractPartitionTest {
     val replicas = List(brokerId, remoteBrokerId)
     val isr = List[Integer](brokerId).asJava
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1124,7 +1124,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas.map(Int.box).asJava)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId), partition.isrState.isr)
 
     val remoteReplica = partition.getReplica(remoteBrokerId).get
@@ -1166,7 +1166,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testIsrNotExpandedIfUpdateFails(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1175,7 +1175,7 @@ class PartitionTest extends AbstractPartitionTest {
     val replicas = List[Integer](brokerId, remoteBrokerId).asJava
     val isr = List[Integer](brokerId).asJava
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1185,7 +1185,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId), partition.isrState.isr)
 
     val remoteReplica = partition.getReplica(remoteBrokerId).get
@@ -1220,7 +1220,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testMaybeShrinkIsr(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1230,7 +1230,7 @@ class PartitionTest extends AbstractPartitionTest {
     val isr = List[Integer](brokerId, remoteBrokerId).asJava
 
     val initializeTimeMs = time.milliseconds()
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1240,7 +1240,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas.map(Int.box).asJava)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId, remoteBrokerId), partition.isrState.isr)
     assertEquals(0L, partition.localLogOrException.highWatermark)
 
@@ -1267,7 +1267,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testShouldNotShrinkIsrIfPreviousFetchIsCaughtUp(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1277,7 +1277,7 @@ class PartitionTest extends AbstractPartitionTest {
     val isr = List[Integer](brokerId, remoteBrokerId).asJava
 
     val initializeTimeMs = time.milliseconds()
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1287,7 +1287,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas.map(Int.box).asJava)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId, remoteBrokerId), partition.isrState.isr)
     assertEquals(0L, partition.localLogOrException.highWatermark)
 
@@ -1332,7 +1332,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testShouldNotShrinkIsrIfFollowerCaughtUpToLogEnd(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1342,7 +1342,7 @@ class PartitionTest extends AbstractPartitionTest {
     val isr = List[Integer](brokerId, remoteBrokerId).asJava
 
     val initializeTimeMs = time.milliseconds()
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1352,7 +1352,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas.map(Int.box).asJava)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId, remoteBrokerId), partition.isrState.isr)
     assertEquals(0L, partition.localLogOrException.highWatermark)
 
@@ -1383,7 +1383,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testIsrNotShrunkIfUpdateFails(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1393,7 +1393,7 @@ class PartitionTest extends AbstractPartitionTest {
     val isr = List[Integer](brokerId, remoteBrokerId).asJava
 
     val initializeTimeMs = time.milliseconds()
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1403,7 +1403,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId, remoteBrokerId), partition.inSyncReplicaIds)
     assertEquals(0L, partition.localLogOrException.highWatermark)
 
@@ -1462,7 +1462,7 @@ class PartitionTest extends AbstractPartitionTest {
   }
 
   def handleAlterIsrFailure(error: Errors, callback: (Int, Int, Partition) => Unit): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1471,7 +1471,7 @@ class PartitionTest extends AbstractPartitionTest {
     val replicas = List[Integer](brokerId, remoteBrokerId).asJava
     val isr = List[Integer](brokerId).asJava
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1481,7 +1481,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId), partition.isrState.isr)
 
     val remoteReplica = partition.getReplica(remoteBrokerId).get
@@ -1509,7 +1509,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testSingleInFlightAlterIsr(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1522,7 +1522,7 @@ class PartitionTest extends AbstractPartitionTest {
 
     doNothing().when(delayedOperations).checkAndCompleteAll()
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1532,7 +1532,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId, follower1, follower2), partition.isrState.isr)
     assertEquals(0L, partition.localLogOrException.highWatermark)
 
@@ -1574,7 +1574,7 @@ class PartitionTest extends AbstractPartitionTest {
       logManager,
       zkIsrManager)
 
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 10, leaderEpoch = 4)
 
     val controllerEpoch = 0
@@ -1587,7 +1587,7 @@ class PartitionTest extends AbstractPartitionTest {
 
     doNothing().when(delayedOperations).checkAndCompleteAll()
 
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     assertTrue(partition.makeLeader(
         new LeaderAndIsrPartitionState()
           .setControllerEpoch(controllerEpoch)
@@ -1597,7 +1597,7 @@ class PartitionTest extends AbstractPartitionTest {
           .setZkVersion(1)
           .setReplicas(replicas)
           .setIsNew(true),
-        offsetCheckpoints), "Expected become leader transition to succeed")
+        offsetCheckpoints, None), "Expected become leader transition to succeed")
     assertEquals(Set(brokerId, follower1, follower2), partition.isrState.isr)
     assertEquals(0L, partition.localLogOrException.highWatermark)
 
@@ -1617,7 +1617,7 @@ class PartitionTest extends AbstractPartitionTest {
 
   @Test
   def testUseCheckpointToInitializeHighWatermark(): Unit = {
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     seedLogData(log, numRecords = 6, leaderEpoch = 5)
 
     when(offsetCheckpoints.fetch(logDir1.getAbsolutePath, topicPartition))
@@ -1633,7 +1633,7 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(replicas)
       .setIsNew(false)
-    partition.makeLeader(leaderState, offsetCheckpoints)
+    partition.makeLeader(leaderState, offsetCheckpoints, None)
     assertEquals(4, partition.localLogOrException.highWatermark)
   }
 
@@ -1766,11 +1766,11 @@ class PartitionTest extends AbstractPartitionTest {
       .setZkVersion(1)
       .setReplicas(replicas)
       .setIsNew(false)
-    partition.makeLeader(leaderState, offsetCheckpoints)
+    partition.makeLeader(leaderState, offsetCheckpoints, None)
     assertTrue(partition.isUnderReplicated)
 
     leaderState = leaderState.setIsr(replicas)
-    partition.makeLeader(leaderState, offsetCheckpoints)
+    partition.makeLeader(leaderState, offsetCheckpoints, None)
     assertFalse(partition.isUnderReplicated)
   }
 
@@ -1830,7 +1830,7 @@ class PartitionTest extends AbstractPartitionTest {
       spyLogManager,
       alterIsrManager)
 
-    partition.createLog(isNew = true, isFutureReplica = false, offsetCheckpoints)
+    partition.createLog(isNew = true, isFutureReplica = false, offsetCheckpoints, topicId = None)
 
     // Validate that initializingLog and finishedInitializingLog was called
     verify(spyLogManager).initializingLog(ArgumentMatchers.eq(topicPartition))
@@ -1868,7 +1868,7 @@ class PartitionTest extends AbstractPartitionTest {
       spyLogManager,
       alterIsrManager)
 
-    partition.createLog(isNew = true, isFutureReplica = false, offsetCheckpoints)
+    partition.createLog(isNew = true, isFutureReplica = false, offsetCheckpoints, topicId = None)
 
     // Validate that initializingLog and finishedInitializingLog was called
     verify(spyLogManager).initializingLog(ArgumentMatchers.eq(topicPartition))
@@ -1909,7 +1909,7 @@ class PartitionTest extends AbstractPartitionTest {
       spyLogManager,
       alterIsrManager)
 
-    partition.createLog(isNew = true, isFutureReplica = false, offsetCheckpoints)
+    partition.createLog(isNew = true, isFutureReplica = false, offsetCheckpoints, topicId = None)
 
     // Validate that initializingLog and finishedInitializingLog was called
     verify(spyLogManager).initializingLog(ArgumentMatchers.eq(topicPartition))
diff --git a/core/src/test/scala/unit/kafka/log/LogManagerTest.scala b/core/src/test/scala/unit/kafka/log/LogManagerTest.scala
index c3698dcc6ca66..b679fbb712b74 100755
--- a/core/src/test/scala/unit/kafka/log/LogManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogManagerTest.scala
@@ -79,7 +79,7 @@ class LogManagerTest {
    */
   @Test
   def testCreateLog(): Unit = {
-    val log = logManager.getOrCreateLog(new TopicPartition(name, 0))
+    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), topicId = None)
     assertEquals(1, logManager.liveLogDirs.size)
 
     val logFile = new File(logDir, name + "-0")
@@ -104,8 +104,8 @@ class LogManagerTest {
       assertEquals(2, logManagerForTest.get.liveLogDirs.size)
       logManagerForTest.get.startup(Set.empty)
 
-      val log1 = logManagerForTest.get.getOrCreateLog(new TopicPartition(name, 0))
-      val log2 = logManagerForTest.get.getOrCreateLog(new TopicPartition(name, 1))
+      val log1 = logManagerForTest.get.getOrCreateLog(new TopicPartition(name, 0), topicId = None)
+      val log2 = logManagerForTest.get.getOrCreateLog(new TopicPartition(name, 1), topicId = None)
 
       val logFile1 = new File(logDir1, name + "-0")
       assertTrue(logFile1.exists)
@@ -147,7 +147,7 @@ class LogManagerTest {
     logManager = createLogManager(dirs)
     logManager.startup(Set.empty)
 
-    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), isNew = true)
+    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), isNew = true, topicId = None)
     val logFile = new File(logDir, name + "-0")
     assertTrue(logFile.exists)
     log.appendAsLeader(TestUtils.singletonRecords("test".getBytes()), leaderEpoch = 0)
@@ -179,7 +179,7 @@ class LogManagerTest {
 
     // Request creating a new log.
     // LogManager should try using all configured log directories until one succeeds.
-    logManager.getOrCreateLog(new TopicPartition(name, 0), isNew = true)
+    logManager.getOrCreateLog(new TopicPartition(name, 0), isNew = true, topicId = None)
 
     // Verify that half the directories were considered broken,
     assertEquals(dirs.length / 2, brokenDirs.size)
@@ -208,7 +208,7 @@ class LogManagerTest {
    */
   @Test
   def testCleanupExpiredSegments(): Unit = {
-    val log = logManager.getOrCreateLog(new TopicPartition(name, 0))
+    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), topicId = None)
     var offset = 0L
     for(_ <- 0 until 200) {
       val set = TestUtils.singletonRecords("test".getBytes())
@@ -254,7 +254,7 @@ class LogManagerTest {
     logManager.startup(Set.empty)
 
     // create a log
-    val log = logManager.getOrCreateLog(new TopicPartition(name, 0))
+    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), topicId = None)
     var offset = 0L
 
     // add a bunch of messages that should be larger than the retentionSize
@@ -306,7 +306,7 @@ class LogManagerTest {
     configRepository.setTopicConfig(name, LogConfig.CleanupPolicyProp, policy)
 
     logManager = createLogManager(configRepository = configRepository)
-    val log = logManager.getOrCreateLog(new TopicPartition(name, 0))
+    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), topicId = None)
     var offset = 0L
     for (_ <- 0 until 200) {
       val set = TestUtils.singletonRecords("test".getBytes(), key="test".getBytes())
@@ -334,7 +334,7 @@ class LogManagerTest {
 
     logManager = createLogManager(configRepository = configRepository)
     logManager.startup(Set.empty)
-    val log = logManager.getOrCreateLog(new TopicPartition(name, 0))
+    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), topicId = None)
     val lastFlush = log.lastFlushTime
     for (_ <- 0 until 200) {
       val set = TestUtils.singletonRecords("test".getBytes())
@@ -358,7 +358,7 @@ class LogManagerTest {
 
     // verify that logs are always assigned to the least loaded partition
     for(partition <- 0 until 20) {
-      logManager.getOrCreateLog(new TopicPartition("test", partition))
+      logManager.getOrCreateLog(new TopicPartition("test", partition), topicId = None)
       assertEquals(partition + 1, logManager.allLogs.size, "We should have created the right number of logs")
       val counts = logManager.allLogs.groupBy(_.dir.getParent).values.map(_.size)
       assertTrue(counts.max <= counts.min + 1, "Load should balance evenly")
@@ -404,7 +404,7 @@ class LogManagerTest {
   }
 
   private def verifyCheckpointRecovery(topicPartitions: Seq[TopicPartition], logManager: LogManager, logDir: File): Unit = {
-    val logs = topicPartitions.map(logManager.getOrCreateLog(_))
+    val logs = topicPartitions.map(logManager.getOrCreateLog(_, topicId = None))
     logs.foreach { log =>
       for (_ <- 0 until 50)
         log.appendAsLeader(TestUtils.singletonRecords("test".getBytes()), leaderEpoch = 0)
@@ -431,7 +431,7 @@ class LogManagerTest {
 
   @Test
   def testFileReferencesAfterAsyncDelete(): Unit = {
-    val log = logManager.getOrCreateLog(new TopicPartition(name, 0))
+    val log = logManager.getOrCreateLog(new TopicPartition(name, 0), topicId = None)
     val activeSegment = log.activeSegment
     val logName = activeSegment.log.file.getName
     val indexName = activeSegment.offsetIndex.file.getName
@@ -468,7 +468,7 @@ class LogManagerTest {
   @Test
   def testCreateAndDeleteOverlyLongTopic(): Unit = {
     val invalidTopicName = String.join("", Collections.nCopies(253, "x"))
-    logManager.getOrCreateLog(new TopicPartition(invalidTopicName, 0))
+    logManager.getOrCreateLog(new TopicPartition(invalidTopicName, 0), topicId = None)
     logManager.asyncDelete(new TopicPartition(invalidTopicName, 0))
   }
 
@@ -481,7 +481,7 @@ class LogManagerTest {
       new TopicPartition("test-b", 0),
       new TopicPartition("test-b", 1))
 
-    val allLogs = tps.map(logManager.getOrCreateLog(_))
+    val allLogs = tps.map(logManager.getOrCreateLog(_, topicId = None))
     allLogs.foreach { log =>
       for (_ <- 0 until 50)
         log.appendAsLeader(TestUtils.singletonRecords("test".getBytes), leaderEpoch = 0)
@@ -616,7 +616,7 @@ class LogManagerTest {
     }
 
     // Create the Log and assert that the metrics are present
-    logManager.getOrCreateLog(tp)
+    logManager.getOrCreateLog(tp, topicId = None)
     verifyMetrics()
 
     // Trigger the deletion and assert that the metrics have been removed
@@ -624,7 +624,7 @@ class LogManagerTest {
     assertTrue(logMetrics.isEmpty)
 
     // Recreate the Log and assert that the metrics are present
-    logManager.getOrCreateLog(tp)
+    logManager.getOrCreateLog(tp, topicId = None)
     verifyMetrics()
 
     // Advance time past the file deletion delay and assert that the removed log has been deleted but the metrics
@@ -657,9 +657,9 @@ class LogManagerTest {
 
     // Create the current and future logs and verify that metrics are present for both current and future logs
     logManager.maybeUpdatePreferredLogDir(tp, dir1.getAbsolutePath)
-    logManager.getOrCreateLog(tp)
+    logManager.getOrCreateLog(tp, topicId = None)
     logManager.maybeUpdatePreferredLogDir(tp, dir2.getAbsolutePath)
-    logManager.getOrCreateLog(tp, isFuture = true)
+    logManager.getOrCreateLog(tp, isFuture = true, topicId = None)
     verifyMetrics(2)
 
     // Replace the current log with the future one and verify that only one set of metrics are present
diff --git a/core/src/test/scala/unit/kafka/log/LogTest.scala b/core/src/test/scala/unit/kafka/log/LogTest.scala
index fbed2aedda50b..f44b9064dba9e 100755
--- a/core/src/test/scala/unit/kafka/log/LogTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogTest.scala
@@ -127,7 +127,7 @@ class LogTest {
 
     val cleanShutdownFile = new File(logDir, Log.CleanShutdownFile)
     val logManager: LogManager = interceptedLogManager(logConfig, logDirs)
-    log = logManager.getOrCreateLog(topicPartition, isNew = true)
+    log = logManager.getOrCreateLog(topicPartition, isNew = true, topicId = None)
 
     // Load logs after a clean shutdown
     Files.createFile(cleanShutdownFile.toPath)
diff --git a/core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala b/core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala
index c39ae7cf03e4f..d7a16d22cc4c7 100755
--- a/core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala
+++ b/core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala
@@ -75,7 +75,7 @@ class HighwatermarkPersistenceTest {
       val tp0 = new TopicPartition(topic, 0)
       val partition0 = replicaManager.createPartition(tp0)
       // create leader and follower replicas
-      val log0 = logManagers.head.getOrCreateLog(new TopicPartition(topic, 0))
+      val log0 = logManagers.head.getOrCreateLog(new TopicPartition(topic, 0), topicId = None)
       partition0.setLog(log0, isFutureLog = false)
 
       partition0.updateAssignmentAndIsr(
@@ -124,7 +124,7 @@ class HighwatermarkPersistenceTest {
       val t1p0 = new TopicPartition(topic1, 0)
       val topic1Partition0 = replicaManager.createPartition(t1p0)
       // create leader log
-      val topic1Log0 = logManagers.head.getOrCreateLog(t1p0)
+      val topic1Log0 = logManagers.head.getOrCreateLog(t1p0, topicId = None)
       // create a local replica for topic1
       topic1Partition0.setLog(topic1Log0, isFutureLog = false)
       replicaManager.checkpointHighWatermarks()
@@ -141,7 +141,7 @@ class HighwatermarkPersistenceTest {
       val t2p0 = new TopicPartition(topic2, 0)
       val topic2Partition0 = replicaManager.createPartition(t2p0)
       // create leader log
-      val topic2Log0 = logManagers.head.getOrCreateLog(t2p0)
+      val topic2Log0 = logManagers.head.getOrCreateLog(t2p0, topicId = None)
       // create a local replica for topic2
       topic2Partition0.setLog(topic2Log0, isFutureLog = false)
       replicaManager.checkpointHighWatermarks()
diff --git a/core/src/test/scala/unit/kafka/server/LogOffsetTest.scala b/core/src/test/scala/unit/kafka/server/LogOffsetTest.scala
index 746374583434d..bf9ad3e435504 100755
--- a/core/src/test/scala/unit/kafka/server/LogOffsetTest.scala
+++ b/core/src/test/scala/unit/kafka/server/LogOffsetTest.scala
@@ -159,7 +159,7 @@ class LogOffsetTest extends BaseRequestTest {
     createTopic(topic, 3, 1)
 
     val logManager = server.getLogManager
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
 
     for (_ <- 0 until 20)
       log.appendAsLeader(TestUtils.singletonRecords(value = Integer.toString(42).getBytes()), leaderEpoch = 0)
@@ -188,7 +188,7 @@ class LogOffsetTest extends BaseRequestTest {
     createTopic(topic, 3, 1)
 
     val logManager = server.getLogManager
-    val log = logManager.getOrCreateLog(topicPartition)
+    val log = logManager.getOrCreateLog(topicPartition, topicId = None)
     for (_ <- 0 until 20)
       log.appendAsLeader(TestUtils.singletonRecords(value = Integer.toString(42).getBytes()), leaderEpoch = 0)
     log.flush()
diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
index 038a63890992b..56f37dbcc8a5f 100644
--- a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
@@ -97,7 +97,7 @@ class ReplicaManagerTest {
     try {
       val partition = rm.createPartition(new TopicPartition(topic, 1))
       partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
-        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints))
+        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints), None)
       rm.checkpointHighWatermarks()
     } finally {
       // shutdown the replica manager upon test completion
@@ -117,7 +117,7 @@ class ReplicaManagerTest {
     try {
       val partition = rm.createPartition(new TopicPartition(topic, 1))
       partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
-        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints))
+        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints), None)
       rm.checkpointHighWatermarks()
     } finally {
       // shutdown the replica manager upon test completion
@@ -171,7 +171,7 @@ class ReplicaManagerTest {
 
       val partition = rm.createPartition(new TopicPartition(topic, 0))
       partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
-        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints))
+        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints), None)
       // Make this replica the leader.
       val leaderAndIsrRequest1 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
         Seq(new LeaderAndIsrPartitionState()
@@ -231,7 +231,7 @@ class ReplicaManagerTest {
       val topicPartition = new TopicPartition(topic, 0)
       replicaManager.createPartition(topicPartition)
         .createLogIfNotExists(isNew = false, isFutureReplica = false,
-          new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints))
+          new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints), None)
       val topicIds = Collections.singletonMap(topic, Uuid.randomUuid())
 
       def leaderAndIsrRequest(epoch: Int): LeaderAndIsrRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -291,7 +291,7 @@ class ReplicaManagerTest {
 
       val partition = replicaManager.createPartition(new TopicPartition(topic, 0))
       partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
-        new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints))
+        new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints), None)
 
       // Make this replica the leader.
       val leaderAndIsrRequest1 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -352,7 +352,7 @@ class ReplicaManagerTest {
 
       val partition = replicaManager.createPartition(new TopicPartition(topic, 0))
       partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
-        new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints))
+        new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints), None)
 
       // Make this replica the leader.
       val leaderAndIsrRequest1 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -459,7 +459,7 @@ class ReplicaManagerTest {
       val brokerList = Seq[Integer](0, 1).asJava
       val partition = replicaManager.createPartition(new TopicPartition(topic, 0))
       partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
-        new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints))
+        new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints), None)
 
       // Make this replica the leader.
       val leaderAndIsrRequest1 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -536,7 +536,7 @@ class ReplicaManagerTest {
 
       val partition = rm.createPartition(new TopicPartition(topic, 0))
       partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
-        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints))
+        new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints), None)
 
       // Make this replica the leader.
       val leaderAndIsrRequest1 = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -696,8 +696,8 @@ class ReplicaManagerTest {
       val tp0 = new TopicPartition(topic, 0)
       val tp1 = new TopicPartition(topic, 1)
       val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-      replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
-      replicaManager.createPartition(tp1).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+      replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
+      replicaManager.createPartition(tp1).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
       val partition0Replicas = Seq[Integer](0, 1).asJava
       val partition1Replicas = Seq[Integer](0, 2).asJava
       val topicIds = Map(tp0.topic -> Uuid.randomUuid(), tp1.topic -> Uuid.randomUuid()).asJava
@@ -828,10 +828,11 @@ class ReplicaManagerTest {
     val tp = new TopicPartition(topic, topicPartition)
     val partition = replicaManager.createPartition(tp)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     partition.makeFollower(
       leaderAndIsrPartitionState(tp, leaderEpoch, leaderBrokerId, aliveBrokerIds),
-      offsetCheckpoints)
+      offsetCheckpoints,
+      None)
 
     // Make local partition a follower - because epoch increased by more than 1, truncation should
     // trigger even though leader does not change
@@ -869,11 +870,11 @@ class ReplicaManagerTest {
     val partition = replicaManager.createPartition(tp)
 
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     partition.makeLeader(
       leaderAndIsrPartitionState(tp, leaderEpoch, leaderBrokerId, aliveBrokerIds),
-      offsetCheckpoints
-    )
+      offsetCheckpoints,
+      None)
 
     val metadata: ClientMetadata = new DefaultClientMetadata("rack-a", "client-id",
       InetAddress.getByName("localhost"), KafkaPrincipal.ANONYMOUS, "default")
@@ -1099,7 +1100,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val partition0Replicas = Seq[Integer](0, 1).asJava
     val becomeFollowerRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
       Seq(new LeaderAndIsrPartitionState()
@@ -1139,7 +1140,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val partition0Replicas = Seq[Integer](0, 1).asJava
 
     val becomeLeaderRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -1185,7 +1186,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val partition0Replicas = Seq[Integer](0, 1).asJava
     val topicIds = Collections.singletonMap(tp0.topic, Uuid.randomUuid())
 
@@ -1236,7 +1237,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val partition0Replicas = Seq[Integer](0, 1).asJava
     val topicIds = Collections.singletonMap(tp0.topic, Uuid.randomUuid())
 
@@ -1287,7 +1288,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val partition0Replicas = Seq[Integer](0, 1).asJava
 
     val becomeLeaderRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -1330,7 +1331,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val partition0Replicas = Seq[Integer](0, 1).asJava
 
     val becomeLeaderRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -1374,7 +1375,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
     val partition0Replicas = Seq[Integer](0, 1).asJava
 
     val becomeLeaderRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
@@ -1981,7 +1982,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
 
     val becomeLeaderRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 10, brokerEpoch,
       Seq(leaderAndIsrPartitionState(tp0, 1, 0, Seq(0, 1), true)).asJava,
@@ -2008,7 +2009,7 @@ class ReplicaManagerTest {
 
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
-    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
 
     val becomeLeaderRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
       Seq(leaderAndIsrPartitionState(tp0, 1, 0, Seq(0, 1), true)).asJava,
@@ -2050,7 +2051,7 @@ class ReplicaManagerTest {
     val replicaManager = setupReplicaManagerWithMockedPurgatories(mockTimer, aliveBrokerIds = Seq(0, 1))
 
     val tp0 = new TopicPartition(topic, 0)
-    val log = replicaManager.logManager.getOrCreateLog(tp0, true)
+    val log = replicaManager.logManager.getOrCreateLog(tp0, true, topicId = None)
 
     if (throwIOException) {
       // Delete the underlying directory to trigger an KafkaStorageException
@@ -2145,7 +2146,7 @@ class ReplicaManagerTest {
     val tp0 = new TopicPartition(topic, 0)
     val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints)
     val partition = replicaManager.createPartition(tp0)
-    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints)
+    partition.createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
 
     val logDirFailureChannel = new LogDirFailureChannel(replicaManager.config.logDirs.size)
     val logDir = partition.log.get.parentDirFile
@@ -2275,7 +2276,7 @@ class ReplicaManagerTest {
       val brokerList = Seq[Integer](0, 1).asJava
       val topicPartition = new TopicPartition(topic, 0)
 
-      replicaManager.logManager.getOrCreateLog(topicPartition, isNew = true)
+      replicaManager.logManager.getOrCreateLog(topicPartition, isNew = true, topicId = None)
 
       assertTrue(replicaManager.getLog(topicPartition).isDefined)
       var log = replicaManager.getLog(topicPartition).get

From ad65a2515d75c46864cdbda86b0a593ae59b130d Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Fri, 19 Mar 2021 15:20:32 -0700
Subject: [PATCH 7/9] Removed optional arguments, minor style changes

---
 .../main/scala/kafka/cluster/Partition.scala    |  5 ++---
 core/src/main/scala/kafka/log/Log.scala         | 15 +++++++++++----
 core/src/main/scala/kafka/log/LogManager.scala  |  1 +
 .../scala/kafka/raft/KafkaMetadataLog.scala     |  1 +
 .../scala/kafka/server/RaftReplicaManager.scala |  7 +++----
 .../scala/kafka/server/ReplicaManager.scala     | 17 ++++++-----------
 .../test/scala/other/kafka/StressTestLog.scala  |  3 ++-
 .../other/kafka/TestLinearWriteSpeed.scala      |  2 +-
 .../unit/kafka/cluster/PartitionLockTest.scala  |  3 ++-
 .../unit/kafka/cluster/PartitionTest.scala      |  3 ++-
 .../scala/unit/kafka/cluster/ReplicaTest.scala  |  3 ++-
 .../log/AbstractLogCleanerIntegrationTest.scala |  3 ++-
 .../unit/kafka/log/BrokerCompressionTest.scala  |  2 +-
 .../unit/kafka/log/LogCleanerManagerTest.scala  |  7 ++++---
 .../scala/unit/kafka/log/LogCleanerTest.scala   |  5 +++--
 .../unit/kafka/log/LogConcurrencyTest.scala     |  3 ++-
 .../src/test/scala/unit/kafka/log/LogTest.scala | 16 ++++++++++------
 .../unit/kafka/tools/DumpLogSegmentsTest.scala  |  2 +-
 .../scala/unit/kafka/utils/SchedulerTest.scala  |  2 +-
 19 files changed, 57 insertions(+), 43 deletions(-)

diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala
index ed8b3533df33f..e94567a025674 100755
--- a/core/src/main/scala/kafka/cluster/Partition.scala
+++ b/core/src/main/scala/kafka/cluster/Partition.scala
@@ -345,9 +345,8 @@ class Partition(val topicPartition: TopicPartition,
       maybeLog = Some(log)
       updateHighWatermark(log)
       // When running a ZK controller, we may get a log that does not have a topic ID. Assign it here.
-      if (log.topicId == None && topicId.isDefined) {
-        log.partitionMetadataFile.write(topicId.get)
-        log.topicId = Some(topicId.get)
+      if (log.topicId == None) {
+        topicId.foreach(topicId => log.writeTopicIdToExistingLog(topicId))
       }
       log
     } finally {
diff --git a/core/src/main/scala/kafka/log/Log.scala b/core/src/main/scala/kafka/log/Log.scala
index 7d85a8deb4c53..13a4e5e3fdd08 100644
--- a/core/src/main/scala/kafka/log/Log.scala
+++ b/core/src/main/scala/kafka/log/Log.scala
@@ -266,7 +266,7 @@ class Log(@volatile private var _dir: File,
           val producerStateManager: ProducerStateManager,
           logDirFailureChannel: LogDirFailureChannel,
           private val hadCleanShutdown: Boolean = true,
-          @volatile var topicId : Option[Uuid] = None,
+          @volatile var topicId: Option[Uuid],
           val keepPartitionMetadataFile: Boolean = true) extends Logging with KafkaMetricsGroup {
 
   import kafka.log.Log._
@@ -360,8 +360,9 @@ class Log(@volatile private var _dir: File,
           partitionMetadataFile.delete()
         else {
           val fileTopicId = partitionMetadataFile.read().topicId
-          if (topicId.isDefined && fileTopicId != topicId.get)
-            throw new IllegalStateException(s"Tried to assign topic ID $topicId to log, but log already contained topicId $fileTopicId")
+          if (topicId.isDefined && !topicId.contains(fileTopicId))
+            throw new IllegalStateException(s"Tried to assign topic ID $topicId to log for topic partition $topicPartition," +
+              s"but log already contained topic ID $fileTopicId")
           topicId = Some(fileTopicId)
         }
     } else if (topicId.isDefined && keepPartitionMetadataFile) {
@@ -588,6 +589,12 @@ class Log(@volatile private var _dir: File,
     partitionMetadataFile = new PartitionMetadataFile(partitionMetadata, logDirFailureChannel)
   }
 
+  /** Only used for ZK clusters when we update and start using topic IDs on existing topics */
+  def writeTopicIdToExistingLog(topicId: Uuid): Unit = {
+    partitionMetadataFile.write(topicId)
+    this.topicId = Some(topicId)
+  }
+
   private def initializeLeaderEpochCache(): Unit = lock synchronized {
     val leaderEpochFile = LeaderEpochCheckpointFile.newFile(dir)
 
@@ -2612,7 +2619,7 @@ object Log {
             producerIdExpirationCheckIntervalMs: Int,
             logDirFailureChannel: LogDirFailureChannel,
             lastShutdownClean: Boolean = true,
-            topicId: Option[Uuid] = None,
+            topicId: Option[Uuid],
             keepPartitionMetadataFile: Boolean = true): Log = {
     val topicPartition = Log.parseTopicPartitionName(dir)
     val producerStateManager = new ProducerStateManager(topicPartition, dir, maxProducerIdExpirationMs)
diff --git a/core/src/main/scala/kafka/log/LogManager.scala b/core/src/main/scala/kafka/log/LogManager.scala
index 0720af92ca1e8..2d995e73f0bfe 100755
--- a/core/src/main/scala/kafka/log/LogManager.scala
+++ b/core/src/main/scala/kafka/log/LogManager.scala
@@ -270,6 +270,7 @@ class LogManager(logDirs: Seq[File],
       brokerTopicStats = brokerTopicStats,
       logDirFailureChannel = logDirFailureChannel,
       lastShutdownClean = hadCleanShutdown,
+      topicId = None,
       keepPartitionMetadataFile = keepPartitionMetadataFile)
 
     if (logDir.getName.endsWith(Log.DeleteDirSuffix)) {
diff --git a/core/src/main/scala/kafka/raft/KafkaMetadataLog.scala b/core/src/main/scala/kafka/raft/KafkaMetadataLog.scala
index 037e3c91db165..f3b7c3fab1a54 100644
--- a/core/src/main/scala/kafka/raft/KafkaMetadataLog.scala
+++ b/core/src/main/scala/kafka/raft/KafkaMetadataLog.scala
@@ -345,6 +345,7 @@ object KafkaMetadataLog {
       maxProducerIdExpirationMs = Int.MaxValue,
       producerIdExpirationCheckIntervalMs = Int.MaxValue,
       logDirFailureChannel = new LogDirFailureChannel(5),
+      topicId = None,
       keepPartitionMetadataFile = false
     )
 
diff --git a/core/src/main/scala/kafka/server/RaftReplicaManager.scala b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
index b0af58612432a..000b000db8d2e 100644
--- a/core/src/main/scala/kafka/server/RaftReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
@@ -392,14 +392,13 @@ class RaftReplicaManager(config: KafkaConfig,
         logTopicIdOpt.foreach(logTopicId => {
           if (receivedTopicId != logTopicId) {
             // not sure if we need both the logger and the error thrown
-            stateChangeLogger.error(s"Topic Id in memory: $logTopicId does not" +
-              s" match the topic Id for partition $topicPartition received: " +
+            stateChangeLogger.error(s"Topic ID in memory: $logTopicId does not" +
+              s" match the topic ID for partition $topicPartition received: " +
               s"$receivedTopicId.")
             throw new InconsistentTopicIdException(s"Topic partition $topicPartition had an inconsistent topic ID.")
           }
         })
-      case None => throw new IllegalStateException(
-        s"Topic partition $topicPartition is missing a topic ID")
+      case None => throw new IllegalStateException(s"Topic partition $topicPartition is missing a topic ID")
     }
   }
 }
diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala
index 3ca0a22800d09..6319b3eb2809b 100644
--- a/core/src/main/scala/kafka/server/ReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/ReplicaManager.scala
@@ -1326,7 +1326,7 @@ class ReplicaManager(val config: KafkaConfig,
             s"epoch ${leaderAndIsrRequest.controllerEpoch}")
         }
       val topicIds = leaderAndIsrRequest.topicIds()
-      def getTopicId (topicName: String): Option[Uuid] = {
+      def getTopicId(topicName: String): Option[Uuid] = {
         val topicId = topicIds.get(topicName)
         // if invalid topic ID return None
         if (topicId == null || topicId == Uuid.ZERO_UUID)
@@ -1377,9 +1377,9 @@ class ReplicaManager(val config: KafkaConfig,
               val requestLeaderEpoch = partitionState.leaderEpoch
               val requestTopicId = topicIds.get(topicPartition.topic)
 
-              if (!checkTopicId(requestTopicId, partition.topicId)) {
-                stateChangeLogger.error(s"Topic Id in memory: ${partition.topicId.get} does not" +
-                  s" match the topic Id for partition $topicPartition received: " +
+              if (!hasConsistentTopicId(requestTopicId, partition.topicId)) {
+                stateChangeLogger.error(s"Topic ID in memory: ${partition.topicId.get} does not" +
+                  s" match the topic ID for partition $topicPartition received: " +
                   s"$requestTopicId.")
                 responseMap.put(topicPartition, Errors.INCONSISTENT_TOPIC_ID)
               } else if (requestLeaderEpoch > currentLeaderEpoch) {
@@ -1496,16 +1496,11 @@ class ReplicaManager(val config: KafkaConfig,
    * @param logTopicIdOpt the topic ID in the log if the log and the topic ID exist
    * @return true if the request topic id is consistent, false otherwise
    */
-  private def checkTopicId(requestTopicId: Uuid, logTopicIdOpt: Option[Uuid]): Boolean = {
+  private def hasConsistentTopicId(requestTopicId: Uuid, logTopicIdOpt: Option[Uuid]): Boolean = {
     if (requestTopicId == null || requestTopicId == Uuid.ZERO_UUID) {
       true
     } else
-      logTopicIdOpt match {
-        case None =>
-          true
-        case Some(logTopicId)  =>
-           requestTopicId == logTopicId
-    }
+      logTopicIdOpt.isEmpty || logTopicIdOpt.contains(requestTopicId)
   }
 
   /**
diff --git a/core/src/test/scala/other/kafka/StressTestLog.scala b/core/src/test/scala/other/kafka/StressTestLog.scala
index 8162d4d193763..cc94a9845a805 100755
--- a/core/src/test/scala/other/kafka/StressTestLog.scala
+++ b/core/src/test/scala/other/kafka/StressTestLog.scala
@@ -51,7 +51,8 @@ object StressTestLog {
       maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
       brokerTopicStats = new BrokerTopicStats,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10),
+      topicId = None)
     val writer = new WriterThread(log)
     writer.start()
     val reader = new ReaderThread(log)
diff --git a/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala b/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala
index 24a49c32d50cf..21e5290d5a40b 100755
--- a/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala
+++ b/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala
@@ -211,7 +211,7 @@ object TestLinearWriteSpeed {
   class LogWritable(val dir: File, config: LogConfig, scheduler: Scheduler, val messages: MemoryRecords) extends Writable {
     Utils.delete(dir)
     val log = Log(dir, config, 0L, 0L, scheduler, new BrokerTopicStats, Time.SYSTEM, 60 * 60 * 1000,
-      LogManager.ProducerIdExpirationCheckIntervalMs, new LogDirFailureChannel(10))
+      LogManager.ProducerIdExpirationCheckIntervalMs, new LogDirFailureChannel(10), topicId = None)
     def write(): Int = {
       log.appendAsLeader(messages, leaderEpoch = 0)
       messages.sizeInBytes
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
index a65b6fa4c7c09..0cda5b9157623 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
@@ -353,7 +353,8 @@ class PartitionLockTest extends Logging {
     log.producerIdExpirationCheckIntervalMs,
     log.topicPartition,
     log.producerStateManager,
-    new LogDirFailureChannel(1)) {
+    new LogDirFailureChannel(1),
+    topicId = None) {
 
     override def appendAsLeader(records: MemoryRecords, leaderEpoch: Int, origin: AppendOrigin, interBrokerProtocolVersion: ApiVersion): LogAppendInfo = {
       val appendInfo = super.appendAsLeader(records, leaderEpoch, origin, interBrokerProtocolVersion)
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
index 6c0a7f40c2f7f..37345a32f04da 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
@@ -1940,7 +1940,8 @@ class PartitionTest extends AbstractPartitionTest {
     log.producerIdExpirationCheckIntervalMs,
     log.topicPartition,
     log.producerStateManager,
-    new LogDirFailureChannel(1)) {
+    new LogDirFailureChannel(1),
+    topicId = None) {
 
     override def appendAsFollower(records: MemoryRecords): LogAppendInfo = {
       appendSemaphore.acquire()
diff --git a/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala b/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala
index 598c9e2339fae..e29b53f50dddb 100644
--- a/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala
@@ -50,7 +50,8 @@ class ReplicaTest {
       time = time,
       maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10),
+      topicId = None)
   }
 
   @AfterEach
diff --git a/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala b/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala
index 18a5f813a2958..76aa86c8be98f 100644
--- a/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala
+++ b/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala
@@ -110,7 +110,8 @@ abstract class AbstractLogCleanerIntegrationTest {
         brokerTopicStats = new BrokerTopicStats,
         maxProducerIdExpirationMs = 60 * 60 * 1000,
         producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-        logDirFailureChannel = new LogDirFailureChannel(10))
+        logDirFailureChannel = new LogDirFailureChannel(10),
+        topicId = None)
       logMap.put(partition, log)
       this.logs += log
     }
diff --git a/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala b/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
index 7e5197312fbee..6536a1311b730 100755
--- a/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
+++ b/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
@@ -55,7 +55,7 @@ class BrokerCompressionTest {
     val log = Log(logDir, LogConfig(logProps), logStartOffset = 0L, recoveryPoint = 0L, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
 
     /* append two messages */
     log.appendAsLeader(MemoryRecords.withRecords(CompressionType.forId(messageCompressionCode.codec), 0,
diff --git a/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala b/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala
index 267e8d8876a75..017f8bd78e822 100644
--- a/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala
@@ -98,7 +98,7 @@ class LogCleanerManagerTest extends Logging {
     // the exception should be catched and the partition that caused it marked as uncleanable
     class LogMock(dir: File, config: LogConfig) extends Log(dir, config, 0L, 0L,
       time.scheduler, new BrokerTopicStats, time, 60 * 60 * 1000, LogManager.ProducerIdExpirationCheckIntervalMs,
-      topicPartition, new ProducerStateManager(tp, tpDir, 60 * 60 * 1000), new LogDirFailureChannel(10)) {
+      topicPartition, new ProducerStateManager(tp, tpDir, 60 * 60 * 1000), new LogDirFailureChannel(10), topicId = None) {
 
       // Throw an error in getFirstBatchTimestampForSegments since it is called in grabFilthiestLog()
       override def getFirstBatchTimestampForSegments(segments: Iterable[LogSegment]): Iterable[Long] =
@@ -755,7 +755,8 @@ class LogCleanerManagerTest extends Logging {
       brokerTopicStats = new BrokerTopicStats,
       maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10),
+      topicId = None)
   }
 
   private def createLowRetentionLogConfig(segmentSize: Int, cleanupPolicy: String): LogConfig = {
@@ -799,7 +800,7 @@ class LogCleanerManagerTest extends Logging {
     Log(dir = dir, config = config, logStartOffset = 0L, recoveryPoint = 0L, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
 
   private def records(key: Int, value: Int, timestamp: Long) =
     MemoryRecords.withRecords(CompressionType.NONE, new SimpleRecord(timestamp, key.toString.getBytes, value.toString.getBytes))
diff --git a/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala b/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala
index dc51ce6d4e506..6e4d93b2acf9d 100755
--- a/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala
@@ -112,7 +112,8 @@ class LogCleanerTest {
                       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
                       topicPartition = topicPartition,
                       producerStateManager = producerStateManager,
-                      logDirFailureChannel = new LogDirFailureChannel(10)) {
+                      logDirFailureChannel = new LogDirFailureChannel(10),
+                      topicId = None) {
       override def replaceSegments(newSegments: Seq[LogSegment], oldSegments: Seq[LogSegment], isRecoveredSwapFile: Boolean = false): Unit = {
         deleteStartLatch.countDown()
         if (!deleteCompleteLatch.await(5000, TimeUnit.MILLISECONDS)) {
@@ -1674,7 +1675,7 @@ class LogCleanerTest {
     Log(dir = dir, config = config, logStartOffset = 0L, recoveryPoint = recoveryPoint, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
 
   private def makeCleaner(capacity: Int, checkDone: TopicPartition => Unit = _ => (), maxMessageSize: Int = 64*1024) =
     new Cleaner(id = 0,
diff --git a/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala b/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala
index cde75ea12b7dc..0944825cf8d2a 100644
--- a/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala
@@ -150,7 +150,8 @@ class LogConcurrencyTest {
       time = Time.SYSTEM,
       maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10),
+      topicId = None)
   }
 
   private def validateConsumedData(log: Log, consumedBatches: Iterable[FetchedBatch]): Unit = {
diff --git a/core/src/test/scala/unit/kafka/log/LogTest.scala b/core/src/test/scala/unit/kafka/log/LogTest.scala
index f44b9064dba9e..433e3097d9a1a 100755
--- a/core/src/test/scala/unit/kafka/log/LogTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogTest.scala
@@ -110,7 +110,7 @@ class LogTest {
 
           val producerStateManager = new ProducerStateManager(topicPartition, logDir, maxPidExpirationMs)
           val log = new Log(logDir, config, logStartOffset, logRecoveryPoint, time.scheduler, brokerTopicStats, time, maxPidExpirationMs,
-            LogManager.ProducerIdExpirationCheckIntervalMs, topicPartition, producerStateManager, logDirFailureChannel, hadCleanShutdown) {
+            LogManager.ProducerIdExpirationCheckIntervalMs, topicPartition, producerStateManager, logDirFailureChannel, hadCleanShutdown, None) {
             override def recoverLog(): Long = {
               if (simulateError)
                 throw new RuntimeException
@@ -1025,7 +1025,7 @@ class LogTest {
       // Intercept all segment read calls
       new Log(logDir, logConfig, logStartOffset = 0, recoveryPoint = recoveryPoint, mockTime.scheduler,
         brokerTopicStats, mockTime, maxProducerIdExpirationMs, LogManager.ProducerIdExpirationCheckIntervalMs,
-        topicPartition, producerStateManager, new LogDirFailureChannel(10), hadCleanShutdown = false) {
+        topicPartition, producerStateManager, new LogDirFailureChannel(10), hadCleanShutdown = false, topicId = None) {
 
         override def addSegment(segment: LogSegment): LogSegment = {
           val wrapper = new LogSegment(segment.log, segment.lazyOffsetIndex, segment.lazyTimeIndex, segment.txnIndex, segment.baseOffset,
@@ -1128,7 +1128,8 @@ class LogTest {
       topicPartition = Log.parseTopicPartitionName(logDir),
       producerStateManager = stateManager,
       logDirFailureChannel = new LogDirFailureChannel(1),
-      hadCleanShutdown = false)
+      hadCleanShutdown = false,
+      topicId = None)
 
     EasyMock.verify(stateManager)
 
@@ -1206,7 +1207,8 @@ class LogTest {
       producerIdExpirationCheckIntervalMs = 30000,
       topicPartition = Log.parseTopicPartitionName(logDir),
       producerStateManager = stateManager,
-      logDirFailureChannel = null)
+      logDirFailureChannel = null,
+      topicId = None)
 
     EasyMock.verify(stateManager)
   }
@@ -1244,7 +1246,8 @@ class LogTest {
       producerIdExpirationCheckIntervalMs = 30000,
       topicPartition = Log.parseTopicPartitionName(logDir),
       producerStateManager = stateManager,
-      logDirFailureChannel = null)
+      logDirFailureChannel = null,
+      topicId = None)
 
     EasyMock.verify(stateManager)
   }
@@ -1284,7 +1287,8 @@ class LogTest {
       producerIdExpirationCheckIntervalMs = 30000,
       topicPartition = Log.parseTopicPartitionName(logDir),
       producerStateManager = stateManager,
-      logDirFailureChannel = null)
+      logDirFailureChannel = null,
+      topicId = None)
 
     EasyMock.verify(stateManager)
   }
diff --git a/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala b/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala
index 1a5e51e5fe3da..307cf18c6cf48 100644
--- a/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala
+++ b/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala
@@ -61,7 +61,7 @@ class DumpLogSegmentsTest {
     log = Log(logDir, LogConfig(props), logStartOffset = 0L, recoveryPoint = 0L, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10))
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
   }
 
   def addSimpleRecords(): Unit = {
diff --git a/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala b/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala
index eb7e81297c2ba..595d6da3d08b1 100644
--- a/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala
+++ b/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala
@@ -124,7 +124,7 @@ class SchedulerTest {
     val producerStateManager = new ProducerStateManager(topicPartition, logDir, maxProducerIdExpirationMs)
     val log = new Log(logDir, logConfig, logStartOffset = 0, recoveryPoint = recoveryPoint, scheduler,
       brokerTopicStats, mockTime, maxProducerIdExpirationMs, LogManager.ProducerIdExpirationCheckIntervalMs,
-      topicPartition, producerStateManager, new LogDirFailureChannel(10))
+      topicPartition, producerStateManager, new LogDirFailureChannel(10), topicId = None)
     assertTrue(scheduler.taskRunning(log.producerExpireCheck))
     log.close()
     assertFalse(scheduler.taskRunning(log.producerExpireCheck))

From fac1f7a3e926d4fb95f43e175e825843fc9d757a Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Wed, 31 Mar 2021 15:30:48 -0700
Subject: [PATCH 8/9] Small tweaks to style, made optional
 keepPartitionMetadata file non-optional, added check for inconsistent topic
 IDs in getOrCreateLog

---
 .../main/scala/kafka/cluster/Partition.scala  |  4 ----
 core/src/main/scala/kafka/log/Log.scala       | 10 ++++-----
 .../src/main/scala/kafka/log/LogManager.scala | 18 ++++++++++++++--
 .../kafka/server/RaftReplicaManager.scala     |  4 ++--
 .../scala/other/kafka/StressTestLog.scala     |  3 ++-
 .../other/kafka/TestLinearWriteSpeed.scala    |  2 +-
 .../kafka/cluster/PartitionLockTest.scala     |  3 ++-
 .../unit/kafka/cluster/PartitionTest.scala    | 21 ++++++++++++-------
 .../unit/kafka/cluster/ReplicaTest.scala      |  3 ++-
 .../AbstractLogCleanerIntegrationTest.scala   |  3 ++-
 .../kafka/log/BrokerCompressionTest.scala     |  2 +-
 .../kafka/log/LogCleanerManagerTest.scala     |  8 ++++---
 .../scala/unit/kafka/log/LogCleanerTest.scala |  5 +++--
 .../unit/kafka/log/LogConcurrencyTest.scala   |  3 ++-
 .../test/scala/unit/kafka/log/LogTest.scala   | 19 ++++++++++-------
 .../kafka/server/ReplicaManagerTest.scala     |  7 ++++---
 .../kafka/tools/DumpLogSegmentsTest.scala     |  2 +-
 .../unit/kafka/utils/SchedulerTest.scala      |  2 +-
 .../kafka/jmh/server/CheckpointBench.java     |  3 ++-
 19 files changed, 77 insertions(+), 45 deletions(-)

diff --git a/core/src/main/scala/kafka/cluster/Partition.scala b/core/src/main/scala/kafka/cluster/Partition.scala
index e94567a025674..0118c0bef38ea 100755
--- a/core/src/main/scala/kafka/cluster/Partition.scala
+++ b/core/src/main/scala/kafka/cluster/Partition.scala
@@ -344,10 +344,6 @@ class Partition(val topicPartition: TopicPartition,
       val log = logManager.getOrCreateLog(topicPartition, isNew, isFutureReplica, topicId)
       maybeLog = Some(log)
       updateHighWatermark(log)
-      // When running a ZK controller, we may get a log that does not have a topic ID. Assign it here.
-      if (log.topicId == None) {
-        topicId.foreach(topicId => log.writeTopicIdToExistingLog(topicId))
-      }
       log
     } finally {
       logManager.finishedInitializingLog(topicPartition, maybeLog)
diff --git a/core/src/main/scala/kafka/log/Log.scala b/core/src/main/scala/kafka/log/Log.scala
index 13a4e5e3fdd08..c2058312791cc 100644
--- a/core/src/main/scala/kafka/log/Log.scala
+++ b/core/src/main/scala/kafka/log/Log.scala
@@ -267,7 +267,7 @@ class Log(@volatile private var _dir: File,
           logDirFailureChannel: LogDirFailureChannel,
           private val hadCleanShutdown: Boolean = true,
           @volatile var topicId: Option[Uuid],
-          val keepPartitionMetadataFile: Boolean = true) extends Logging with KafkaMetricsGroup {
+          val keepPartitionMetadataFile: Boolean) extends Logging with KafkaMetricsGroup {
 
   import kafka.log.Log._
 
@@ -365,8 +365,8 @@ class Log(@volatile private var _dir: File,
               s"but log already contained topic ID $fileTopicId")
           topicId = Some(fileTopicId)
         }
-    } else if (topicId.isDefined && keepPartitionMetadataFile) {
-      partitionMetadataFile.write(topicId.get)
+    } else if (keepPartitionMetadataFile) {
+      topicId.foreach(partitionMetadataFile.write)
     }
   }
 
@@ -590,7 +590,7 @@ class Log(@volatile private var _dir: File,
   }
 
   /** Only used for ZK clusters when we update and start using topic IDs on existing topics */
-  def writeTopicIdToExistingLog(topicId: Uuid): Unit = {
+  def assignTopicId(topicId: Uuid): Unit = {
     partitionMetadataFile.write(topicId)
     this.topicId = Some(topicId)
   }
@@ -2620,7 +2620,7 @@ object Log {
             logDirFailureChannel: LogDirFailureChannel,
             lastShutdownClean: Boolean = true,
             topicId: Option[Uuid],
-            keepPartitionMetadataFile: Boolean = true): Log = {
+            keepPartitionMetadataFile: Boolean): Log = {
     val topicPartition = Log.parseTopicPartitionName(dir)
     val producerStateManager = new ProducerStateManager(topicPartition, dir, maxProducerIdExpirationMs)
     new Log(dir, config, logStartOffset, recoveryPoint, scheduler, brokerTopicStats, time, maxProducerIdExpirationMs,
diff --git a/core/src/main/scala/kafka/log/LogManager.scala b/core/src/main/scala/kafka/log/LogManager.scala
index 2d995e73f0bfe..aadd58028a814 100755
--- a/core/src/main/scala/kafka/log/LogManager.scala
+++ b/core/src/main/scala/kafka/log/LogManager.scala
@@ -776,12 +776,12 @@ class LogManager(logDirs: Seq[File],
    * @param topicPartition The partition whose log needs to be returned or created
    * @param isNew Whether the replica should have existed on the broker or not
    * @param isFuture True if the future log of the specified partition should be returned or created
-   * @param topicId The topic ID of the topic used in the case of log creation.
+   * @param topicId The topic ID of the partition's topic
    * @throws KafkaStorageException if isNew=false, log is not found in the cache and there is offline log directory on the broker
    */
   def getOrCreateLog(topicPartition: TopicPartition, isNew: Boolean = false, isFuture: Boolean = false, topicId: Option[Uuid]): Log = {
     logCreationOrDeletionLock synchronized {
-      getLog(topicPartition, isFuture).getOrElse {
+      val log = getLog(topicPartition, isFuture).getOrElse {
         // create the log if it has not already been created in another thread
         if (!isNew && offlineLogDirs.nonEmpty)
           throw new KafkaStorageException(s"Can not create log for $topicPartition because log directories ${offlineLogDirs.mkString(",")} are offline")
@@ -842,6 +842,20 @@ class LogManager(logDirs: Seq[File],
 
         log
       }
+      // When running a ZK controller, we may get a log that does not have a topic ID. Assign it here.
+      if (log.topicId.isEmpty) {
+        topicId.foreach(log.assignTopicId)
+      }
+
+      // Ensure topic IDs are consistent
+      topicId.foreach { topicId =>
+        log.topicId.foreach { logTopicId =>
+          if (topicId != logTopicId)
+            throw new IllegalStateException(s"Tried to assign topic ID $topicId to log for topic partition $topicPartition," +
+              s"but log already contained topic ID $logTopicId")
+        }
+      }
+      log
     }
   }
 
diff --git a/core/src/main/scala/kafka/server/RaftReplicaManager.scala b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
index 000b000db8d2e..30960deed0369 100644
--- a/core/src/main/scala/kafka/server/RaftReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/RaftReplicaManager.scala
@@ -389,7 +389,7 @@ class RaftReplicaManager(config: KafkaConfig,
   private def checkTopicId(receivedTopicIdOpt: Option[Uuid], logTopicIdOpt: Option[Uuid], topicPartition: TopicPartition): Unit = {
     receivedTopicIdOpt match {
       case Some(receivedTopicId) =>
-        logTopicIdOpt.foreach(logTopicId => {
+        logTopicIdOpt.foreach { logTopicId =>
           if (receivedTopicId != logTopicId) {
             // not sure if we need both the logger and the error thrown
             stateChangeLogger.error(s"Topic ID in memory: $logTopicId does not" +
@@ -397,7 +397,7 @@ class RaftReplicaManager(config: KafkaConfig,
               s"$receivedTopicId.")
             throw new InconsistentTopicIdException(s"Topic partition $topicPartition had an inconsistent topic ID.")
           }
-        })
+        }
       case None => throw new IllegalStateException(s"Topic partition $topicPartition is missing a topic ID")
     }
   }
diff --git a/core/src/test/scala/other/kafka/StressTestLog.scala b/core/src/test/scala/other/kafka/StressTestLog.scala
index cc94a9845a805..e3c1e65ea4a9f 100755
--- a/core/src/test/scala/other/kafka/StressTestLog.scala
+++ b/core/src/test/scala/other/kafka/StressTestLog.scala
@@ -52,7 +52,8 @@ object StressTestLog {
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
       brokerTopicStats = new BrokerTopicStats,
       logDirFailureChannel = new LogDirFailureChannel(10),
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
     val writer = new WriterThread(log)
     writer.start()
     val reader = new ReaderThread(log)
diff --git a/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala b/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala
index 21e5290d5a40b..cb2180ae476d8 100755
--- a/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala
+++ b/core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala
@@ -211,7 +211,7 @@ object TestLinearWriteSpeed {
   class LogWritable(val dir: File, config: LogConfig, scheduler: Scheduler, val messages: MemoryRecords) extends Writable {
     Utils.delete(dir)
     val log = Log(dir, config, 0L, 0L, scheduler, new BrokerTopicStats, Time.SYSTEM, 60 * 60 * 1000,
-      LogManager.ProducerIdExpirationCheckIntervalMs, new LogDirFailureChannel(10), topicId = None)
+      LogManager.ProducerIdExpirationCheckIntervalMs, new LogDirFailureChannel(10), topicId = None, keepPartitionMetadataFile = true)
     def write(): Int = {
       log.appendAsLeader(messages, leaderEpoch = 0)
       messages.sizeInBytes
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
index 0cda5b9157623..a8198c9c02e09 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionLockTest.scala
@@ -354,7 +354,8 @@ class PartitionLockTest extends Logging {
     log.topicPartition,
     log.producerStateManager,
     new LogDirFailureChannel(1),
-    topicId = None) {
+    topicId = None,
+    keepPartitionMetadataFile = true) {
 
     override def appendAsLeader(records: MemoryRecords, leaderEpoch: Int, origin: AppendOrigin, interBrokerProtocolVersion: ApiVersion): LogAppendInfo = {
       val appendInfo = super.appendAsLeader(records, leaderEpoch, origin, interBrokerProtocolVersion)
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
index 37345a32f04da..022c312d8345f 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
@@ -1672,9 +1672,12 @@ class PartitionTest extends AbstractPartitionTest {
     assertEquals(topicId, partition2.topicId.get)
     assertFalse(partition2.log.isDefined)
 
-    // Calling makeLeader with a new topic ID should not overwrite the old topic ID. We should get the same log.
-    // This scenario should not occur, since the topic ID check will fail, but it is good to check we grab the old log.
-    partition2.makeLeader(leaderState, offsetCheckpoints, Some(Uuid.randomUuid()))
+    // Calling makeLeader with a new topic ID should not overwrite the old topic ID. We should get an IllegalStateException.
+    // This scenario should not occur, since the topic ID check will fail.
+    assertThrows(classOf[IllegalStateException], () => partition2.makeLeader(leaderState, offsetCheckpoints, Some(Uuid.randomUuid())))
+
+    // Calling makeLeader with no topic ID should not overwrite the old topic ID. We should get the original log.
+    partition2.makeLeader(leaderState, offsetCheckpoints, None)
     checkTopicId(topicId, partition2)
   }
 
@@ -1713,9 +1716,12 @@ class PartitionTest extends AbstractPartitionTest {
     assertEquals(topicId, partition2.topicId.get)
     assertFalse(partition2.log.isDefined)
 
-    // Calling makeFollower with a new topic ID should not overwrite the old topic ID. We should get the same log.
-    // This scenario should not occur, since the topic ID check will fail, but it is good to check we grab the old log.
-    partition2.makeFollower(leaderState, offsetCheckpoints, Some(Uuid.randomUuid()))
+    // Calling makeFollower with a new topic ID should not overwrite the old topic ID. We should get an IllegalStateException.
+    // This scenario should not occur, since the topic ID check will fail.
+    assertThrows(classOf[IllegalStateException], () => partition2.makeFollower(leaderState, offsetCheckpoints, Some(Uuid.randomUuid())))
+
+    // Calling makeFollower with no topic ID should not overwrite the old topic ID. We should get the original log.
+    partition2.makeFollower(leaderState, offsetCheckpoints, None)
     checkTopicId(topicId, partition2)
   }
 
@@ -1941,7 +1947,8 @@ class PartitionTest extends AbstractPartitionTest {
     log.topicPartition,
     log.producerStateManager,
     new LogDirFailureChannel(1),
-    topicId = None) {
+    topicId = None,
+    keepPartitionMetadataFile = true) {
 
     override def appendAsFollower(records: MemoryRecords): LogAppendInfo = {
       appendSemaphore.acquire()
diff --git a/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala b/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala
index e29b53f50dddb..9ec90719419c8 100644
--- a/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/ReplicaTest.scala
@@ -51,7 +51,8 @@ class ReplicaTest {
       maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
       logDirFailureChannel = new LogDirFailureChannel(10),
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
   }
 
   @AfterEach
diff --git a/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala b/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala
index 76aa86c8be98f..99c85b6e1dd22 100644
--- a/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala
+++ b/core/src/test/scala/unit/kafka/log/AbstractLogCleanerIntegrationTest.scala
@@ -111,7 +111,8 @@ abstract class AbstractLogCleanerIntegrationTest {
         maxProducerIdExpirationMs = 60 * 60 * 1000,
         producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
         logDirFailureChannel = new LogDirFailureChannel(10),
-        topicId = None)
+        topicId = None,
+        keepPartitionMetadataFile = true)
       logMap.put(partition, log)
       this.logs += log
     }
diff --git a/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala b/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
index 6536a1311b730..af6265ccec5f4 100755
--- a/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
+++ b/core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala
@@ -55,7 +55,7 @@ class BrokerCompressionTest {
     val log = Log(logDir, LogConfig(logProps), logStartOffset = 0L, recoveryPoint = 0L, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None, keepPartitionMetadataFile = true)
 
     /* append two messages */
     log.appendAsLeader(MemoryRecords.withRecords(CompressionType.forId(messageCompressionCode.codec), 0,
diff --git a/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala b/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala
index 017f8bd78e822..16aac94dc5df6 100644
--- a/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala
@@ -98,7 +98,8 @@ class LogCleanerManagerTest extends Logging {
     // the exception should be catched and the partition that caused it marked as uncleanable
     class LogMock(dir: File, config: LogConfig) extends Log(dir, config, 0L, 0L,
       time.scheduler, new BrokerTopicStats, time, 60 * 60 * 1000, LogManager.ProducerIdExpirationCheckIntervalMs,
-      topicPartition, new ProducerStateManager(tp, tpDir, 60 * 60 * 1000), new LogDirFailureChannel(10), topicId = None) {
+      topicPartition, new ProducerStateManager(tp, tpDir, 60 * 60 * 1000),
+      new LogDirFailureChannel(10), topicId = None, keepPartitionMetadataFile = true) {
 
       // Throw an error in getFirstBatchTimestampForSegments since it is called in grabFilthiestLog()
       override def getFirstBatchTimestampForSegments(segments: Iterable[LogSegment]): Iterable[Long] =
@@ -756,7 +757,8 @@ class LogCleanerManagerTest extends Logging {
       maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
       logDirFailureChannel = new LogDirFailureChannel(10),
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
   }
 
   private def createLowRetentionLogConfig(segmentSize: Int, cleanupPolicy: String): LogConfig = {
@@ -800,7 +802,7 @@ class LogCleanerManagerTest extends Logging {
     Log(dir = dir, config = config, logStartOffset = 0L, recoveryPoint = 0L, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None, keepPartitionMetadataFile = true)
 
   private def records(key: Int, value: Int, timestamp: Long) =
     MemoryRecords.withRecords(CompressionType.NONE, new SimpleRecord(timestamp, key.toString.getBytes, value.toString.getBytes))
diff --git a/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala b/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala
index 6e4d93b2acf9d..45727a0f639c5 100755
--- a/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala
@@ -113,7 +113,8 @@ class LogCleanerTest {
                       topicPartition = topicPartition,
                       producerStateManager = producerStateManager,
                       logDirFailureChannel = new LogDirFailureChannel(10),
-                      topicId = None) {
+                      topicId = None,
+                      keepPartitionMetadataFile = true) {
       override def replaceSegments(newSegments: Seq[LogSegment], oldSegments: Seq[LogSegment], isRecoveredSwapFile: Boolean = false): Unit = {
         deleteStartLatch.countDown()
         if (!deleteCompleteLatch.await(5000, TimeUnit.MILLISECONDS)) {
@@ -1675,7 +1676,7 @@ class LogCleanerTest {
     Log(dir = dir, config = config, logStartOffset = 0L, recoveryPoint = recoveryPoint, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None, keepPartitionMetadataFile = true)
 
   private def makeCleaner(capacity: Int, checkDone: TopicPartition => Unit = _ => (), maxMessageSize: Int = 64*1024) =
     new Cleaner(id = 0,
diff --git a/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala b/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala
index 0944825cf8d2a..3efe2bef03566 100644
--- a/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogConcurrencyTest.scala
@@ -151,7 +151,8 @@ class LogConcurrencyTest {
       maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
       logDirFailureChannel = new LogDirFailureChannel(10),
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
   }
 
   private def validateConsumedData(log: Log, consumedBatches: Iterable[FetchedBatch]): Unit = {
diff --git a/core/src/test/scala/unit/kafka/log/LogTest.scala b/core/src/test/scala/unit/kafka/log/LogTest.scala
index 433e3097d9a1a..08c62099e4f1f 100755
--- a/core/src/test/scala/unit/kafka/log/LogTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogTest.scala
@@ -110,7 +110,7 @@ class LogTest {
 
           val producerStateManager = new ProducerStateManager(topicPartition, logDir, maxPidExpirationMs)
           val log = new Log(logDir, config, logStartOffset, logRecoveryPoint, time.scheduler, brokerTopicStats, time, maxPidExpirationMs,
-            LogManager.ProducerIdExpirationCheckIntervalMs, topicPartition, producerStateManager, logDirFailureChannel, hadCleanShutdown, None) {
+            LogManager.ProducerIdExpirationCheckIntervalMs, topicPartition, producerStateManager, logDirFailureChannel, hadCleanShutdown, None, true) {
             override def recoverLog(): Long = {
               if (simulateError)
                 throw new RuntimeException
@@ -1025,7 +1025,7 @@ class LogTest {
       // Intercept all segment read calls
       new Log(logDir, logConfig, logStartOffset = 0, recoveryPoint = recoveryPoint, mockTime.scheduler,
         brokerTopicStats, mockTime, maxProducerIdExpirationMs, LogManager.ProducerIdExpirationCheckIntervalMs,
-        topicPartition, producerStateManager, new LogDirFailureChannel(10), hadCleanShutdown = false, topicId = None) {
+        topicPartition, producerStateManager, new LogDirFailureChannel(10), hadCleanShutdown = false, topicId = None, keepPartitionMetadataFile = true) {
 
         override def addSegment(segment: LogSegment): LogSegment = {
           val wrapper = new LogSegment(segment.log, segment.lazyOffsetIndex, segment.lazyTimeIndex, segment.txnIndex, segment.baseOffset,
@@ -1129,7 +1129,8 @@ class LogTest {
       producerStateManager = stateManager,
       logDirFailureChannel = new LogDirFailureChannel(1),
       hadCleanShutdown = false,
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
 
     EasyMock.verify(stateManager)
 
@@ -1208,7 +1209,8 @@ class LogTest {
       topicPartition = Log.parseTopicPartitionName(logDir),
       producerStateManager = stateManager,
       logDirFailureChannel = null,
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
 
     EasyMock.verify(stateManager)
   }
@@ -1247,7 +1249,8 @@ class LogTest {
       topicPartition = Log.parseTopicPartitionName(logDir),
       producerStateManager = stateManager,
       logDirFailureChannel = null,
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
 
     EasyMock.verify(stateManager)
   }
@@ -1288,7 +1291,8 @@ class LogTest {
       topicPartition = Log.parseTopicPartitionName(logDir),
       producerStateManager = stateManager,
       logDirFailureChannel = null,
-      topicId = None)
+      topicId = None,
+      keepPartitionMetadataFile = true)
 
     EasyMock.verify(stateManager)
   }
@@ -4959,7 +4963,8 @@ object LogTest {
       producerIdExpirationCheckIntervalMs = producerIdExpirationCheckIntervalMs,
       logDirFailureChannel = new LogDirFailureChannel(10),
       lastShutdownClean = lastShutdownClean,
-      topicId = topicId)
+      topicId = topicId,
+      keepPartitionMetadataFile = true)
   }
 
   /**
diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
index 56f37dbcc8a5f..ced71864bc892 100644
--- a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
+++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
@@ -1495,7 +1495,8 @@ class ReplicaManagerTest {
       producerStateManager = new ProducerStateManager(new TopicPartition(topic, topicPartition),
         new File(new File(config.logDirs.head), s"$topic-$topicPartition"), 30000),
       logDirFailureChannel = mockLogDirFailureChannel,
-      topicId = topicId) {
+      topicId = topicId,
+      keepPartitionMetadataFile = true) {
 
       override def endOffsetForEpoch(leaderEpoch: Int): Option[OffsetAndEpoch] = {
         assertEquals(leaderEpoch, leaderEpochFromLeader)
@@ -2315,7 +2316,7 @@ class ReplicaManagerTest {
   }
 
   @Test
-  def testInvalidIdReturnsError(): Unit = {
+  def testInconsistentIdReturnsError(): Unit = {
     val replicaManager = setupReplicaManagerWithMockedPurgatories(new MockTimer(time))
     try {
       val brokerList = Seq[Integer](0, 1).asJava
@@ -2347,7 +2348,7 @@ class ReplicaManagerTest {
       val response2 = replicaManager.becomeLeaderOrFollower(0, leaderAndIsrRequest(1, topicIds), (_, _) => ())
       assertEquals(Errors.NONE, response2.partitionErrors(topicNames).get(topicPartition))
 
-      // Send request with invalid ID.
+      // Send request with inconsistent ID.
       val response3 = replicaManager.becomeLeaderOrFollower(0, leaderAndIsrRequest(1, invalidTopicIds), (_, _) => ())
       assertEquals(Errors.INCONSISTENT_TOPIC_ID, response3.partitionErrors(invalidTopicNames).get(topicPartition))
 
diff --git a/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala b/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala
index 307cf18c6cf48..256262a99b4e0 100644
--- a/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala
+++ b/core/src/test/scala/unit/kafka/tools/DumpLogSegmentsTest.scala
@@ -61,7 +61,7 @@ class DumpLogSegmentsTest {
     log = Log(logDir, LogConfig(props), logStartOffset = 0L, recoveryPoint = 0L, scheduler = time.scheduler,
       time = time, brokerTopicStats = new BrokerTopicStats, maxProducerIdExpirationMs = 60 * 60 * 1000,
       producerIdExpirationCheckIntervalMs = LogManager.ProducerIdExpirationCheckIntervalMs,
-      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None)
+      logDirFailureChannel = new LogDirFailureChannel(10), topicId = None, keepPartitionMetadataFile = true)
   }
 
   def addSimpleRecords(): Unit = {
diff --git a/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala b/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala
index 595d6da3d08b1..913d3e2449a4b 100644
--- a/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala
+++ b/core/src/test/scala/unit/kafka/utils/SchedulerTest.scala
@@ -124,7 +124,7 @@ class SchedulerTest {
     val producerStateManager = new ProducerStateManager(topicPartition, logDir, maxProducerIdExpirationMs)
     val log = new Log(logDir, logConfig, logStartOffset = 0, recoveryPoint = recoveryPoint, scheduler,
       brokerTopicStats, mockTime, maxProducerIdExpirationMs, LogManager.ProducerIdExpirationCheckIntervalMs,
-      topicPartition, producerStateManager, new LogDirFailureChannel(10), topicId = None)
+      topicPartition, producerStateManager, new LogDirFailureChannel(10), topicId = None, keepPartitionMetadataFile = true)
     assertTrue(scheduler.taskRunning(log.producerExpireCheck))
     log.close()
     assertFalse(scheduler.taskRunning(log.producerExpireCheck))
diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java
index 649835950c7ec..6c16efb816d5f 100644
--- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java
+++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/server/CheckpointBench.java
@@ -33,6 +33,7 @@
 import kafka.utils.MockTime;
 import kafka.utils.Scheduler;
 import kafka.utils.TestUtils;
+import org.apache.kafka.common.Uuid;
 import org.apache.kafka.common.TopicPartition;
 import org.apache.kafka.common.metrics.Metrics;
 import org.apache.kafka.common.utils.Utils;
@@ -145,7 +146,7 @@ public void setup() {
         OffsetCheckpoints checkpoints = (logDir, topicPartition) -> Option.apply(0L);
         for (TopicPartition topicPartition : topicPartitions) {
             final Partition partition = this.replicaManager.createPartition(topicPartition);
-            partition.createLogIfNotExists(true, false, checkpoints, Option.empty());
+            partition.createLogIfNotExists(true, false, checkpoints, Option.apply(Uuid.randomUuid()));
         }
 
         replicaManager.checkpointHighWatermarks();

From c221911132c55da8f57814b71432c810bf7effa9 Mon Sep 17 00:00:00 2001
From: Justine <jolshan@confluent.io>
Date: Thu, 1 Apr 2021 12:08:51 -0700
Subject: [PATCH 9/9] Address comments. Changed error message and cleaned up
 some code.

---
 core/src/main/scala/kafka/log/Log.scala       |  2 +-
 .../src/main/scala/kafka/log/LogManager.scala |  5 ++--
 .../scala/kafka/server/ReplicaManager.scala   | 24 +++++++++----------
 .../unit/kafka/cluster/PartitionTest.scala    | 10 ++++----
 .../test/scala/unit/kafka/log/LogTest.scala   |  2 +-
 5 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/core/src/main/scala/kafka/log/Log.scala b/core/src/main/scala/kafka/log/Log.scala
index c2058312791cc..1c7ec6e338b57 100644
--- a/core/src/main/scala/kafka/log/Log.scala
+++ b/core/src/main/scala/kafka/log/Log.scala
@@ -361,7 +361,7 @@ class Log(@volatile private var _dir: File,
         else {
           val fileTopicId = partitionMetadataFile.read().topicId
           if (topicId.isDefined && !topicId.contains(fileTopicId))
-            throw new IllegalStateException(s"Tried to assign topic ID $topicId to log for topic partition $topicPartition," +
+            throw new InconsistentTopicIdException(s"Tried to assign topic ID $topicId to log for topic partition $topicPartition," +
               s"but log already contained topic ID $fileTopicId")
           topicId = Some(fileTopicId)
         }
diff --git a/core/src/main/scala/kafka/log/LogManager.scala b/core/src/main/scala/kafka/log/LogManager.scala
index aadd58028a814..117d2138d9a20 100755
--- a/core/src/main/scala/kafka/log/LogManager.scala
+++ b/core/src/main/scala/kafka/log/LogManager.scala
@@ -29,7 +29,7 @@ import kafka.server._
 import kafka.utils._
 import org.apache.kafka.common.{KafkaException, TopicPartition, Uuid}
 import org.apache.kafka.common.utils.Time
-import org.apache.kafka.common.errors.{KafkaStorageException, LogDirNotFoundException}
+import org.apache.kafka.common.errors.{InconsistentTopicIdException, KafkaStorageException, LogDirNotFoundException}
 
 import scala.jdk.CollectionConverters._
 import scala.collection._
@@ -778,6 +778,7 @@ class LogManager(logDirs: Seq[File],
    * @param isFuture True if the future log of the specified partition should be returned or created
    * @param topicId The topic ID of the partition's topic
    * @throws KafkaStorageException if isNew=false, log is not found in the cache and there is offline log directory on the broker
+   * @throws InconsistentTopicIdException if the topic ID in the log does not match the topic ID provided
    */
   def getOrCreateLog(topicPartition: TopicPartition, isNew: Boolean = false, isFuture: Boolean = false, topicId: Option[Uuid]): Log = {
     logCreationOrDeletionLock synchronized {
@@ -851,7 +852,7 @@ class LogManager(logDirs: Seq[File],
       topicId.foreach { topicId =>
         log.topicId.foreach { logTopicId =>
           if (topicId != logTopicId)
-            throw new IllegalStateException(s"Tried to assign topic ID $topicId to log for topic partition $topicPartition," +
+            throw new InconsistentTopicIdException(s"Tried to assign topic ID $topicId to log for topic partition $topicPartition," +
               s"but log already contained topic ID $logTopicId")
         }
       }
diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala
index 6319b3eb2809b..a97f29b83d954 100644
--- a/core/src/main/scala/kafka/server/ReplicaManager.scala
+++ b/core/src/main/scala/kafka/server/ReplicaManager.scala
@@ -1326,7 +1326,7 @@ class ReplicaManager(val config: KafkaConfig,
             s"epoch ${leaderAndIsrRequest.controllerEpoch}")
         }
       val topicIds = leaderAndIsrRequest.topicIds()
-      def getTopicId(topicName: String): Option[Uuid] = {
+      def topicIdFromRequest(topicName: String): Option[Uuid] = {
         val topicId = topicIds.get(topicName)
         // if invalid topic ID return None
         if (topicId == null || topicId == Uuid.ZERO_UUID)
@@ -1375,12 +1375,12 @@ class ReplicaManager(val config: KafkaConfig,
             partitionOpt.foreach { partition =>
               val currentLeaderEpoch = partition.getLeaderEpoch
               val requestLeaderEpoch = partitionState.leaderEpoch
-              val requestTopicId = topicIds.get(topicPartition.topic)
+              val requestTopicId = topicIdFromRequest(topicPartition.topic)
 
               if (!hasConsistentTopicId(requestTopicId, partition.topicId)) {
                 stateChangeLogger.error(s"Topic ID in memory: ${partition.topicId.get} does not" +
                   s" match the topic ID for partition $topicPartition received: " +
-                  s"$requestTopicId.")
+                  s"${requestTopicId.get}.")
                 responseMap.put(topicPartition, Errors.INCONSISTENT_TOPIC_ID)
               } else if (requestLeaderEpoch > currentLeaderEpoch) {
                 // If the leader epoch is valid record the epoch of the controller that made the leadership decision.
@@ -1418,12 +1418,12 @@ class ReplicaManager(val config: KafkaConfig,
           val highWatermarkCheckpoints = new LazyOffsetCheckpoints(this.highWatermarkCheckpoints)
           val partitionsBecomeLeader = if (partitionsToBeLeader.nonEmpty)
             makeLeaders(controllerId, controllerEpoch, partitionsToBeLeader, correlationId, responseMap,
-              highWatermarkCheckpoints, getTopicId)
+              highWatermarkCheckpoints, topicIdFromRequest)
           else
             Set.empty[Partition]
           val partitionsBecomeFollower = if (partitionsToBeFollower.nonEmpty)
             makeFollowers(controllerId, controllerEpoch, partitionsToBeFollower, correlationId, responseMap,
-              highWatermarkCheckpoints, getTopicId)
+              highWatermarkCheckpoints, topicIdFromRequest)
           else
             Set.empty[Partition]
 
@@ -1446,7 +1446,7 @@ class ReplicaManager(val config: KafkaConfig,
           // have been completely populated before starting the checkpointing there by avoiding weird race conditions
           startHighWatermarkCheckPointThread()
 
-          maybeAddLogDirFetchers(partitionStates.keySet, highWatermarkCheckpoints, getTopicId)
+          maybeAddLogDirFetchers(partitionStates.keySet, highWatermarkCheckpoints, topicIdFromRequest)
 
           replicaFetcherManager.shutdownIdleFetcherThreads()
           replicaAlterLogDirsManager.shutdownIdleFetcherThreads()
@@ -1492,15 +1492,15 @@ class ReplicaManager(val config: KafkaConfig,
    * If the log does not exist or the topic ID is not yet set, logTopicIdOpt will be None.
    * In both cases, the ID is not inconsistent so return true.
    *
-   * @param requestTopicId the topic ID from the LeaderAndIsr request
+   * @param requestTopicIdOpt the topic ID from the LeaderAndIsr request if it exists
    * @param logTopicIdOpt the topic ID in the log if the log and the topic ID exist
    * @return true if the request topic id is consistent, false otherwise
    */
-  private def hasConsistentTopicId(requestTopicId: Uuid, logTopicIdOpt: Option[Uuid]): Boolean = {
-    if (requestTopicId == null || requestTopicId == Uuid.ZERO_UUID) {
-      true
-    } else
-      logTopicIdOpt.isEmpty || logTopicIdOpt.contains(requestTopicId)
+  private def hasConsistentTopicId(requestTopicIdOpt: Option[Uuid], logTopicIdOpt: Option[Uuid]): Boolean = {
+    requestTopicIdOpt match {
+      case None => true
+      case Some(requestTopicId) => logTopicIdOpt.isEmpty || logTopicIdOpt.contains(requestTopicId)
+    }
   }
 
   /**
diff --git a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
index 022c312d8345f..dd671e543e29c 100644
--- a/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
+++ b/core/src/test/scala/unit/kafka/cluster/PartitionTest.scala
@@ -25,7 +25,7 @@ import kafka.server._
 import kafka.server.checkpoints.OffsetCheckpoints
 import kafka.utils._
 import kafka.zk.KafkaZkClient
-import org.apache.kafka.common.errors.{ApiException, NotLeaderOrFollowerException, OffsetNotAvailableException, OffsetOutOfRangeException}
+import org.apache.kafka.common.errors.{ApiException, InconsistentTopicIdException, NotLeaderOrFollowerException, OffsetNotAvailableException, OffsetOutOfRangeException}
 import org.apache.kafka.common.message.FetchResponseData
 import org.apache.kafka.common.message.LeaderAndIsrRequestData.LeaderAndIsrPartitionState
 import org.apache.kafka.common.protocol.Errors
@@ -1672,9 +1672,9 @@ class PartitionTest extends AbstractPartitionTest {
     assertEquals(topicId, partition2.topicId.get)
     assertFalse(partition2.log.isDefined)
 
-    // Calling makeLeader with a new topic ID should not overwrite the old topic ID. We should get an IllegalStateException.
+    // Calling makeLeader with a new topic ID should not overwrite the old topic ID. We should get an InconsistentTopicIdException.
     // This scenario should not occur, since the topic ID check will fail.
-    assertThrows(classOf[IllegalStateException], () => partition2.makeLeader(leaderState, offsetCheckpoints, Some(Uuid.randomUuid())))
+    assertThrows(classOf[InconsistentTopicIdException], () => partition2.makeLeader(leaderState, offsetCheckpoints, Some(Uuid.randomUuid())))
 
     // Calling makeLeader with no topic ID should not overwrite the old topic ID. We should get the original log.
     partition2.makeLeader(leaderState, offsetCheckpoints, None)
@@ -1716,9 +1716,9 @@ class PartitionTest extends AbstractPartitionTest {
     assertEquals(topicId, partition2.topicId.get)
     assertFalse(partition2.log.isDefined)
 
-    // Calling makeFollower with a new topic ID should not overwrite the old topic ID. We should get an IllegalStateException.
+    // Calling makeFollower with a new topic ID should not overwrite the old topic ID. We should get an InconsistentTopicIdException.
     // This scenario should not occur, since the topic ID check will fail.
-    assertThrows(classOf[IllegalStateException], () => partition2.makeFollower(leaderState, offsetCheckpoints, Some(Uuid.randomUuid())))
+    assertThrows(classOf[InconsistentTopicIdException], () => partition2.makeFollower(leaderState, offsetCheckpoints, Some(Uuid.randomUuid())))
 
     // Calling makeFollower with no topic ID should not overwrite the old topic ID. We should get the original log.
     partition2.makeFollower(leaderState, offsetCheckpoints, None)
diff --git a/core/src/test/scala/unit/kafka/log/LogTest.scala b/core/src/test/scala/unit/kafka/log/LogTest.scala
index 08c62099e4f1f..42974d165e277 100755
--- a/core/src/test/scala/unit/kafka/log/LogTest.scala
+++ b/core/src/test/scala/unit/kafka/log/LogTest.scala
@@ -2586,7 +2586,7 @@ class LogTest {
       log = createLog(logDir, logConfig, topicId = Some(Uuid.randomUuid()))
       log.close()
     } catch {
-      case e: Throwable => assertTrue(e.isInstanceOf[IllegalStateException])
+      case e: Throwable => assertTrue(e.isInstanceOf[InconsistentTopicIdException])
     }
   }