You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/05/06 03:27:25 UTC

[GitHub] [kafka] showuon opened a new pull request #8622: update stream documentation

showuon opened a new pull request #8622:
URL: https://github.com/apache/kafka/pull/8622


   1. fix broken links
   2. rephrase a sentence
   3. update the version number
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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



[GitHub] [kafka] showuon commented on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-629912212


   No problem  @bbejeck . PR to update then version 25 doc in`kafka-site` repo is ready: https://github.com/apache/kafka-site/pull/265. Thank you.


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



[GitHub] [kafka] showuon commented on a change in pull request #8622: update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r420526237



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       `2.4` is now the possible old version value




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r425054297



##########
File path: docs/streams/developer-guide/dsl-api.html
##########
@@ -3738,7 +3738,7 @@ <h5><a class="toc-backref" href="#id34">KTable-KTable Foreign-Key
             </div>
         </div>
         <div class="section" id="testing-a-streams-app">
-            <a class="headerlink" href="#testing-a-streams-app" title="Permalink to this headline"><h2>Testing a Streams application</h2></a>
+            <h2><a class="headerlink" href="#testing-a-streams-app" title="Permalink to this headline">Testing a Streams application</a></h2>

Review comment:
       Currently, the `TESTING A STREAMS APPLICATION` header is in blue, which is different from others. Turns out that it's because we put `<h2>` tag inside `<a>`, and cause to apply the wrong css format.
   ![image](https://user-images.githubusercontent.com/43372967/81927117-b1d18780-9615-11ea-979b-db48c294e742.png)
   




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



[GitHub] [kafka] bbejeck merged pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
bbejeck merged pull request #8622:
URL: https://github.com/apache/kafka/pull/8622


   


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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r425055442



##########
File path: docs/streams/developer-guide/running-app.html
##########
@@ -51,7 +51,7 @@
           <span id="streams-developer-guide-execution"></span><h1>Running Streams Applications<a class="headerlink" href="#running-streams-applications" title="Permalink to this headline"></a></h1>
           <p>You can run Java applications that use the Kafka Streams library without any additional configuration or requirements. Kafka Streams
               also provides the ability to receive notification of the various states of the application. The ability to monitor the runtime
-              status is discussed in <a class="reference internal" href="../monitoring.html#streams-monitoring"><span class="std std-ref">the monitoring guide</span></a>.</p>
+              status is discussed in <a class="reference internal" href="/documentation/#kafka_streams_monitoring"><span class="std std-ref">the monitoring guide</span></a>.</p>

Review comment:
       Fix the broken link. 




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r425055023



##########
File path: docs/streams/developer-guide/write-streams.html
##########
@@ -208,7 +208,7 @@ <h2>Using Kafka Streams within your application code<a class="headerlink" href="
 </div>
 
       <div class="section" id="testing-a-streams-app">
-          <a class="headerlink" href="#testing-a-streams-app" title="Permalink to this headline"><h2>Testing a Streams application</a></h2>
+          <h2><a class="headerlink" href="#testing-a-streams-app" title="Permalink to this headline">Testing a Streams application</a></h2>

Review comment:
       wrong HTML formatting. The starting/ending `<h2>` tag and `<a>` tag are mixed places.




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r420526581



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -77,7 +77,7 @@ <h3><a id="streams_api_changes_250" href="#streams_api_changes_250">Streams API
         We add a new <code>cogroup()</code> operator (via <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-150+-+Kafka-Streams+Cogroup">KIP-150</a>>)
         that allows to aggregate multiple streams in a single operation.
         Cogrouped streams can also be windowed before they are aggregated.
-        We refer to the <a href="/{{version}}/documentation/streams/developer-guide/dsl-api.html">developer guide</a> for more details.
+        Please refer to the <a href="/{{version}}/documentation/streams/developer-guide/dsl-api.html">developer guide</a> for more details.

Review comment:
       Rephrase this sentence. 
   Before: `We refer to the developer guide for more details.`
   After: `Please refer to the developer guide for more details.`




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427018759



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       Thanks for suggestion, @ableegoldman , it makes it more clear! I've updated in this commit https://github.com/apache/kafka/pull/8622/commits/a3accf681e72bbba9a774a464253c1ccd7746188. Thank you.




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



[GitHub] [kafka] bbejeck commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
bbejeck commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r426163653



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       not sure about this as it describes a rolling upgrade from an older version, so we'll need @ableegoldman to confirm




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



[GitHub] [kafka] bbejeck commented on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
bbejeck commented on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-630882502


   Thanks @showuon for the contribution!


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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427028165



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       I've also updated in the kafka-site repo: https://github.com/apache/kafka-site/pull/265/commits/513a8205e6b115ca4f876aa5d95d3756061266d5. Thank you.




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427012797



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       hi @ableegoldman , After reading the whole paragraph again, I think you're right. 
   > This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. ....  
   > you can safely switch over to cooperative at any time once the entire group is on 2.4+ by removing the config value and bouncing.
   
   So, I think here we keep it as `2.3` is fine and correct. 
   
   Thank you.




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r425055023



##########
File path: docs/streams/developer-guide/write-streams.html
##########
@@ -208,7 +208,7 @@ <h2>Using Kafka Streams within your application code<a class="headerlink" href="
 </div>
 
       <div class="section" id="testing-a-streams-app">
-          <a class="headerlink" href="#testing-a-streams-app" title="Permalink to this headline"><h2>Testing a Streams application</a></h2>
+          <h2><a class="headerlink" href="#testing-a-streams-app" title="Permalink to this headline">Testing a Streams application</a></h2>

Review comment:
       wrong HTML formatting. `<h2>` tag and `<a>` tag are mixed places.




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



[GitHub] [kafka] bbejeck commented on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
bbejeck commented on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-630882690


   Merged #8622 into trunk.


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



[GitHub] [kafka] ableegoldman commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427019844



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       Awesome, thank you!




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427012797



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       hi @ableegoldman , After reading the whole paragraph again, I think you're right. 
   > (possible values are "0.10.0" - "2.3") and during the second you remove it. ....
   > This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. ....  
   > you can safely switch over to cooperative at any time once the entire group is on 2.4+ by removing the config value and bouncing.
   
   So, Because we explicitly said since 2.4+, there'll be cooperative rebalancing protocol available, I think here we keep it as `2.3` is fine and correct. 
   
   Thank you.




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



[GitHub] [kafka] showuon edited a comment on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon edited a comment on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-629912212


   No problem  @bbejeck . PR to update the version 25 doc in`kafka-site` repo is ready: https://github.com/apache/kafka-site/pull/265. Thank you.


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



[GitHub] [kafka] showuon edited a comment on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon edited a comment on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-628563253


   Hi  @bbejeck , I appended more fixs for the streams documents while I'm reading it. Please review it when available. Thank you.


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



[GitHub] [kafka] showuon commented on a change in pull request #8622: update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r420527021



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -688,7 +688,7 @@ <h3><a id="streams_api_changes_100" href="#streams_api_changes_100">Streams API
         If you are monitoring on task level or processor-node / state store level Streams metrics, please note that the metrics sensor name and hierarchy was changed:
         The task ids, store names and processor names are no longer in the sensor metrics names, but instead are added as tags of the sensors to achieve consistent metrics hierarchy.
         As a result you may need to make corresponding code changes on your metrics reporting and monitoring tools when upgrading to 1.0.0.
-        Detailed metrics sensor can be found in the <a href="#kafka_streams_monitoring">Streams Monitoring</a> section.
+        Detailed metrics sensor can be found in the <a href="/{{version}}/documentation/#kafka_streams_monitoring">Streams Monitoring</a> section.

Review comment:
       `#kafka_streams_monitoring` is located in `/documentation`, not the current path (i.e. `documentation/streams/upgrade-guide`)




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



[GitHub] [kafka] bbejeck commented on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
bbejeck commented on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-625287393


   @showuon thanks for the contribution, I'll take a look soon


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



[GitHub] [kafka] showuon edited a comment on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon edited a comment on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-628563253


   Hi  @bbejeck , I appended more fixs for the streams documents. Please review it when available. Thank you.


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



[GitHub] [kafka] showuon commented on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-629607982


   Thanks @abbccdda , I'll wait for @ableegoldman 's confirmation to see if I have to revert this version back to `2.3`. Thank you very much.


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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427012797



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       hi @ableegoldman , After reading the whole paragraph again, I think you're right. 
   > (possible values are "0.10.0" - "2.3") and during the second you remove it. ....
   > This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. ....  
   > you can safely switch over to cooperative at any time once the entire group is on 2.4+ by removing the config value and bouncing.
   
   So, I think here we keep it as `2.3` is fine and correct. 
   
   Thank you.




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



[GitHub] [kafka] ableegoldman commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427013934



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       Can we just add a small qualifier in the first line?
   `you will need to do two rolling bounces` --> `if upgrading from 2.3 or below, you will need to do two rolling bounces`




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



[GitHub] [kafka] showuon edited a comment on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon edited a comment on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-628563253


   Hi  @bbejeck , I appended more fixs for the streams documents when I read it. Please review it when available. Thank you.


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



[GitHub] [kafka] ableegoldman commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427009991



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       @showuon Can we clarify that you only need to do this if you're upgrading from 2.3 or below? I know this seems implied by the fact that the config's possible values stop at 2.3 but there are always creative interpretations of seemingly obvious things 🙂 




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



[GitHub] [kafka] ableegoldman commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r426808431



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       @showuon Boyang is right, there is no `upgrade.from` config for 2.4 since that's when cooperative rebalancing was enabled. So if you upgrade to 2.5 from any version lower than 2.4, you will need to go through this upgrade path and set the config.




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427012797



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       hi @ableegoldman , After reading the whole paragraph again, I think you're right. 
   > (possible values are "0.10.0" - "2.3") and during the second you remove it. ....
   > This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. ....  
   > you can safely switch over to cooperative at any time once the entire group is on 2.4+ by removing the config value and bouncing.
   
   So, Because we explicitly said since 2.4+, there'll be cooperative rebalancing protocol available, I think here we keep it as `2.3` is fine and correct. 
   
   Or do you have any other suggestion? 
   Thank you.




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



[GitHub] [kafka] showuon commented on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-625011283


   Hi @bbejeck , could you please review this small PR? Thanks.


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



[GitHub] [kafka] bbejeck commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
bbejeck commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r426163653



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       I think this change is ok as it describes the rolling upgrades from an older version. But we should @ableegoldman to confirm.




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427009933



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       Thank you, @ableegoldman @abbccdda @bbejeck , I've reverted back this version to `2.3` now in this commit https://github.com/apache/kafka/pull/8622/commits/460768e71f5c7d427a6faffdded9b0478ade1db1. Thank you very much!




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



[GitHub] [kafka] abbccdda commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
abbccdda commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r426121743



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       I think 2.4 already launches the incremental rebalancing, cc @ableegoldman 




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



[GitHub] [kafka] showuon commented on pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#issuecomment-628563253


   Hi  @bbejeck , I appended more fixs for the streams documents. Please review it when available. Thank you.


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



[GitHub] [kafka] ableegoldman commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r427009991



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -35,7 +35,7 @@ <h1>Upgrade Guide and API Changes</h1>
 
     <p>
         Upgrading from any older version to {{fullDotVersion}} is possible: you will need to do two rolling bounces, where during the first rolling bounce phase you set the config <code>upgrade.from="older version"</code>
-        (possible values are <code>"0.10.0" - "2.4"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager
+        (possible values are <code>"0.10.0" - "2.3"</code>) and during the second you remove it. This is required to safely upgrade to the new cooperative rebalancing protocol of the embedded consumer. Note that you will remain using the old eager

Review comment:
       @showuon Can we clarify that you only need to do this if you're upgrading from 2.3 or below? I know this seems implied by the fact that the config's possible values stop at 2.3 but there are always creative interpretations of seemingly obvious thing 🙂 




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



[GitHub] [kafka] showuon commented on a change in pull request #8622: MINOR: Update stream documentation

Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #8622:
URL: https://github.com/apache/kafka/pull/8622#discussion_r425054711



##########
File path: docs/streams/developer-guide/write-streams.html
##########
@@ -154,7 +154,7 @@ <h2>Using Kafka Streams within your application code<a class="headerlink" href="
       </div>
       <p>If there are other instances of this stream processing application running elsewhere (e.g., on another machine), Kafka
         Streams transparently re-assigns tasks from the existing instances to the new instance that you just started.
-        For more information, see <a class="reference internal" href="../architecture.html#streams_architecture_tasks"><span class="std std-ref">Stream Partitions and Tasks</span></a> and <a class="reference internal" href="../architecture.html#streams-architecture-threads"><span class="std std-ref">Threading Model</span></a>.</p>
+        For more information, see <a class="reference internal" href="../architecture.html#streams_architecture_tasks"><span class="std std-ref">Stream Partitions and Tasks</span></a> and <a class="reference internal" href="../architecture.html#streams_architecture_threads"><span class="std std-ref">Threading Model</span></a>.</p>

Review comment:
       fix the broken link from wrong `href="../architecture.html#streams-architecture-threads"` to `href="../architecture.html#streams_architecture_threads"` (the one with underscore)




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