KAFKA-3697: Update quick start documentation to use the new consumer #1921

vahidhashemian · 2016-09-27T23:09:57Z

This is to imply that the Java consumer/producer are the recommended consumer/producer now.

vahidhashemian · 2016-09-29T17:28:28Z

hachikuji · 2016-09-29T19:58:17Z

docs/configuration.html

@@ -70,7 +70,7 @@

 <h3><a id="consumerconfigs" href="#consumerconfigs">3.3 Consumer Configs</a></h3>

-We introduce both the old 0.8 consumer configs and the new consumer configs respectively below.
+We introduce both the old 0.8 consumer configs and the Java consumer configs respectively below.


Might be just me, but it reads a little awkwardly to mention the "0.8 consumer configs" contrasted with the "Java consumer configs." Maybe no need to mention versions? Also maybe we should swap the order?

We introduce both the Java consumer configs and the older Scala consumer configs respectively below.

hachikuji · 2016-09-29T20:01:16Z

docs/configuration.html

@@ -143,7 +143,7 @@
    <tr>
      <td>rebalance.max.retries</td>
      <td colspan="1">4</td>
-      <td>When a new consumer joins a consumer group the set of consumers attempt to "rebalance" the load to assign partitions to each consumer. If the set of consumers changes while this assignment is taking place the rebalance will fail and retry. This setting controls the maximum number of attempts before giving up.</td>
+      <td>When a Java consumer joins a consumer group the set of consumers attempt to "rebalance" the load to assign partitions to each consumer. If the set of consumers changes while this assignment is taking place the rebalance will fail and retry. This setting controls the maximum number of attempts before giving up.</td>


Looks like a mistaken find/replace?

Sorry for all the mistaken find/replaces; and thanks for catching them. Will fix those in the next update.

hachikuji · 2016-09-29T20:03:01Z

docs/configuration.html

-<h4><a id="newconsumerconfigs" href="#newconsumerconfigs">3.3.2 New Consumer Configs</a></h4>
-Since 0.9.0.0 we have been working on a replacement for our existing simple and high-level consumers. The code is considered beta quality. Below is the configuration for the new consumer:
+<h4><a id="newconsumerconfigs" href="#newconsumerconfigs">3.3.2 Java Consumer Configs</a></h4>
+Since 0.9.0.0 we have been working on a replacement for our existing simple and high-level consumers. The code is considered beta quality. Below is the configuration for the Java consumer:


This comment seems a little old. While it's true that we're still working on it, it's more or less a completed product. Maybe we just say that it was introduce in 0.9.0.0? Also, we should remove the mention of "beta quality." We merged a PR about this previously, but it looks like we missed this.

hachikuji · 2016-09-29T20:03:26Z

docs/configuration.html

@@ -128,7 +128,7 @@
    <tr>
      <td>auto.commit.enable</td>
      <td colspan="1">true</td>
-      <td>If true, periodically commit to ZooKeeper the offset of messages already fetched by the consumer. This committed offset will be used when the process fails as the position from which the new consumer will begin.</td>
+      <td>If true, periodically commit to ZooKeeper the offset of messages already fetched by the consumer. This committed offset will be used when the process fails as the position from which the Java consumer will begin.</td>


Another mistaken find/replace.

hachikuji · 2016-09-29T20:05:42Z

docs/implementation.html

@@ -372,7 +372,7 @@
 <ol>
   <li> Register itself in the consumer id registry under its group.
   </li>
-   <li> Register a watch on changes (new consumers joining or any existing consumers leaving) under the consumer id registry. (Each change triggers rebalancing among all consumers within the group to which the changed consumer belongs.)
+   <li> Register a watch on changes (Java consumers joining or any existing consumers leaving) under the consumer id registry. (Each change triggers rebalancing among all consumers within the group to which the changed consumer belongs.)


Mistaken find/replace.

hachikuji · 2016-09-29T20:08:25Z

docs/ops.html

@@ -143,7 +143,7 @@
 </pre>


