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


##########
docs/ops.html:
##########
@@ -4498,9 +4498,17 @@ <h4 class="anchor-heading"><a 
id="eligible_leader_replicas_upgrade" class="ancho
   <p>Downgrades are safe to perform by setting 
<code>eligible.leader.replicas.version=0</code>.</p>
 
   <h4 class="anchor-heading"><a id="eligible_leader_replicas_tool" 
class="anchor-link"></a><a href="#eligible_leader_replicas_tool">Tool</a></h4>
-  <p>The ELR fields can be checked through the API DescribeTopicPartitions. 
The admin client can fetch the ELR info by describing the topics.
-    Also note that, if <code>min.insync.replicas</code> is updated for a 
topic, the ELR field will be cleaned. If cluster default min ISR is updated,
-    all the ELR fields will be cleaned.</p>
+  <p>The ELR fields can be checked through the API DescribeTopicPartitions. 
The admin client can fetch the ELR info by describing the topics.</p>
+  <p>Note that when the ELR feature is enabled:</p>
+  <ul>
+    <li>The cluster-level <code>min.insync.replicas</code> config will be 
added if there is not any. The value is the same as the static config in the 
active controller.</li>
+    <li>The previously set <code>min.insync.replicas</code> value at the 
broker-level config will be removed.</li>
+    <li>The override of <code>min.insync.replicas</code> in broker-level will 
be removed.</li>

Review Comment:
   The above two line are basically the same. We can just keep one. But it will 
be useful to inform the user to reset min.insync.replicas as the cluster level 
if it's previously set only at the broker level. 



##########
docs/ops.html:
##########
@@ -4498,9 +4498,17 @@ <h4 class="anchor-heading"><a 
id="eligible_leader_replicas_upgrade" class="ancho
   <p>Downgrades are safe to perform by setting 
<code>eligible.leader.replicas.version=0</code>.</p>
 
   <h4 class="anchor-heading"><a id="eligible_leader_replicas_tool" 
class="anchor-link"></a><a href="#eligible_leader_replicas_tool">Tool</a></h4>
-  <p>The ELR fields can be checked through the API DescribeTopicPartitions. 
The admin client can fetch the ELR info by describing the topics.
-    Also note that, if <code>min.insync.replicas</code> is updated for a 
topic, the ELR field will be cleaned. If cluster default min ISR is updated,
-    all the ELR fields will be cleaned.</p>
+  <p>The ELR fields can be checked through the API DescribeTopicPartitions. 
The admin client can fetch the ELR info by describing the topics.</p>
+  <p>Note that when the ELR feature is enabled:</p>
+  <ul>
+    <li>The cluster-level <code>min.insync.replicas</code> config will be 
added if there is not any. The value is the same as the static config in the 
active controller.</li>
+    <li>The previously set <code>min.insync.replicas</code> value at the 
broker-level config will be removed.</li>
+    <li>The override of <code>min.insync.replicas</code> in broker-level will 
be removed.</li>
+    <li>The alteration of <code>min.insync.replicas</code> config in 
broker-level is not allowed.</li>
+    <li>The removal of <code>min.insync.replicas</code> config in 
cluster-level is not allowed.</li>

Review Comment:
   Could we group the cluster level changes next to each other?



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