[DISCUSS] KIP-623: Add "internal-topics" option to streams application reset tool
Hi all I would like to start a vote for KIP-623, which adds the option --internal-topics to the streams-application-reset-tool: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158862177. Please let me know what you think. Best Joel
Re: [DISCUSS] KIP-623: Add "internal-topics" option to streams application reset tool
Hi Bruce Thank you for the comments. I agree with 1 and 3 and have updated the KIP. 2. Yes, the user needs to execute the script twice. Once with `--dry-run` and once without. There are no confirmation prompts when executing without `--dry-run`. I have updated the KIP to make this clearer. It does sound like it would make sense to build the dry-run functionality into the script, but I think that prompting for confirmation will break backwards compatibility – e.g. for anyone who uses the script as part of a fully automated process. I’m not sure if the benefits outweigh the costs here? Best Joel > On 12 Jun 2020, at 4:33 PM, Bruno Cadonna wrote: > > Hi Joel, > > Thank you for the KIP. > > The KIP is well motivated. > > I have a couple comments: > > 1. I would not describe the new option with Java code that you want to > add to the `StreamsResetter` class since this class is not part of the > public API. Only the script kafka-streams-application-reset.sh in /bin > belongs to the public API. I would illustrate the new option similarly > as in the output of kafka-streams-application-reset.sh --help. > > 2. It is not entirely clear to me what you mean by "Do a dry-run to > check ..." and "execute without dry-run". Does the user need to > execute the script twice? Do you get a prompt to confirm the deletion > of the internal topic in the same execution of the script? > > 3. I would remove "No compatibility issues". The sentence afterward > describes how the KIP avoids backward compatibility issues. > > On a different note: > Since it seems that the current behavior can have quite critical > consequences on applications -- namely that users might damage other > applications without a warning -- I would like to propose to change > the script in such a way so that it always makes a dry-run, it asks > the users for confirmation, and then resets the app. > I do not know if this still counts as backward compatible, though. Any > thoughts? > > Best, > Bruno > > On Fri, Jun 12, 2020 at 9:52 AM Joel Wee Hotmail > wrote: >> >> Thanks Boyang, that’s helpful! >> >> Have made the changes and updated the KIP now. >> >> Joel >> >>> On 11 Jun 2020, at 5:38 PM, Boyang Chen wrote: >>> >>> Thanks for the KIP Joel! Some quick questions and suggestions: >>> >>> 1. The KIP links to a wrong JIRA, which should be >>> https://issues.apache.org/jira/browse/KAFKA-6435 >>> 2. Typo: *deletes all topic that *-> *deletes all topics that* >>> 3. We need to explain in the Motivation section that we want to implement >>> an alternative approach for users to just delete internal topics they would >>> like to instead of using app-id-* as the regex >>> 4. In the Proposed Changes section, we need to explain the general usage >>> pattern, by doing a dry-run first and then choose whether to use the >>> `--internal-topics` flag or not. >>> 5. Seems we don't need bullet points for the Compatibility section? >>> >>> Boyang >>> >>> On Wed, Jun 10, 2020 at 2:16 PM Joel Wee wrote: >>> Hi all I would like to start the discussion for KIP-623 which proposes adding an “internal-topics” option to the streams application reset tool: https://cwiki.apache.org/confluence/x/YQt4CQ Thanks Joel >>
Re: [DISCUSS] KIP-623: Add "internal-topics" option to streams application reset tool
Hi Joel, Thank you for the KIP. The KIP is well motivated. I have a couple comments: 1. I would not describe the new option with Java code that you want to add to the `StreamsResetter` class since this class is not part of the public API. Only the script kafka-streams-application-reset.sh in /bin belongs to the public API. I would illustrate the new option similarly as in the output of kafka-streams-application-reset.sh --help. 2. It is not entirely clear to me what you mean by "Do a dry-run to check ..." and "execute without dry-run". Does the user need to execute the script twice? Do you get a prompt to confirm the deletion of the internal topic in the same execution of the script? 3. I would remove "No compatibility issues". The sentence afterward describes how the KIP avoids backward compatibility issues. On a different note: Since it seems that the current behavior can have quite critical consequences on applications -- namely that users might damage other applications without a warning -- I would like to propose to change the script in such a way so that it always makes a dry-run, it asks the users for confirmation, and then resets the app. I do not know if this still counts as backward compatible, though. Any thoughts? Best, Bruno On Fri, Jun 12, 2020 at 9:52 AM Joel Wee Hotmail wrote: > > Thanks Boyang, that’s helpful! > > Have made the changes and updated the KIP now. > > Joel > > > On 11 Jun 2020, at 5:38 PM, Boyang Chen wrote: > > > > Thanks for the KIP Joel! Some quick questions and suggestions: > > > > 1. The KIP links to a wrong JIRA, which should be > > https://issues.apache.org/jira/browse/KAFKA-6435 > > 2. Typo: *deletes all topic that *-> *deletes all topics that* > > 3. We need to explain in the Motivation section that we want to implement > > an alternative approach for users to just delete internal topics they would > > like to instead of using app-id-* as the regex > > 4. In the Proposed Changes section, we need to explain the general usage > > pattern, by doing a dry-run first and then choose whether to use the > > `--internal-topics` flag or not. > > 5. Seems we don't need bullet points for the Compatibility section? > > > > Boyang > > > > On Wed, Jun 10, 2020 at 2:16 PM Joel Wee wrote: > > > >> Hi all > >> > >> I would like to start the discussion for KIP-623 which proposes adding an > >> “internal-topics” option to the streams application reset tool: > >> > >> https://cwiki.apache.org/confluence/x/YQt4CQ > >> > >> Thanks > >> > >> Joel > >> >
Re: [DISCUSS] KIP-623: Add "internal-topics" option to streams application reset tool
Thanks Boyang, that’s helpful! Have made the changes and updated the KIP now. Joel > On 11 Jun 2020, at 5:38 PM, Boyang Chen wrote: > > Thanks for the KIP Joel! Some quick questions and suggestions: > > 1. The KIP links to a wrong JIRA, which should be > https://issues.apache.org/jira/browse/KAFKA-6435 > 2. Typo: *deletes all topic that *-> *deletes all topics that* > 3. We need to explain in the Motivation section that we want to implement > an alternative approach for users to just delete internal topics they would > like to instead of using app-id-* as the regex > 4. In the Proposed Changes section, we need to explain the general usage > pattern, by doing a dry-run first and then choose whether to use the > `--internal-topics` flag or not. > 5. Seems we don't need bullet points for the Compatibility section? > > Boyang > > On Wed, Jun 10, 2020 at 2:16 PM Joel Wee wrote: > >> Hi all >> >> I would like to start the discussion for KIP-623 which proposes adding an >> “internal-topics” option to the streams application reset tool: >> >> https://cwiki.apache.org/confluence/x/YQt4CQ >> >> Thanks >> >> Joel >>
Re: [DISCUSS] KIP-623: Add "internal-topics" option to streams application reset tool
Thanks for the KIP Joel! Some quick questions and suggestions: 1. The KIP links to a wrong JIRA, which should be https://issues.apache.org/jira/browse/KAFKA-6435 2. Typo: *deletes all topic that *-> *deletes all topics that* 3. We need to explain in the Motivation section that we want to implement an alternative approach for users to just delete internal topics they would like to instead of using app-id-* as the regex 4. In the Proposed Changes section, we need to explain the general usage pattern, by doing a dry-run first and then choose whether to use the `--internal-topics` flag or not. 5. Seems we don't need bullet points for the Compatibility section? Boyang On Wed, Jun 10, 2020 at 2:16 PM Joel Wee wrote: > Hi all > > I would like to start the discussion for KIP-623 which proposes adding an > “internal-topics” option to the streams application reset tool: > > https://cwiki.apache.org/confluence/x/YQt4CQ > > Thanks > > Joel >
[DISCUSS] KIP-623: Add "internal-topics" option to streams application reset tool
Hi all I would like to start the discussion for KIP-623 which proposes adding an “internal-topics” option to the streams application reset tool: https://cwiki.apache.org/confluence/x/YQt4CQ Thanks Joel