-Note, however, after 0.9.0, the kafka.tools.ConsumerOffsetChecker tool is deprecated and you should use the kafka.admin.ConsumerGroupCommand (or the bin/kafka-consumer-groups.sh script) to manage consumer groups, including consumers created with the <a href="http://kafka.apache.org/documentation.html#newconsumerapi">new consumer API</a>.
+Note, however, after 0.9.0, the kafka.tools.ConsumerOffsetChecker tool is deprecated and you should use the kafka.admin.ConsumerGroupCommand (or the bin/kafka-consumer-groups.sh script) to manage consumer groups, including consumers created with the <a href="http://kafka.apache.org/documentation.html#newconsumerapi">Java consumer API</a>.


First sentence reads painfully. Maybe we could do it like this:

NOTE: Since 0.9.0.0, the kafka.tools.ConsumerOffsetChecker tool has been deprecated. You should use the kafka.admin.ConsumerGroupCommand (or the bin/kafka-consumer-groups.sh script) to manage consumer groups, including consumers...

hachikuji · 2016-09-29T20:10:21Z

docs/implementation.html

@@ -40,9 +40,9 @@

 The goal is to expose all the producer functionality through a single API to the client.

-The new producer -
+The Java producer


I'm wondering if it's really necessary to mention Java. It's the only producer that we mention in this documentation, so can't we just call it the "Kafka Producer"?

I thought about this too. Then I noticed that there are references to "old producer" in the upgrade section. If that doesn't cause a confusion I would also like to use "Kafka producer".

ijuma · 2016-09-29T20:17:20Z

I'm not really sure about this PR for 0.10.1, we have used the term "new producer" and "new consumer" quite a bit and it is perhaps a bit too early to remove that. Maybe we should merge this in trunk but leave 0.10.1 as is? My thinking is that in 0.10.1, -new-consumer is no longer needed and by next release, we have a good case for getting rid of the term. We would want to update the tool option documentation too, in that case.

vahidhashemian · 2016-09-29T20:30:39Z

@ijuma It makes sense that we hold on to the terms 'new consumer' and 'old consumer' a bit longer. How about updating the quick start examples to use the new consumer (which is part of this PR)? Should I open a separate minor PR to update those examples for 0.10.1?

hachikuji · 2016-09-29T20:34:13Z

Seems like we could drop the "new" terminology for the producer at least. It's hardly new anymore and we don't even support the old one. As long as we continue supporting both consumers, however, the "new" word is useful to distinguish which one users should be using.

ijuma · 2016-09-29T20:40:50Z

@hachikuji the old producers are still supported, they've just been deprecated (which only means that they will be removed in a future major release and that they won't be gaining new features).

@vahidhashemian updating the quick start examples sounds great. Also, the remaining "beta" note for the new consumer that I missed last time around should be fixed.

hachikuji · 2016-09-29T20:49:01Z

@ijuma Fair enough. Looking through the documentation, we're not consistent on the usage. In most places, it's just "the producer," or even "the Java producer." I doubt anyone would care if we dropped the word "new" from the other mentions, but no strong preference here.

ijuma · 2016-09-29T20:52:38Z

if we're not consistent already, then fair enough.

vahidhashemian · 2016-09-29T21:02:03Z

@ijuma @hachikuji Updated the PR as advised. Also dropped 'new' from producer mentions as per latest comments.

On a separate note, I've been wondering if there is anyway to stop outdated PR builds on Jenkins? Sometimes I push a few updates in a short timeframe and it would be helpful to stop the old builds to free up Jenkins resources. Thanks.

hachikuji · 2016-09-29T21:23:56Z

docs/security.html

@@ -715,7 +715,7 @@
 	<li> Set the configuration property <tt>zookeeper.set.acl</tt> in each broker to true</li>
 </ol>

-The metadata stored in ZooKeeper is such that only brokers will be able to modify the corresponding znodes, but znodes are world readable. The rationale behind this decision is that the data stored in ZooKeeper is not sensitive, but inappropriate manipulation of znodes can cause cluster disruption. We also recommend limiting the access to ZooKeeper via network segmentation (only brokers and some admin tools need access to ZooKeeper if the new consumer and new producer are used).
+The metadata stored in ZooKeeper is such that only brokers will be able to modify the corresponding znodes, but znodes are world readable. The rationale behind this decision is that the data stored in ZooKeeper is not sensitive, but inappropriate manipulation of znodes can cause cluster disruption. We also recommend limiting the access to ZooKeeper via network segmentation (only brokers and some admin tools need access to ZooKeeper if the new consumer and Java producer are used).


