Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINOR: Update stream documentation #8622

Merged
merged 5 commits into from
May 19, 2020
Merged

Conversation

showuon
Copy link
Contributor

@showuon showuon commented May 6, 2020

  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)

1. fix broken links
2. rephrase a sentence
3. update the version number
@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.4 is now the possible old version value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@bbejeck bbejeck May 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @ableegoldman @abbccdda @bbejeck , I've reverted back this version to 2.3 now in this commit 460768e. Thank you very much!

@@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@showuon showuon changed the title update stream documentation MINOR: Update stream documentation May 6, 2020
@showuon
Copy link
Contributor Author

showuon commented May 7, 2020

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

@bbejeck
Copy link
Contributor

bbejeck commented May 7, 2020

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

@showuon
Copy link
Contributor Author

showuon commented May 8, 2020

Thanks, @bbejeck , take your time.

@@ -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>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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>
Copy link
Contributor Author

@showuon showuon May 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the broken link.

@showuon
Copy link
Contributor Author

showuon commented May 14, 2020

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

Copy link
Contributor

@abbccdda abbccdda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch! Overall LGTM, just a minor comment.

@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@showuon
Copy link
Contributor Author

showuon commented May 16, 2020

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.

Copy link
Contributor

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@showuon thanks for the contribution! LGTM. Once we get confirmation on the rolling upgrade section we can merge this.

I validated these changes by running asf-site locally.

Also, can you prepare an identical PR for https://github.com/apache/kafka-site/tree/asf-site/25 ? This is needed so the current live docs are updated. Thanks!

@@ -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
Copy link
Contributor

@bbejeck bbejeck May 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@showuon
Copy link
Contributor Author

showuon commented May 18, 2020

No problem @bbejeck . PR to update the version 25 doc inkafka-site repo is ready: apache/kafka-site#265. Thank you.

@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@showuon showuon May 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for suggestion, @ableegoldman , it makes it more clear! I've updated in this commit a3accf6. Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also updated in the kafka-site repo: apache/kafka-site@513a820. Thank you.

@bbejeck bbejeck merged commit 82fb6e9 into apache:trunk May 19, 2020
@bbejeck
Copy link
Contributor

bbejeck commented May 19, 2020

Thanks @showuon for the contribution!

@bbejeck
Copy link
Contributor

bbejeck commented May 19, 2020

Merged #8622 into trunk.

@showuon showuon deleted the stream_doc_update branch May 20, 2020 01:39
Kvicii pushed a commit to Kvicii/kafka that referenced this pull request May 21, 2020
* 'trunk' of github.com:apache/kafka:
  MINOR: Increase gradle daemon’s heap size to 2g (apache#8700)
  KAFKA-9603: Do not turn on bulk loading for segmented stores on stand-by tasks (apache#8661)
  KAFKA-9859 / kafka-streams-application-reset tool doesn't take into account topics generated by KTable foreign key join operation (apache#8671)
  MINOR: Fix redundant typos in comments and javadocs (apache#8693)
  KAFKA-10010: Should make state store registration idempotent (apache#8681)
  KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll (apache#8682)
  KAFKA-9992: Eliminate JavaConverters in EmbeddedKafkaCluster (apache#8673)
  KAFKA-6145: Add unit tests to verify fix of bug KAFKA-9173 (apache#8689)
  MINOR: Update stream documentation (apache#8622)
  MINOR: Small fixes in the documentation (apache#8623)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants