You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Boyang Chen <re...@gmail.com> on 2020/07/01 02:04:35 UTC
Re: [VOTE] KIP-623: Add "internal-topics" option to streams
application reset tool
Hey Bruno,
I agree adding a prompt would be a nice precaution, but it is not backward
compatible as you suggested and could make the automation harder to
achieve.
If you want, we may consider starting a separate ticket to discuss whether
adding a prompt to let users be aware of the topics that are about to
delete. However, this is also inverting the assumptions we made about
`--dry-run` mode, which would become useless to me once we are adding a
prompt asking users whether they want to remove these topics completely, or
do nothing at all.
Back to KIP-623, I think this discussion could be held in orthogonal, which
applies to more general considerations of reducing human errors, etc.
Boyang
On Tue, Jun 30, 2020 at 12:55 AM Bruno Cadonna <br...@confluent.io> wrote:
> Hi,
>
> I have already brought this up in the discussion thread.
>
> Should we not run a dry-run in any case to avoid inadvertently
> deleting topics of other applications?
>
> I know it is a backward incompatible change if users use it in
> scripts, but I think it is still worth discussing it. I would to hear
> what committers think about it.
>
> Best,
> Bruno
>
> On Tue, Jun 30, 2020 at 5:33 AM Boyang Chen <re...@gmail.com>
> wrote:
> >
> > Thanks John for the great suggestion. +1 for enforcing the prefix check
> for
> > the `--internal-topics` list.
> >
> > On Mon, Jun 29, 2020 at 5:11 PM John Roesler <vv...@apache.org>
> wrote:
> >
> > > Oh, I meant to say, if that’s the case, then I’m +1 (binding) also :)
> > >
> > > Thanks again,
> > > John
> > >
> > > On Mon, Jun 29, 2020, at 19:09, John Roesler wrote:
> > > > Thanks for the KIP, Joel!
> > > >
> > > > It seems like a good pattern to document would be to first run with
> > > > —dry-run and without —internal-topics to list all potential topics,
> and
> > > > then to use —internal-topics if needed to limit the internal topics
> to
> > > > delete.
> > > >
> > > > Just to make sure, would we have a sanity check to forbid including
> > > > arbitrary topics? I.e., it seems like —internal-topics should require
> > > > all the topics to be prefixed with the app id. Is that right?
> > > >
> > > > Thanks,
> > > > John
> > > >
> > > > On Mon, Jun 29, 2020, at 18:25, Guozhang Wang wrote:
> > > > > Thanks Joel, the KIP lgtm.
> > > > >
> > > > > A minor suggestion is to explain where users can get the list of
> > > internal
> > > > > topics of a given application, and maybe also add it as part of the
> > > helper
> > > > > scripts, for example via topology description.
> > > > >
> > > > > Overall, I'm +1 as well (binding).
> > > > >
> > > > > Guozhang
> > > > >
> > > > >
> > > > > On Sat, Jun 27, 2020 at 4:33 AM Joel Wee <Jo...@outlook.com>
> wrote:
> > > > >
> > > > > > Thanks Boyang, I think what you’ve said makes sense. I’ve made
> the
> > > > > > motivation clearer now:
> > > > > >
> > > > > >
> > > > > > Users may want to specify which internal topics should be
> deleted. At
> > > > > > present, the streams reset tool deletes all topics that start
> with "<
> > > > > > application.id<http://application.id>>-" and there are no
> options to
> > > > > > control it.
> > > > > >
> > > > > > The `--internal-topics` option is especially useful when there
> are
> > > prefix
> > > > > > conflicts between applications, e.g. "app" and "app-v2". In this
> > > case, if
> > > > > > we want to reset "app", the reset tool’s default behaviour will
> > > delete both
> > > > > > the internal topics of "app" and "app-v2" (since both are
> prefixed by
> > > > > > "app-"). With the `--internal-topics` option, we can provide
> > > internal topic
> > > > > > names for "app" and delete the internal topics for "app" without
> > > deleting
> > > > > > the internal topics for "app-v2".
> > > > > >
> > > > > > Best
> > > > > >
> > > > > > Joel
> > > > > >
> > > > > > On 27 Jun 2020, at 2:07 AM, Boyang Chen <
> reluctanthero104@gmail.com
> > > > > > <ma...@gmail.com>> wrote:
> > > > > >
> > > > > > Thanks for driving the proposal Joel, I have a minor
> suggestion: we
> > > should
> > > > > > be more clear about why we introduce this flag, so it would be
> > > better to
> > > > > > also state clearly in the document for the default behavior as
> well,
> > > such
> > > > > > like:
> > > > > >
> > > > > > Comma-separated list of internal topics to be deleted. By
> default,
> > > > > > Streams reset tool will delete all topics prefixed by the
> > > > > > application.id<http://application.id>.
> > > > > >
> > > > > > This flag is useful when you need to keep certain topics intact
> due
> > > to
> > > > > > the prefix conflict with another application (such like "app" vs
> > > > > > "app-v2").
> > > > > >
> > > > > > With provided internal topic names for "app", the reset tool will
> > > only
> > > > > > delete internal topics associated with "app", instead of both
> "app"
> > > > > > and "app-v2".
> > > > > >
> > > > > >
> > > > > > Other than that, +1 from me (binding).
> > > > > >
> > > > > > On Wed, Jun 24, 2020 at 1:19 PM Joel Wee <Joel.Wee@outlook.com
> > > <mailto:
> > > > > > Joel.Wee@outlook.com>> wrote:
> > > > > >
> > > > > > Apologies. Changing the subject.
> > > > > >
> > > > > > On 24 Jun 2020, at 9:14 PM, Joel Wee <Joel.Wee@outlook.com
> <mailto:
> > > > > > Joel.Wee@outlook.com><mailto:
> > > > > > Joel.Wee@outlook.com<ma...@outlook.com>>> wrote:
> > > > > >
> > > > > > 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
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > >
>
Re: [VOTE] KIP-623: Add "internal-topics" option to streams
application reset tool
Posted by Boyang Chen <re...@gmail.com>.
Sounds good, thank you! Moreover, I do feel it would be great to integrate
some of the motivation stuff into the option description, but we could
discuss more in the PR.
On Sat, Jul 18, 2020 at 3:24 AM Joel Wee <Jo...@outlook.com> wrote:
> I agree Boyang, we should be able to specify both `—internal-topics` and
> `—dry-run` at the same time and this should display the topics that will be
> deleted. What I was trying to communicate in the description is that if you
> want to see all the topics that are valid as input into the option, then
> first do a dry-run without the `—internal-topics` option. I’ve rephrased it
> slightly which hopefully makes it clearer.
>
> --internal-topics <String: list> Comma-separated list of internal
> topics
> to delete. Must be a subset of the
> internal topics marked for deletion
> by
> the default behaviour (do a dry-run
> without this
> option to view these topics).
>
>
> - Joel
>
> On 11 Jul 2020, at 8:41 AM, Boyang Chen <reluctanthero104@gmail.com
> <ma...@gmail.com>> wrote:
>
> Thanks for the update!
>
> On Fri, Jul 3, 2020 at 3:18 PM Joel Wee <Joel.Wee@outlook.com<mailto:
> Joel.Wee@outlook.com>> wrote:
>
> Thanks all for voting!
>
> I am closing the vote as accepted with three binding +1 votes (Boyang,
> Guozhang, John).
>
> Thanks John for the suggestion. I think that makes sense. I have updated
> the KIP to say that only topics flagged as internal by the tool can be
> deleted, and have also rephrased the option description to make this
> clearer:
>
> --internal-topics <String: list> Comma-separated list of internal
> topics
> to delete. Must be a subset of
> the
> internal topics marked for
> deletion by
> the default behaviour. To view
> these
> topics, do a dry-run without
> this
> option.
>
> Hmm, could you explain why we couldn't run with both --internal-topics and
> --dry-run? To me we could either display the exact set of internal topics
> specified to be deleted, or reject the same way as we are doing an actual
> reset.
>
>
> Thanks Guozhang for the suggestion. The updated option description should
> address your first point. By the “topology description” script are you
> referring to bin/kafka-topics.sh? It currently has the option to display
> all topics (including internal topics). Were you thinking about adding
> something to view just internal topics?
>
> Thanks Bruno for the suggestion. I will close this vote for now, and we
> can continue the discussion on another thread. (P.S. apologies for
> misspelling your name previously)
>
> - Joel
>
> On 1 Jul 2020, at 3:04 AM, Boyang Chen <reluctanthero104@gmail.com<mailto:
> reluctanthero104@gmail.com>>
> wrote:
>
> Hey Bruno,
>
> I agree adding a prompt would be a nice precaution, but it is not
> backward
> compatible as you suggested and could make the automation harder to
> achieve.
>
> If you want, we may consider starting a separate ticket to discuss
> whether
> adding a prompt to let users be aware of the topics that are about to
> delete. However, this is also inverting the assumptions we made about
> `--dry-run` mode, which would become useless to me once we are adding a
> prompt asking users whether they want to remove these topics completely,
> or
> do nothing at all.
>
> Back to KIP-623, I think this discussion could be held in orthogonal,
> which
> applies to more general considerations of reducing human errors, etc.
>
> Boyang
>
> On Tue, Jun 30, 2020 at 12:55 AM Bruno Cadonna <bruno@confluent.io<mailto:
> bruno@confluent.io>>
> wrote:
>
> Hi,
>
> I have already brought this up in the discussion thread.
>
> Should we not run a dry-run in any case to avoid inadvertently
> deleting topics of other applications?
>
> I know it is a backward incompatible change if users use it in
> scripts, but I think it is still worth discussing it. I would to hear
> what committers think about it.
>
> Best,
> Bruno
>
> On Tue, Jun 30, 2020 at 5:33 AM Boyang Chen <reluctanthero104@gmail.com
> <ma...@gmail.com>
>
> wrote:
>
> Thanks John for the great suggestion. +1 for enforcing the prefix check
> for
> the `--internal-topics` list.
>
> On Mon, Jun 29, 2020 at 5:11 PM John Roesler <vvcephei@apache.org<mailto:
> vvcephei@apache.org>>
> wrote:
>
> Oh, I meant to say, if that’s the case, then I’m +1 (binding) also :)
>
> Thanks again,
> John
>
> On Mon, Jun 29, 2020, at 19:09, John Roesler wrote:
> Thanks for the KIP, Joel!
>
> It seems like a good pattern to document would be to first run with
> —dry-run and without —internal-topics to list all potential topics,
> and
> then to use —internal-topics if needed to limit the internal topics
> to
> delete.
>
> Just to make sure, would we have a sanity check to forbid including
> arbitrary topics? I.e., it seems like —internal-topics should require
> all the topics to be prefixed with the app id. Is that right?
>
> Thanks,
> John
>
> On Mon, Jun 29, 2020, at 18:25, Guozhang Wang wrote:
> Thanks Joel, the KIP lgtm.
>
> A minor suggestion is to explain where users can get the list of
> internal
> topics of a given application, and maybe also add it as part of the
> helper
> scripts, for example via topology description.
>
> Overall, I'm +1 as well (binding).
>
> Guozhang
>
>
> On Sat, Jun 27, 2020 at 4:33 AM Joel Wee <Joel.Wee@outlook.com<mailto:
> Joel.Wee@outlook.com>>
> wrote:
>
> Thanks Boyang, I think what you’ve said makes sense. I’ve made
> the
> motivation clearer now:
>
>
> Users may want to specify which internal topics should be
> deleted. At
> present, the streams reset tool deletes all topics that start
> with "<
> application.id<http://application.id><http://application.id>>-" and there
> are no
> options to
> control it.
>
> The `--internal-topics` option is especially useful when there
> are
> prefix
> conflicts between applications, e.g. "app" and "app-v2". In this
> case, if
> we want to reset "app", the reset tool’s default behaviour will
> delete both
> the internal topics of "app" and "app-v2" (since both are
> prefixed by
> "app-"). With the `--internal-topics` option, we can provide
> internal topic
> names for "app" and delete the internal topics for "app" without
> deleting
> the internal topics for "app-v2".
>
> Best
>
> Joel
>
> On 27 Jun 2020, at 2:07 AM, Boyang Chen <
> reluctanthero104@gmail.com<ma...@gmail.com>
> <ma...@gmail.com>> wrote:
>
> Thanks for driving the proposal Joel, I have a minor
> suggestion: we
> should
> be more clear about why we introduce this flag, so it would be
> better to
> also state clearly in the document for the default behavior as
> well,
> such
> like:
>
> Comma-separated list of internal topics to be deleted. By
> default,
> Streams reset tool will delete all topics prefixed by the
> application.id<http://application.id>.
>
> This flag is useful when you need to keep certain topics intact
> due
> to
> the prefix conflict with another application (such like "app" vs
> "app-v2").
>
> With provided internal topic names for "app", the reset tool will
> only
> delete internal topics associated with "app", instead of both
> "app"
> and "app-v2".
>
>
> Other than that, +1 from me (binding).
>
> On Wed, Jun 24, 2020 at 1:19 PM Joel Wee <Joel.Wee@outlook.com
> <mailto:
> Joel.Wee@outlook.com>> wrote:
>
> Apologies. Changing the subject.
>
> On 24 Jun 2020, at 9:14 PM, Joel Wee <Joel.Wee@outlook.com
> <mailto:
> Joel.Wee@outlook.com><mailto:
> Joel.Wee@outlook.com<ma...@outlook.com>>> wrote:
>
> 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
>
>
>
>
>
> --
> -- Guozhang
>
>
>
>
>
>
>
>
Re: [VOTE] KIP-623: Add "internal-topics" option to streams
application reset tool
Posted by Joel Wee <Jo...@outlook.com>.
I agree Boyang, we should be able to specify both `—internal-topics` and `—dry-run` at the same time and this should display the topics that will be deleted. What I was trying to communicate in the description is that if you want to see all the topics that are valid as input into the option, then first do a dry-run without the `—internal-topics` option. I’ve rephrased it slightly which hopefully makes it clearer.
--internal-topics <String: list> Comma-separated list of internal topics
to delete. Must be a subset of the
internal topics marked for deletion by
the default behaviour (do a dry-run without this
option to view these topics).
- Joel
On 11 Jul 2020, at 8:41 AM, Boyang Chen <re...@gmail.com>> wrote:
Thanks for the update!
On Fri, Jul 3, 2020 at 3:18 PM Joel Wee <Jo...@outlook.com>> wrote:
Thanks all for voting!
I am closing the vote as accepted with three binding +1 votes (Boyang,
Guozhang, John).
Thanks John for the suggestion. I think that makes sense. I have updated
the KIP to say that only topics flagged as internal by the tool can be
deleted, and have also rephrased the option description to make this
clearer:
--internal-topics <String: list> Comma-separated list of internal
topics
to delete. Must be a subset of
the
internal topics marked for
deletion by
the default behaviour. To view
these
topics, do a dry-run without
this
option.
Hmm, could you explain why we couldn't run with both --internal-topics and
--dry-run? To me we could either display the exact set of internal topics
specified to be deleted, or reject the same way as we are doing an actual
reset.
Thanks Guozhang for the suggestion. The updated option description should
address your first point. By the “topology description” script are you
referring to bin/kafka-topics.sh? It currently has the option to display
all topics (including internal topics). Were you thinking about adding
something to view just internal topics?
Thanks Bruno for the suggestion. I will close this vote for now, and we
can continue the discussion on another thread. (P.S. apologies for
misspelling your name previously)
- Joel
On 1 Jul 2020, at 3:04 AM, Boyang Chen <re...@gmail.com>>
wrote:
Hey Bruno,
I agree adding a prompt would be a nice precaution, but it is not
backward
compatible as you suggested and could make the automation harder to
achieve.
If you want, we may consider starting a separate ticket to discuss
whether
adding a prompt to let users be aware of the topics that are about to
delete. However, this is also inverting the assumptions we made about
`--dry-run` mode, which would become useless to me once we are adding a
prompt asking users whether they want to remove these topics completely,
or
do nothing at all.
Back to KIP-623, I think this discussion could be held in orthogonal,
which
applies to more general considerations of reducing human errors, etc.
Boyang
On Tue, Jun 30, 2020 at 12:55 AM Bruno Cadonna <br...@confluent.io>>
wrote:
Hi,
I have already brought this up in the discussion thread.
Should we not run a dry-run in any case to avoid inadvertently
deleting topics of other applications?
I know it is a backward incompatible change if users use it in
scripts, but I think it is still worth discussing it. I would to hear
what committers think about it.
Best,
Bruno
On Tue, Jun 30, 2020 at 5:33 AM Boyang Chen <re...@gmail.com>
wrote:
Thanks John for the great suggestion. +1 for enforcing the prefix check
for
the `--internal-topics` list.
On Mon, Jun 29, 2020 at 5:11 PM John Roesler <vv...@apache.org>>
wrote:
Oh, I meant to say, if that’s the case, then I’m +1 (binding) also :)
Thanks again,
John
On Mon, Jun 29, 2020, at 19:09, John Roesler wrote:
Thanks for the KIP, Joel!
It seems like a good pattern to document would be to first run with
—dry-run and without —internal-topics to list all potential topics,
and
then to use —internal-topics if needed to limit the internal topics
to
delete.
Just to make sure, would we have a sanity check to forbid including
arbitrary topics? I.e., it seems like —internal-topics should require
all the topics to be prefixed with the app id. Is that right?
Thanks,
John
On Mon, Jun 29, 2020, at 18:25, Guozhang Wang wrote:
Thanks Joel, the KIP lgtm.
A minor suggestion is to explain where users can get the list of
internal
topics of a given application, and maybe also add it as part of the
helper
scripts, for example via topology description.
Overall, I'm +1 as well (binding).
Guozhang
On Sat, Jun 27, 2020 at 4:33 AM Joel Wee <Jo...@outlook.com>>
wrote:
Thanks Boyang, I think what you’ve said makes sense. I’ve made
the
motivation clearer now:
Users may want to specify which internal topics should be
deleted. At
present, the streams reset tool deletes all topics that start
with "<
application.id<http://application.id><http://application.id>>-" and there are no
options to
control it.
The `--internal-topics` option is especially useful when there
are
prefix
conflicts between applications, e.g. "app" and "app-v2". In this
case, if
we want to reset "app", the reset tool’s default behaviour will
delete both
the internal topics of "app" and "app-v2" (since both are
prefixed by
"app-"). With the `--internal-topics` option, we can provide
internal topic
names for "app" and delete the internal topics for "app" without
deleting
the internal topics for "app-v2".
Best
Joel
On 27 Jun 2020, at 2:07 AM, Boyang Chen <
reluctanthero104@gmail.com<ma...@gmail.com>
<ma...@gmail.com>> wrote:
Thanks for driving the proposal Joel, I have a minor
suggestion: we
should
be more clear about why we introduce this flag, so it would be
better to
also state clearly in the document for the default behavior as
well,
such
like:
Comma-separated list of internal topics to be deleted. By
default,
Streams reset tool will delete all topics prefixed by the
application.id<http://application.id>.
This flag is useful when you need to keep certain topics intact
due
to
the prefix conflict with another application (such like "app" vs
"app-v2").
With provided internal topic names for "app", the reset tool will
only
delete internal topics associated with "app", instead of both
"app"
and "app-v2".
Other than that, +1 from me (binding).
On Wed, Jun 24, 2020 at 1:19 PM Joel Wee <Joel.Wee@outlook.com
<mailto:
Joel.Wee@outlook.com>> wrote:
Apologies. Changing the subject.
On 24 Jun 2020, at 9:14 PM, Joel Wee <Joel.Wee@outlook.com
<mailto:
Joel.Wee@outlook.com><mailto:
Joel.Wee@outlook.com<ma...@outlook.com>>> wrote:
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
--
-- Guozhang
Re: [VOTE] KIP-623: Add "internal-topics" option to streams
application reset tool
Posted by Boyang Chen <re...@gmail.com>.
Thanks for the update!
On Fri, Jul 3, 2020 at 3:18 PM Joel Wee <Jo...@outlook.com> wrote:
> Thanks all for voting!
>
> I am closing the vote as accepted with three binding +1 votes (Boyang,
> Guozhang, John).
>
> Thanks John for the suggestion. I think that makes sense. I have updated
> the KIP to say that only topics flagged as internal by the tool can be
> deleted, and have also rephrased the option description to make this
> clearer:
>
> > --internal-topics <String: list> Comma-separated list of internal
> topics
> > to delete. Must be a subset of
> the
> > internal topics marked for
> deletion by
> > the default behaviour. To view
> these
> > topics, do a dry-run without
> this
> > option.
>
> Hmm, could you explain why we couldn't run with both --internal-topics and
--dry-run? To me we could either display the exact set of internal topics
specified to be deleted, or reject the same way as we are doing an actual
reset.
>
> Thanks Guozhang for the suggestion. The updated option description should
> address your first point. By the “topology description” script are you
> referring to bin/kafka-topics.sh? It currently has the option to display
> all topics (including internal topics). Were you thinking about adding
> something to view just internal topics?
>
> Thanks Bruno for the suggestion. I will close this vote for now, and we
> can continue the discussion on another thread. (P.S. apologies for
> misspelling your name previously)
>
> - Joel
>
> > On 1 Jul 2020, at 3:04 AM, Boyang Chen <re...@gmail.com>
> wrote:
> >
> > Hey Bruno,
> >
> > I agree adding a prompt would be a nice precaution, but it is not
> backward
> > compatible as you suggested and could make the automation harder to
> > achieve.
> >
> > If you want, we may consider starting a separate ticket to discuss
> whether
> > adding a prompt to let users be aware of the topics that are about to
> > delete. However, this is also inverting the assumptions we made about
> > `--dry-run` mode, which would become useless to me once we are adding a
> > prompt asking users whether they want to remove these topics completely,
> or
> > do nothing at all.
> >
> > Back to KIP-623, I think this discussion could be held in orthogonal,
> which
> > applies to more general considerations of reducing human errors, etc.
> >
> > Boyang
> >
> > On Tue, Jun 30, 2020 at 12:55 AM Bruno Cadonna <br...@confluent.io>
> wrote:
> >
> >> Hi,
> >>
> >> I have already brought this up in the discussion thread.
> >>
> >> Should we not run a dry-run in any case to avoid inadvertently
> >> deleting topics of other applications?
> >>
> >> I know it is a backward incompatible change if users use it in
> >> scripts, but I think it is still worth discussing it. I would to hear
> >> what committers think about it.
> >>
> >> Best,
> >> Bruno
> >>
> >> On Tue, Jun 30, 2020 at 5:33 AM Boyang Chen <reluctanthero104@gmail.com
> >
> >> wrote:
> >>>
> >>> Thanks John for the great suggestion. +1 for enforcing the prefix check
> >> for
> >>> the `--internal-topics` list.
> >>>
> >>> On Mon, Jun 29, 2020 at 5:11 PM John Roesler <vv...@apache.org>
> >> wrote:
> >>>
> >>>> Oh, I meant to say, if that’s the case, then I’m +1 (binding) also :)
> >>>>
> >>>> Thanks again,
> >>>> John
> >>>>
> >>>> On Mon, Jun 29, 2020, at 19:09, John Roesler wrote:
> >>>>> Thanks for the KIP, Joel!
> >>>>>
> >>>>> It seems like a good pattern to document would be to first run with
> >>>>> —dry-run and without —internal-topics to list all potential topics,
> >> and
> >>>>> then to use —internal-topics if needed to limit the internal topics
> >> to
> >>>>> delete.
> >>>>>
> >>>>> Just to make sure, would we have a sanity check to forbid including
> >>>>> arbitrary topics? I.e., it seems like —internal-topics should require
> >>>>> all the topics to be prefixed with the app id. Is that right?
> >>>>>
> >>>>> Thanks,
> >>>>> John
> >>>>>
> >>>>> On Mon, Jun 29, 2020, at 18:25, Guozhang Wang wrote:
> >>>>>> Thanks Joel, the KIP lgtm.
> >>>>>>
> >>>>>> A minor suggestion is to explain where users can get the list of
> >>>> internal
> >>>>>> topics of a given application, and maybe also add it as part of the
> >>>> helper
> >>>>>> scripts, for example via topology description.
> >>>>>>
> >>>>>> Overall, I'm +1 as well (binding).
> >>>>>>
> >>>>>> Guozhang
> >>>>>>
> >>>>>>
> >>>>>> On Sat, Jun 27, 2020 at 4:33 AM Joel Wee <Jo...@outlook.com>
> >> wrote:
> >>>>>>
> >>>>>>> Thanks Boyang, I think what you’ve said makes sense. I’ve made
> >> the
> >>>>>>> motivation clearer now:
> >>>>>>>
> >>>>>>>
> >>>>>>> Users may want to specify which internal topics should be
> >> deleted. At
> >>>>>>> present, the streams reset tool deletes all topics that start
> >> with "<
> >>>>>>> application.id<http://application.id>>-" and there are no
> >> options to
> >>>>>>> control it.
> >>>>>>>
> >>>>>>> The `--internal-topics` option is especially useful when there
> >> are
> >>>> prefix
> >>>>>>> conflicts between applications, e.g. "app" and "app-v2". In this
> >>>> case, if
> >>>>>>> we want to reset "app", the reset tool’s default behaviour will
> >>>> delete both
> >>>>>>> the internal topics of "app" and "app-v2" (since both are
> >> prefixed by
> >>>>>>> "app-"). With the `--internal-topics` option, we can provide
> >>>> internal topic
> >>>>>>> names for "app" and delete the internal topics for "app" without
> >>>> deleting
> >>>>>>> the internal topics for "app-v2".
> >>>>>>>
> >>>>>>> Best
> >>>>>>>
> >>>>>>> Joel
> >>>>>>>
> >>>>>>> On 27 Jun 2020, at 2:07 AM, Boyang Chen <
> >> reluctanthero104@gmail.com
> >>>>>>> <ma...@gmail.com>> wrote:
> >>>>>>>
> >>>>>>> Thanks for driving the proposal Joel, I have a minor
> >> suggestion: we
> >>>> should
> >>>>>>> be more clear about why we introduce this flag, so it would be
> >>>> better to
> >>>>>>> also state clearly in the document for the default behavior as
> >> well,
> >>>> such
> >>>>>>> like:
> >>>>>>>
> >>>>>>> Comma-separated list of internal topics to be deleted. By
> >> default,
> >>>>>>> Streams reset tool will delete all topics prefixed by the
> >>>>>>> application.id<http://application.id>.
> >>>>>>>
> >>>>>>> This flag is useful when you need to keep certain topics intact
> >> due
> >>>> to
> >>>>>>> the prefix conflict with another application (such like "app" vs
> >>>>>>> "app-v2").
> >>>>>>>
> >>>>>>> With provided internal topic names for "app", the reset tool will
> >>>> only
> >>>>>>> delete internal topics associated with "app", instead of both
> >> "app"
> >>>>>>> and "app-v2".
> >>>>>>>
> >>>>>>>
> >>>>>>> Other than that, +1 from me (binding).
> >>>>>>>
> >>>>>>> On Wed, Jun 24, 2020 at 1:19 PM Joel Wee <Joel.Wee@outlook.com
> >>>> <mailto:
> >>>>>>> Joel.Wee@outlook.com>> wrote:
> >>>>>>>
> >>>>>>> Apologies. Changing the subject.
> >>>>>>>
> >>>>>>> On 24 Jun 2020, at 9:14 PM, Joel Wee <Joel.Wee@outlook.com
> >> <mailto:
> >>>>>>> Joel.Wee@outlook.com><mailto:
> >>>>>>> Joel.Wee@outlook.com<ma...@outlook.com>>> wrote:
> >>>>>>>
> >>>>>>> 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
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> -- Guozhang
> >>>>>>
> >>>>>
> >>>>
> >>
>
>
Re: [VOTE] KIP-623: Add "internal-topics" option to streams
application reset tool
Posted by Joel Wee <Jo...@outlook.com>.
Thanks all for voting!
I am closing the vote as accepted with three binding +1 votes (Boyang, Guozhang, John).
Thanks John for the suggestion. I think that makes sense. I have updated the KIP to say that only topics flagged as internal by the tool can be deleted, and have also rephrased the option description to make this clearer:
> --internal-topics <String: list> Comma-separated list of internal topics
> to delete. Must be a subset of the
> internal topics marked for deletion by
> the default behaviour. To view these
> topics, do a dry-run without this
> option.
Thanks Guozhang for the suggestion. The updated option description should address your first point. By the “topology description” script are you referring to bin/kafka-topics.sh? It currently has the option to display all topics (including internal topics). Were you thinking about adding something to view just internal topics?
Thanks Bruno for the suggestion. I will close this vote for now, and we can continue the discussion on another thread. (P.S. apologies for misspelling your name previously)
- Joel
> On 1 Jul 2020, at 3:04 AM, Boyang Chen <re...@gmail.com> wrote:
>
> Hey Bruno,
>
> I agree adding a prompt would be a nice precaution, but it is not backward
> compatible as you suggested and could make the automation harder to
> achieve.
>
> If you want, we may consider starting a separate ticket to discuss whether
> adding a prompt to let users be aware of the topics that are about to
> delete. However, this is also inverting the assumptions we made about
> `--dry-run` mode, which would become useless to me once we are adding a
> prompt asking users whether they want to remove these topics completely, or
> do nothing at all.
>
> Back to KIP-623, I think this discussion could be held in orthogonal, which
> applies to more general considerations of reducing human errors, etc.
>
> Boyang
>
> On Tue, Jun 30, 2020 at 12:55 AM Bruno Cadonna <br...@confluent.io> wrote:
>
>> Hi,
>>
>> I have already brought this up in the discussion thread.
>>
>> Should we not run a dry-run in any case to avoid inadvertently
>> deleting topics of other applications?
>>
>> I know it is a backward incompatible change if users use it in
>> scripts, but I think it is still worth discussing it. I would to hear
>> what committers think about it.
>>
>> Best,
>> Bruno
>>
>> On Tue, Jun 30, 2020 at 5:33 AM Boyang Chen <re...@gmail.com>
>> wrote:
>>>
>>> Thanks John for the great suggestion. +1 for enforcing the prefix check
>> for
>>> the `--internal-topics` list.
>>>
>>> On Mon, Jun 29, 2020 at 5:11 PM John Roesler <vv...@apache.org>
>> wrote:
>>>
>>>> Oh, I meant to say, if that’s the case, then I’m +1 (binding) also :)
>>>>
>>>> Thanks again,
>>>> John
>>>>
>>>> On Mon, Jun 29, 2020, at 19:09, John Roesler wrote:
>>>>> Thanks for the KIP, Joel!
>>>>>
>>>>> It seems like a good pattern to document would be to first run with
>>>>> —dry-run and without —internal-topics to list all potential topics,
>> and
>>>>> then to use —internal-topics if needed to limit the internal topics
>> to
>>>>> delete.
>>>>>
>>>>> Just to make sure, would we have a sanity check to forbid including
>>>>> arbitrary topics? I.e., it seems like —internal-topics should require
>>>>> all the topics to be prefixed with the app id. Is that right?
>>>>>
>>>>> Thanks,
>>>>> John
>>>>>
>>>>> On Mon, Jun 29, 2020, at 18:25, Guozhang Wang wrote:
>>>>>> Thanks Joel, the KIP lgtm.
>>>>>>
>>>>>> A minor suggestion is to explain where users can get the list of
>>>> internal
>>>>>> topics of a given application, and maybe also add it as part of the
>>>> helper
>>>>>> scripts, for example via topology description.
>>>>>>
>>>>>> Overall, I'm +1 as well (binding).
>>>>>>
>>>>>> Guozhang
>>>>>>
>>>>>>
>>>>>> On Sat, Jun 27, 2020 at 4:33 AM Joel Wee <Jo...@outlook.com>
>> wrote:
>>>>>>
>>>>>>> Thanks Boyang, I think what you’ve said makes sense. I’ve made
>> the
>>>>>>> motivation clearer now:
>>>>>>>
>>>>>>>
>>>>>>> Users may want to specify which internal topics should be
>> deleted. At
>>>>>>> present, the streams reset tool deletes all topics that start
>> with "<
>>>>>>> application.id<http://application.id>>-" and there are no
>> options to
>>>>>>> control it.
>>>>>>>
>>>>>>> The `--internal-topics` option is especially useful when there
>> are
>>>> prefix
>>>>>>> conflicts between applications, e.g. "app" and "app-v2". In this
>>>> case, if
>>>>>>> we want to reset "app", the reset tool’s default behaviour will
>>>> delete both
>>>>>>> the internal topics of "app" and "app-v2" (since both are
>> prefixed by
>>>>>>> "app-"). With the `--internal-topics` option, we can provide
>>>> internal topic
>>>>>>> names for "app" and delete the internal topics for "app" without
>>>> deleting
>>>>>>> the internal topics for "app-v2".
>>>>>>>
>>>>>>> Best
>>>>>>>
>>>>>>> Joel
>>>>>>>
>>>>>>> On 27 Jun 2020, at 2:07 AM, Boyang Chen <
>> reluctanthero104@gmail.com
>>>>>>> <ma...@gmail.com>> wrote:
>>>>>>>
>>>>>>> Thanks for driving the proposal Joel, I have a minor
>> suggestion: we
>>>> should
>>>>>>> be more clear about why we introduce this flag, so it would be
>>>> better to
>>>>>>> also state clearly in the document for the default behavior as
>> well,
>>>> such
>>>>>>> like:
>>>>>>>
>>>>>>> Comma-separated list of internal topics to be deleted. By
>> default,
>>>>>>> Streams reset tool will delete all topics prefixed by the
>>>>>>> application.id<http://application.id>.
>>>>>>>
>>>>>>> This flag is useful when you need to keep certain topics intact
>> due
>>>> to
>>>>>>> the prefix conflict with another application (such like "app" vs
>>>>>>> "app-v2").
>>>>>>>
>>>>>>> With provided internal topic names for "app", the reset tool will
>>>> only
>>>>>>> delete internal topics associated with "app", instead of both
>> "app"
>>>>>>> and "app-v2".
>>>>>>>
>>>>>>>
>>>>>>> Other than that, +1 from me (binding).
>>>>>>>
>>>>>>> On Wed, Jun 24, 2020 at 1:19 PM Joel Wee <Joel.Wee@outlook.com
>>>> <mailto:
>>>>>>> Joel.Wee@outlook.com>> wrote:
>>>>>>>
>>>>>>> Apologies. Changing the subject.
>>>>>>>
>>>>>>> On 24 Jun 2020, at 9:14 PM, Joel Wee <Joel.Wee@outlook.com
>> <mailto:
>>>>>>> Joel.Wee@outlook.com><mailto:
>>>>>>> Joel.Wee@outlook.com<ma...@outlook.com>>> wrote:
>>>>>>>
>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> -- Guozhang
>>>>>>
>>>>>
>>>>
>>