Maybe this could be "... if the new Java consumer and producer clients are used."

hachikuji · 2016-09-29T21:24:03Z

docs/ops.html

@@ -900,9 +900,9 @@
  </tbody>
 </table>

-<h4><a id="new_producer_monitoring" href="#new_producer_monitoring">New producer monitoring</a></h4>
+<h4><a id="new_producer_monitoring" href="#new_producer_monitoring">Producer monitoring</a></h4>


Perhaps we may as well update the id as well?

hachikuji · 2016-09-29T21:27:01Z

docs/configuration.html

@@ -70,9 +70,13 @@

 <h3><a id="consumerconfigs" href="#consumerconfigs">3.3 Consumer Configs</a></h3>

-We introduce both the old 0.8 consumer configs and the new consumer configs respectively below.
+We introduce both the new Java consumer configs and the older Scala consumer configs respectively below.


Maybe even more concisely: "We include both the new Java and the older Scala consumer configs below."

hachikuji · 2016-09-29T21:29:18Z

docs/configuration.html


-<h4><a id="oldconsumerconfigs" href="#oldconsumerconfigs">3.3.1 Old Consumer Configs</a></h4>
+<h4><a id="newconsumerconfigs" href="#newconsumerconfigs">3.3.1 New Consumer Configs</a></h4>
+In 0.9.0.0 we introduced a replacement for simple and high-level consumers. Below is the configuration for the new consumer:


Maybe "In 0.9.0.0 we introduced the new Java consumer as a replacement for the older Scala-based simple and high-level consumers."

Also, would it make sense to move this to the paragraph above ("We introduce both...")?

Sounds good. Please check the variation I used in the updated PR and let me know if it needs further modification. Thanks.

Yep, looks good.

hachikuji · 2016-09-29T23:04:52Z

docs/security.html

@@ -715,7 +715,7 @@
 	<li> Set the configuration property <tt>zookeeper.set.acl</tt> in each broker to true</li>
 </ol>

-The metadata stored in ZooKeeper is such that only brokers will be able to modify the corresponding znodes, but znodes are world readable. The rationale behind this decision is that the data stored in ZooKeeper is not sensitive, but inappropriate manipulation of znodes can cause cluster disruption. We also recommend limiting the access to ZooKeeper via network segmentation (only brokers and some admin tools need access to ZooKeeper if the new consumer and new producer are used).
+The metadata stored in ZooKeeper is such that only brokers will be able to modify the corresponding znodes, but znodes are world readable. The rationale behind this decision is that the data stored in ZooKeeper is not sensitive, but inappropriate manipulation of znodes can cause cluster disruption. We also recommend limiting the access to ZooKeeper via network segmentation (only brokers and some admin tools need access to ZooKeeper if the new Java consumer and producer clients are used).


The first sentence has some redundancy. Maybe something like this:

The metadata stored in ZooKeeper for the Kafka cluster is world-readable, but can only be modified by the brokers.

Done in the updated PR. Thanks.

hachikuji · 2016-09-30T00:53:33Z

docs/security.html

@@ -715,7 +715,7 @@
 	<li> Set the configuration property <tt>zookeeper.set.acl</tt> in each broker to true</li>
 </ol>

-The metadata stored in ZooKeeper is such that only brokers will be able to modify the corresponding znodes, but znodes are world readable. The rationale behind this decision is that the data stored in ZooKeeper is not sensitive, but inappropriate manipulation of znodes can cause cluster disruption. We also recommend limiting the access to ZooKeeper via network segmentation (only brokers and some admin tools need access to ZooKeeper if the new consumer and new producer are used).
+The metadata stored in ZooKeeper for the Kafka cluster is world-readable, but can only be modified by the brokers. We also recommend limiting the access to ZooKeeper via network segmentation (only brokers and some admin tools need access to ZooKeeper if the new Java consumer and producer clients are used).


Did you mean to take out the second line? Seems helpful to know the reason. Probably just no need to mention znodes directly.

