mjsax commented on a change in pull request #9969:
URL: https://github.com/apache/kafka/pull/9969#discussion_r564787782



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -91,6 +91,20 @@ <h3><a id="streams_api_changes_280" 
href="#streams_api_changes_280">Streams API
         We extended <code>StreamJoined</code> to include the options 
<code>withLoggingEnabled()</code> and <code>withLoggingDisabled()</code> in
         <a 
href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-689%3A+Extend+%60StreamJoined%60+to+allow+more+store+configs";>KIP-689</a>.
     </p>
+    <p>
+        We add and option to <code>addThread()</code> and 
<code>removeThread()</code> in
+        <a href="https://cwiki.apache.org/confluence/x/FDd4CQ";>KIP-663</a>.
+        This also caused the KafkaStreams client state machine to change.
+        The ERROR state is now terminal with PENDING_ERROR being a 
transitional state where the resources are closing.
+        When all thread are dead there is not automatic transition to ERROR as 
a new thread can be added.
+    </p>
+    <p>
+        We replaced <code>setUncaughtExceptionHandler(final 
Thread.UncaughtExceptionHandler uncaughtExceptionHandler)</code>

Review comment:
       "replaced" sounds like as if the the method was remove. Better:
   ```
   We deprecated Foo() in favor of Bar()....
   ```

##########
File path: docs/streams/upgrade-guide.html
##########
@@ -91,6 +91,20 @@ <h3><a id="streams_api_changes_280" 
href="#streams_api_changes_280">Streams API
         We extended <code>StreamJoined</code> to include the options 
<code>withLoggingEnabled()</code> and <code>withLoggingDisabled()</code> in
         <a 
href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-689%3A+Extend+%60StreamJoined%60+to+allow+more+store+configs";>KIP-689</a>.
     </p>
+    <p>
+        We add and option to <code>addThread()</code> and 
<code>removeThread()</code> in
+        <a href="https://cwiki.apache.org/confluence/x/FDd4CQ";>KIP-663</a>.
+        This also caused the KafkaStreams client state machine to change.
+        The ERROR state is now terminal with PENDING_ERROR being a 
transitional state where the resources are closing.
+        When all thread are dead there is not automatic transition to ERROR as 
a new thread can be added.
+    </p>
+    <p>
+        We replaced <code>setUncaughtExceptionHandler(final 
Thread.UncaughtExceptionHandler uncaughtExceptionHandler)</code>
+        with <code>setUncaughtExceptionHandler(final 
StreamsUncaughtExceptionHandler streamsUncaughtExceptionHandler)</code>.
+        The default handler will close the client into the state ERROR.
+        This new handler will return a 
<code>StreamThreadExceptionResponse</code>,

Review comment:
       `If you implement a custom handler, the new interface allows you to 
return a <code>StreamThreadExceptionResponse</code>,...`

##########
File path: docs/streams/upgrade-guide.html
##########
@@ -91,6 +91,20 @@ <h3><a id="streams_api_changes_280" 
href="#streams_api_changes_280">Streams API
         We extended <code>StreamJoined</code> to include the options 
<code>withLoggingEnabled()</code> and <code>withLoggingDisabled()</code> in
         <a 
href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-689%3A+Extend+%60StreamJoined%60+to+allow+more+store+configs";>KIP-689</a>.
     </p>
+    <p>
+        We add and option to <code>addThread()</code> and 
<code>removeThread()</code> in
+        <a href="https://cwiki.apache.org/confluence/x/FDd4CQ";>KIP-663</a>.
+        This also caused the KafkaStreams client state machine to change.
+        The ERROR state is now terminal with PENDING_ERROR being a 
transitional state where the resources are closing.
+        When all thread are dead there is not automatic transition to ERROR as 
a new thread can be added.
+    </p>
+    <p>
+        We replaced <code>setUncaughtExceptionHandler(final 
Thread.UncaughtExceptionHandler uncaughtExceptionHandler)</code>
+        with <code>setUncaughtExceptionHandler(final 
StreamsUncaughtExceptionHandler streamsUncaughtExceptionHandler)</code>.
+        The default handler will close the client into the state ERROR.
+        This new handler will return a 
<code>StreamThreadExceptionResponse</code>,
+        which will determine how the application will respond to a thread 
failure.
+    </p>

Review comment:
       Please all link to the KIP

##########
File path: docs/streams/upgrade-guide.html
##########
@@ -91,6 +91,20 @@ <h3><a id="streams_api_changes_280" 
href="#streams_api_changes_280">Streams API
         We extended <code>StreamJoined</code> to include the options 
<code>withLoggingEnabled()</code> and <code>withLoggingDisabled()</code> in
         <a 
href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-689%3A+Extend+%60StreamJoined%60+to+allow+more+store+configs";>KIP-689</a>.
     </p>
+    <p>
+        We add and option to <code>addThread()</code> and 
<code>removeThread()</code> in
+        <a href="https://cwiki.apache.org/confluence/x/FDd4CQ";>KIP-663</a>.
+        This also caused the KafkaStreams client state machine to change.
+        The ERROR state is now terminal with PENDING_ERROR being a 
transitional state where the resources are closing.
+        When all thread are dead there is not automatic transition to ERROR as 
a new thread can be added.
+    </p>
+    <p>
+        We replaced <code>setUncaughtExceptionHandler(final 
Thread.UncaughtExceptionHandler uncaughtExceptionHandler)</code>
+        with <code>setUncaughtExceptionHandler(final 
StreamsUncaughtExceptionHandler streamsUncaughtExceptionHandler)</code>.
+        The default handler will close the client into the state ERROR.

Review comment:
       suggestion: `the client into the state ERROR` -> `the client and the 
client will transit to state ERROR.` ?

##########
File path: docs/streams/upgrade-guide.html
##########
@@ -91,6 +91,20 @@ <h3><a id="streams_api_changes_280" 
href="#streams_api_changes_280">Streams API
         We extended <code>StreamJoined</code> to include the options 
<code>withLoggingEnabled()</code> and <code>withLoggingDisabled()</code> in
         <a 
href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-689%3A+Extend+%60StreamJoined%60+to+allow+more+store+configs";>KIP-689</a>.
     </p>
+    <p>
+        We add and option to <code>addThread()</code> and 
<code>removeThread()</code> in

Review comment:
       `We add and option to` ?
   
   -> `We added two new method to `KafkaStreams`, namely 
<code>addThread()</code> and <code>removeThread()</code> ...` 




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to