mimaison commented on code in PR #20237:
URL: https://github.com/apache/kafka/pull/20237#discussion_r2243374479
##########
docs/ops.html:
##########
@@ -4499,8 +4499,15 @@ <h4 class="anchor-heading"><a
id="eligible_leader_replicas_upgrade" class="ancho
<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>
+ Note that when the ELR feature is enabled:
+ <ul>
+ <li>The override of <code>min.insync.replicas</code> in broker-level
will be removed.</li>
+ <li>The alter of <code>min.insync.replicas</code> config in broker-level
is not allowed.</li>
Review Comment:
`The alter` -> `The alteration`
##########
clients/src/main/java/org/apache/kafka/common/config/TopicConfig.java:
##########
@@ -181,7 +181,8 @@ public class TopicConfig {
"When used together, <code>min.insync.replicas</code> and
<code>acks</code> allow you to enforce greater durability guarantees. " +
"A typical scenario would be to create a topic with a replication
factor of 3, " +
"set <code>min.insync.replicas</code> to 2, and produce with
<code>acks</code> of \"all\". " +
- "This will ensure that a majority of replicas must persist a write
before it's considered successful by the producer and it's visible to
consumers.";
+ "This will ensure that a majority of replicas must persist a write
before it's considered successful by the producer and it's visible to
consumers." +
Review Comment:
I think we can stick to present tense: `This ensures that ...`
##########
docs/ops.html:
##########
@@ -4499,8 +4499,15 @@ <h4 class="anchor-heading"><a
id="eligible_leader_replicas_upgrade" class="ancho
<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>
+ Note that when the ELR feature is enabled:
+ <ul>
+ <li>The override of <code>min.insync.replicas</code> in broker-level
will be removed.</li>
+ <li>The alter 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>
+ <li>If <code>min.insync.replicas</code> is updated for a topic, the ELR
field will be cleaned.</li>
Review Comment:
Can we switch to present tense? Also what does `will be cleaned` means
exactly?
##########
docs/ops.html:
##########
@@ -4499,8 +4499,15 @@ <h4 class="anchor-heading"><a
id="eligible_leader_replicas_upgrade" class="ancho
<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>
+ Note that when the ELR feature is enabled:
+ <ul>
+ <li>The override of <code>min.insync.replicas</code> in broker-level
will be removed.</li>
Review Comment:
`will be removed` -> `is removed`
##########
clients/src/main/java/org/apache/kafka/common/config/TopicConfig.java:
##########
@@ -181,7 +181,8 @@ public class TopicConfig {
"When used together, <code>min.insync.replicas</code> and
<code>acks</code> allow you to enforce greater durability guarantees. " +
"A typical scenario would be to create a topic with a replication
factor of 3, " +
"set <code>min.insync.replicas</code> to 2, and produce with
<code>acks</code> of \"all\". " +
- "This will ensure that a majority of replicas must persist a write
before it's considered successful by the producer and it's visible to
consumers.";
+ "This will ensure that a majority of replicas must persist a write
before it's considered successful by the producer and it's visible to
consumers." +
+ "<p> Note that when the Eligible Leader Replicas feature is enabled,
the semantics of this config has small changes. Please refer to <a
href=\"#eligible_leader_replicas\">the ELR section</a> for more info.</p>";
Review Comment:
`config has small changes` -> `config change`
##########
docs/ops.html:
##########
@@ -4499,8 +4499,15 @@ <h4 class="anchor-heading"><a
id="eligible_leader_replicas_upgrade" class="ancho
<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>
+ Note that when the ELR feature is enabled:
+ <ul>
+ <li>The override of <code>min.insync.replicas</code> in broker-level
will be removed.</li>
+ <li>The alter 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>
+ <li>If <code>min.insync.replicas</code> is updated for a topic, the ELR
field will be cleaned.</li>
+ <li>If the cluster default <code>min.insync.replicas</code> is updated,
all the ELR fields will be cleaned.</li>
Review Comment:
Ditto
--
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]