Sorry my bad. I added it back.

hachikuji · 2016-09-30T00:54:16Z

@vahidhashemian Left a minor comment on the recent commit. The rest LGTM.

…nd drop 'new' from producer mentions

This is to imply that the Java consumer/producer are the recommended consumer/producer now. Author: Vahid Hashemian <vahidhashemian@us.ibm.com> Reviewers: Jason Gustafson <jason@confluent.io> Closes #1921 from vahidhashemian/KAFKA-3697 (cherry picked from commit d2a267b) Signed-off-by: Jason Gustafson <jason@confluent.io>

hachikuji · 2016-09-30T02:38:21Z

Merged to trunk and 0.10.1. Thanks for the patch!

vahidhashemian force-pushed the KAFKA-3697 branch 3 times, most recently from 1922e65 to cb1b984 Compare September 29, 2016 17:26

hachikuji reviewed Sep 29, 2016

View reviewed changes

vahidhashemian force-pushed the KAFKA-3697 branch from cb1b984 to 1ccd3cc Compare September 29, 2016 20:51

vahidhashemian changed the title ~~KAFKA-3697: Refer to "new" consumer/producer as "Java" consumer/producer in the documentation~~ KAFKA-3697: Update quick start documentation to use the new consumer Sep 29, 2016

vahidhashemian force-pushed the KAFKA-3697 branch from 1ccd3cc to 052cf00 Compare September 29, 2016 20:59

hachikuji reviewed Sep 29, 2016

View reviewed changes

vahidhashemian force-pushed the KAFKA-3697 branch from 052cf00 to ba2f9f3 Compare September 29, 2016 21:56

hachikuji reviewed Sep 29, 2016

View reviewed changes

vahidhashemian force-pushed the KAFKA-3697 branch from ba2f9f3 to ba4cba7 Compare September 30, 2016 00:47

hachikuji reviewed Sep 30, 2016

View reviewed changes

vahidhashemian force-pushed the KAFKA-3697 branch 2 times, most recently from 6354547 to 5879386 Compare September 30, 2016 01:10

KAFKA-3697: Update quickstart documentation to use the new consumer a…

9e0bce8

…nd drop 'new' from producer mentions

vahidhashemian force-pushed the KAFKA-3697 branch from 5879386 to 9e0bce8 Compare September 30, 2016 01:52

asfgit closed this in d2a267b Sep 30, 2016

efeg pushed a commit to efeg/kafka that referenced this pull request May 29, 2024

Add Vertx.io based API with swagger UI (apache#1921)

1619795

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-3697: Update quick start documentation to use the new consumer #1921

KAFKA-3697: Update quick start documentation to use the new consumer #1921

vahidhashemian commented Sep 27, 2016

vahidhashemian commented Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

vahidhashemian Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

vahidhashemian Sep 29, 2016

ijuma commented Sep 29, 2016

vahidhashemian commented Sep 29, 2016 •

edited

Loading

hachikuji commented Sep 29, 2016

ijuma commented Sep 29, 2016

hachikuji commented Sep 29, 2016

ijuma commented Sep 29, 2016

vahidhashemian commented Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

vahidhashemian Sep 29, 2016

hachikuji Sep 29, 2016

hachikuji Sep 29, 2016

vahidhashemian Sep 30, 2016

hachikuji Sep 30, 2016

vahidhashemian Sep 30, 2016

hachikuji commented Sep 30, 2016

hachikuji commented Sep 30, 2016

KAFKA-3697: Update quick start documentation to use the new consumer #1921

KAFKA-3697: Update quick start documentation to use the new consumer #1921

Conversation

vahidhashemian commented Sep 27, 2016

vahidhashemian commented Sep 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijuma commented Sep 29, 2016

vahidhashemian commented Sep 29, 2016 • edited Loading

hachikuji commented Sep 29, 2016

ijuma commented Sep 29, 2016

hachikuji commented Sep 29, 2016

ijuma commented Sep 29, 2016

vahidhashemian commented Sep 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hachikuji commented Sep 30, 2016

hachikuji commented Sep 30, 2016

vahidhashemian commented Sep 29, 2016 •

edited

Loading