[GitHub] [kafka] showuon commented on a change in pull request #10591: Fix minor bugs in the existing documentation

[GitBox <gi...@apache.org>]

showuon commented on a change in pull request #10591:
URL: https://github.com/apache/kafka/pull/10591#discussion_r620956779



##########
File path: docs/ops.html
##########
@@ -78,7 +78,7 @@ <h4 class="anchor-heading"><a id="basic_ops_leader_balancing" class="anchor-link
 
   <pre class="line-numbers"><code class="language-text">      auto.leader.rebalance.enable=true</code></pre>
     You can also set this to false, but you will then need to manually restore leadership to the restored replicas by running the command:
-  <pre class="line-numbers"><code class="language-bash">  &gt; bin/kafka-preferred-replica-election.sh --bootstrap-server broker_host:port</code></pre>
+  <pre class="line-numbers"><code class="language-bash">  &gt; bin/kafka-leader-election.sh --bootstrap-server broker_host:port</code></pre>

Review comment:
       The paragraph is talking about `preferred replicas` update, why do we need to change to use `kafka-leader-election.sh`? 

##########
File path: docs/security.html
##########
@@ -319,7 +319,7 @@ <h5>SSL key and certificates in PEM format</h5>
             We need to configure the following property in server.properties, which must have one or more comma-separated values:
             <pre>listeners</code></pre>
 
-            If SSL is not enabled for inter-broker communication (see below for how to enable it), both PLAINTEXT and SSL ports will be necessary.
+            If SSL is enabled for inter-broker communication (see below for how to enable it), both PLAINTEXT and SSL ports will be necessary.

Review comment:
       I think the section is saying:
   > If SSL is not enabled, we need to set both  PLAINTEXT and SSL ports, but if SSL is enabled, we only need SSL ports. 
   
   What do you think?

##########
File path: docs/upgrade.html
##########
@@ -420,11 +420,11 @@ <h4><a id="upgrade_2_4_0" href="#upgrade_2_4_0">Upgrading from 0.8.x, 0.9.x, 0.1
         if there are no snapshot files in 3.4 data directory. For more details about the workaround please refer to <a href="https://cwiki.apache.org/confluence/display/ZOOKEEPER/Upgrade+FAQ">ZooKeeper Upgrade FAQ</a>.
     </li>
     <li>
-        An embedded Jetty based <a href="http://zookeeper.apache.org/doc/r3.5.6/zookeeperAdmin.html#sc_adminserver">AdminServer</a> added in ZooKeeper 3.5.
+        An embedded Jetty based <a href="http://zookeeper.apache.org/doc/r3.5.7/zookeeperAdmin.html#sc_adminserver">AdminServer</a> added in ZooKeeper 3.5.

Review comment:
       Actually, we're using zookeeper 3.5.9. Please help update it here and below. Thanks.

##########
File path: docs/upgrade.html
##########
@@ -337,8 +337,8 @@ <h5 class="anchor-heading"><a id="upgrade_250_notable" class="anchor-link"></a><
         <a href="https://github.com/apache/kafka/tree/2.5/examples">examples</a> folder. Check out
         <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-447%3A+Producer+scalability+for+exactly+once+semantics">KIP-447</a>
         for the full details.</li>
-    <li>Added a new public api <code>KafkaStreams.queryMetadataForKey(String, K, Serializer) to get detailed information on the key being queried.
-        It provides information about the partition number where the key resides in addition to hosts containing the active and standby partitions for the key.</code></li>
+    <li>Added a new public api <code>KafkaStreams.queryMetadataForKey(String, K, Serializer)</code> to get detailed information on the key being queried.

Review comment:
       nice catch!

##########
File path: docs/toc.html
##########
@@ -79,11 +79,15 @@
                         <li><a href="#basic_ops_modify_topic">Modifying topics</a>
                         <li><a href="#basic_ops_restarting">Graceful shutdown</a>
                         <li><a href="#basic_ops_leader_balancing">Balancing leadership</a>
-                        <li><a href="#basic_ops_consumer_lag">Checking consumer position</a>
+                        <li><a href="#basic_ops_racks">Balancing Replicas Across Racks</a>
                         <li><a href="#basic_ops_mirror_maker">Mirroring data between clusters</a>
+                        <li><a href="#basic_ops_consumer_lag">Checking consumer position</a>
+                        <li><a href="#basic_ops_consumer_group">Managing Consumer Groups</a>

Review comment:
       Nice improvement to add missing TOC!

##########
File path: docs/upgrade.html
##########
@@ -136,8 +136,8 @@ <h5><a id="upgrade_270_notable" href="#upgrade_270_notable">Notable changes in 2
         The 2.7.0 release includes the core Raft implementation specified in
         <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-595%3A+A+Raft+Protocol+for+the+Metadata+Quorum">KIP-595</a>.
         There is a separate "raft" module containing most of the logic. Until integration with the
-        controller is complete, there is a standalone server that users can use for testing the p
-        erformance of the Raft implementation.  See the README.md in the raft module for details

Review comment:
       Nice catch!




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

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