kevin-wu24 commented on code in PR #21053:
URL: https://github.com/apache/kafka/pull/21053#discussion_r2591110374


##########
metadata/src/main/java/org/apache/kafka/image/ConfigurationsDelta.java:
##########
@@ -21,17 +21,48 @@
 import org.apache.kafka.common.config.ConfigResource.Type;
 import org.apache.kafka.common.metadata.ConfigRecord;
 import org.apache.kafka.common.metadata.RemoveTopicRecord;
+import org.apache.kafka.metadata.KafkaConfigSchema;
 import org.apache.kafka.server.common.MetadataVersion;
 
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
+import java.util.function.Supplier;
 
 
 /**
  * Represents changes to the configurations in the metadata image.
  */
 public final class ConfigurationsDelta {
+    /**

Review Comment:
   We shouldn't need to change this file at all. Remember that each 
`ConfigurationDelta` object will contain the current configuration image for a 
given `ConfigResource`, as well as its deltas. When we call 
`ConfigurationDelta#apply`, that is the only place we need to make changes.



##########
metadata/src/main/java/org/apache/kafka/controller/ConfigurationControlManager.java:
##########
@@ -508,6 +509,16 @@ private List<String> getParts(String value, String key, 
ConfigResource configRes
      */
     public void replay(ConfigRecord record) {
         Type type = Type.forId(record.resourceType());
+        // Filter out invalid configs

Review Comment:
   I think there is a better way to implement this than trying to clean these 
configs up in every place where we replay a ConfigRecord. This approach is 
disjoint and not very maintainable.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to