junrao commented on code in PR #20334:
URL: https://github.com/apache/kafka/pull/20334#discussion_r2747756859


##########
docs/upgrade.html:
##########
@@ -113,6 +113,35 @@ <h5><a id="upgrade_420_notable" 
href="#upgrade_420_notable">Notable changes in 4
     <li>
         The <code>num.replica.fetchers</code> config has a new lower bound of 
1.
     </li>
+    <li>
+        Improvements have been made to the validation rules and default values 
of LIST-type configurations
+        (<a href="https://cwiki.apache.org/confluence/x/HArXF";>KIP-1161</a>).
+        <ul>
+            <li>
+                LIST-type configurations now enforce stricter validation:
+                <ul>
+                    <li>Null values are no longer accepted for most LIST-type 
configurations, except those that explicitly
+                        allow a null default value or where a null value has a 
well-defined semantic meaning.</li>
+                    <li>Duplicate entries within the same list are no longer 
permitted.</li>

Review Comment:
   > However, BrokerMetadataPublisher calls initializeManagers before 
dynamicConfigPublisher.onMetadataUpdate(delta, newImage), so the LogManager 
cannot see either the latest broker-level configs or the latest cluster-level 
configs if those dynamic configs are from the tail of the metadata log.
   
   The following is my understanding. DynamicBrokerConfig adds DynamicLogConfig 
as a reconfigurable.
     def addReconfigurables(kafkaServer: KafkaBroker): Unit = {
        ...
       addBrokerReconfigurable(new DynamicLogConfig(kafkaServer.logManager))
     }
   
   During the first BrokerMetadataPublisher.onMetadataUpdate(), the passed in 
metadata reflects all metadata up to KRaft log's high watermark. We initialize 
LogManger first and then call dynamicConfigPublisher.onMetadataUpdate(delta, 
newImage). This will then call BrokerConfigHandler, DynamicBrokerConfig, and 
DynamicLogConfig, which will update the config in LogManager. So by the time 
the broker starts, LogManger should have the update to date broker/topic level 
configs. Is this not correct?
   



-- 
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