divijvaidya commented on code in PR #13703:
URL: https://github.com/apache/kafka/pull/13703#discussion_r1203778428


##########
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java:
##########
@@ -473,8 +488,8 @@ public void replay(PartitionChangeRecord record) {
 
         if (record.removingReplicas() != null || record.addingReplicas() != 
null) {
             log.info("Replayed partition assignment change {} for topic {}", 
record, topicInfo.name);
-        } else if (log.isTraceEnabled()) {
-            log.trace("Replayed partition change {} for topic {}", record, 
topicInfo.name);
+        } else if (log.isDebugEnabled()) {

Review Comment:
   Do we really need this check? My understanding is that the parameterised 
messages (which we are using here) removes the requirement of this check. see: 
https://www.slf4j.org/faq.html#logging_performance 



##########
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java:
##########
@@ -377,7 +377,19 @@ private ReplicationControlManager(
     }
 
     public void replay(TopicRecord record) {
-        topicsByName.put(record.name(), record.topicId());
+        Uuid existingUuid = topicsByName.put(record.name(), record.topicId());
+        if (existingUuid != null) {
+            // We don't currently support sending a second TopicRecord for the 
same topic name...
+            // unless, of course, there is a RemoveTopicRecord in between.
+            if (existingUuid.equals(record.topicId())) {
+                throw new RuntimeException("Found duplicate TopicRecord for " 
+ record.name() +
+                        " with topic ID " + record.topicId());
+            } else {
+                throw new RuntimeException("Found duplicate TopicRecord for " 
+ record.name() +
+                        " with a different ID than before. Previous ID was " + 
existingUuid +
+                        " and new ID is " + record.topicId());
+            }
+        }

Review Comment:
   1. This should perhaps go in a separate PR? It would help keeping commits as 
separate (thus helping in faster rollback if required)
   2. I am curious to understand where the RuntimeException gets logged 
(probably handled by event queue?)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to