mjsax commented on code in PR #16761:
URL: https://github.com/apache/kafka/pull/16761#discussion_r1760366589
##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -687,7 +687,7 @@ public List<String> inputTopicsOption() {
}
public List<String> intermediateTopicsOption() {
- log.warn("intermediateTopicsOption will be removed in version
5.0");
+ log.warn("intermediateTopicsOption is deprecated and will be
removed in version 5.0");
Review Comment:
Btw: Given that this is a command line tool, I think we should not actually
user a `Logger` but `System.out.println(...)` here (we do this everywhere else,
too).
##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -584,8 +584,8 @@ public StreamsResetterOptions(String[] args) {
.ofType(String.class)
.withValuesSeparatedBy(',')
.describedAs("list");
- intermediateTopicsOption = parser.accepts("intermediate-topics",
"Comma-separated list of intermediate user topics (topics that are input and
output topics, "
- + "e.g., used in the deprecated through() method). For
these topics, the tool will skip to the end.")
+ intermediateTopicsOption = parser.accepts("intermediate-topics",
"Comma-separated list of intermediate user topics (topics that are input and
output topics). "
Review Comment:
@ardada2468 Seems you did not address this comment yet?
##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -112,6 +115,7 @@ public class StreamsResetter {
+ "reset tool!\n\n"
+ "*** Warning! This tool makes irreversible changes to your
application. It is strongly recommended that "
+ "you run this once with \"--dry-run\" to preview your changes
before making them.\n\n";
+ private static final Logger log =
LoggerFactory.getLogger(StreamsResetter.class);
Review Comment:
Cf my comment below. We should not use an actual `Logger` -- let's remove
this again.
##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -687,7 +687,7 @@ public List<String> inputTopicsOption() {
}
public List<String> intermediateTopicsOption() {
- log.warn("intermediateTopicsOption will be removed in version
5.0");
+ log.warn("intermediateTopicsOption is deprecated and will be
removed in version 5.0");
Review Comment:
Seems you did not address this comment yet?
##########
docs/streams/developer-guide/app-reset-tool.html:
##########
@@ -59,17 +58,7 @@
<li><p class="first">All instances of your application must be
stopped. Otherwise, the application may enter an invalid state, crash, or
produce incorrect results. You can verify whether the consumer group with ID
<code class="docutils literal"><span class="pre">application.id</span></code>
is still active by using <code class="docutils literal"><span
class="pre">bin/kafka-consumer-groups</span></code>.
When long session timeout has been configured, active
members could take longer to get expired on the broker thus blocking the reset
job to complete. Use the <code class="docutils literal"><span
class="pre">--force</span></code> option could remove those left-over members
immediately. Make sure to shut down all stream applications when this option is
specified to avoid unexpected rebalances.</p>
</li>
- <li><p class="first">Use this tool with care and double-check
its parameters: If you provide wrong parameter values (e.g., typos in <code
class="docutils literal"><span class="pre">application.id</span></code>) or
specify parameters inconsistently (e.g., specify the wrong input topics for the
application), this tool might invalidate the application’s state or even
impact other applications, consumer groups, or your Kafka topics.</p>
- </li>
- <li><p class="first">You should manually delete and re-create
any intermediate topics before running the application reset tool. This will
free up disk space in Kafka brokers.</p>
- </li>
- <li><p class="first">You should delete and recreate
intermediate topics before running the application reset tool, unless the
following applies:</p>
- <blockquote>
- <div><ul class="simple">
- <li>You have external downstream consumers for the
application’s intermediate topics.</li>
- <li>You are in a development environment where
manually deleting and re-creating intermediate topics is unnecessary.</li>
- </ul>
- </div></blockquote>
+ <li><p class="first">Use this tool with care and
double-check its parameters: If you provide wrong parameter values (e.g., typos
in <code class="docutils literal"><span
class="pre">application.id</span></code>) or specify parameters inconsistently
(e.g., specify the wrong input topics for the application), this tool might
invalidate the application’s state or even impact other applications,
consumer groups, or your Kafka topics.</p>
Review Comment:
```suggestion
<li><p class="first">Use this tool with care and
double-check its parameters: If you provide wrong parameter values (e.g., typos
in <code class="docutils literal"><span
class="pre">application.id</span></code>) or specify parameters inconsistently
(e.g., specify the wrong input topics for the application), this tool might
invalidate the application’s state or even impact other applications,
consumer groups, or your Kafka topics.</p>
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
