Hi, Sophie
    For 1) Sorry, I found that my expression is kind of misleading, what I 
actually mean is: "if --force not specified, an exception saying there are 
still active members on broker side will be thrown and suggesting using 
StreamsResetter with --force", I just updated the KIP page.

    For 2)
        I may also had some misleading expression previous, to clarify :

Also, it's more efficient to just send a single "clear the group" request vs 
sending a LeaveGroup
request for every single member. What do you think?
=> the comparison is to send a single "clear the group" request vs sending a 
"get members" + a "remove members" request since the 
adminClient.removeMembersFromConsumerGroup support batch removal. We don't need 
to send lots of leaveGroup requests for every single member.

       I can understand your point, but I think we could reuse the current 
adminClient.removeMembersFromConsumerGroup interface effectively with the KIP. 
        What do you think?
 
    Thanks!

Feyman


------------------------------------------------------------------
发件人:Sophie Blee-Goldman <sop...@confluent.io>
发送时间:2020年3月10日(星期二) 03:02
收件人:dev <dev@kafka.apache.org>; feyman2009 <feyman2...@aliyun.com>
主 题:Re: 回复:回复:[Vote] KIP-571: Add option to force remove members in 
StreamsResetter

Hey Feyman,

1) Regarding point 2 in your last email, if I understand correctly you propose 
to change
the current behavior of the reset tool when --force is not specified, and wait 
for (up to)
the session timeout for all members to be removed. I'm not sure we should 
change this,
especially now that we have a better way to handle the case when the group is 
not empty:
we should continue to throw an exception and fail fast, but can print a message 
suggesting
to use the new --force option to remove remaining group members. Why make users 
wait
for the session timeout when we've just added a new feature that means they 
don't have to?

2) Regarding Matthias' question:

> I am really wondering, if for a static group, we should allow users toremove 
> individual members? For a dynamic group this feature would not
make much sense IMHO, because the `memberId` is not know by the user.

I think his point is similar to what I was trying to get at earlier, with the 
proposal to add a new
#removeAllMembers API rather than an API to remove individual members according 
to their
memberId. As he explained, removing based on memberId is likely not that useful 
in general.
Also, it's not actually what we want to do here; maybe we should avoid adding a 
new API 
that we think may be useful in other contexts (remove individual member based 
on memberId),
and just add the API we actually need (remove all members from group) in this 
KIP? We can
always add the "remove individual member by memberId" API at a later point, if 
it turns out to
actually be requested for specific reasons?

Also, it's more efficient to just send a single "clear the group" request vs 
sending a LeaveGroup
request for every single member. What do you think?




On Sat, Mar 7, 2020 at 1:41 AM feyman2009 <feyman2...@aliyun.com.invalid> wrote:
Hi, Matthias
     Thanks, I updated the KIP to mention the deprecated and newly added 
methods.

 1. What happens is `groupInstanceId` is used for a dynamic group? What
 happens if both parameters are specified? What happens if `memberId`
 is specified for a static group?

 => my understanding is that the dynamic/static membership is member level 
other than group level, and I think above questions could be answered by the 
"Leave Group Logic Change" section in KIP-345: 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-345%3A+Introduce+static+membership+protocol+to+reduce+consumer+rebalances,
 this KIP stays consistent with KIP-345.

 2. About the `--force` option. Currently, StreamsResetter fails with an
 error if the consumer group is not empty. You state in your KIP that:

 > without --force, we need to wait for session timeout.

 Is this an intended behavior change if `--force` is not used or is the
 KIP description incorrect?

 => This is the intended behavior. For this part, I think there are two ways to 
go:
 1) (the implicit way) Not introducing the new "--force" option, with this KIP, 
StreamsResetter will by default remove active members(with long session timeout 
configured) on broker side 
 2) (the explicit way) Introduce the new "--force" option, users need to 
explicitly specify --force to remove the active members. If --force not 
specified, the StreamsResetter behaviour is as previous versions'.

 I think the two alternatives above are both feasible, personally I prefer way 
2.

 3. For my own understanding: with the `--force` option, we intend to get
 all `memberIds` and send a "remove member" request for each with
 corresponding `memberId` to remove the member from the group
 (independent is the group is static or dynamic)?

 => Yeah, minor thing to mention is we will send the "remove member" request 
for each member(could be dynamic member or static member) to remove them from 
group
 for dynamic members, both "group.instance.id" and "member.id" will be specified
 for dynamic members, only "member.id" will be specified

 4. I am really wondering, if for a static group, we should allow users to
 remove individual members? For a dynamic group this feature would not
 make much sense IMHO, because the `memberId` is not know by the user.

 => KIP-345 introduced the batch removal feature for both static member and 
dynamic member, my understanding is that "allow users to
 remove individual members" could be useful for rolling bounce and scale down 
accoding to KIP-345. KafkaAdminClient currently only support static member 
removal and this KIP-571 enables dynamic member removal for it, which is also 
consistent with the broker side logic. Users could get the member.id (and 
group.instance.id if for static member) by adminClient.describeConsumerGroups.

 Furthermore, I don't have an assumption that a consumer group should contain 
only static or dynamic members, and I think KIP-345 and this KIP don't need to 
be based on this assumption.
 You could correct me if I have the wrong understanding :)

 Thanks!
 Feyman







 ------------------------------------------------------------------
 发件人:Matthias J. Sax <mj...@apache.org>
 发送时间:2020年3月6日(星期五) 02:20
 收件人:dev <dev@kafka.apache.org>
 主 题:Re: 回复:回复:[Vote] KIP-571: Add option to force remove members in 
StreamsResetter

 -----BEGIN PGP SIGNED MESSAGE-----
 Hash: SHA512

 Feyman,

 thanks a lot for the KIP. Overall LGTM. I have a few more comment and
 questions (sorry for the late reply):


 The KIP mentions that some constructors will be deprecated. Those should
 be listed explicitly. For example:


 public class MemberToRemove {

   // deprecated methods

   @Deprecated
   public MemberToRemove(String groupInstanceId);

   // new methods

   public MemberToRemove()

   public MemberToRemove withGroupInstanceId(String groupInstanceId)

   public MemberToRemove withMemberId(String memberId)
 }

 What happens is `groupInstanceId` is used for a dynamic group? What
 happens if both parameters are specified? What happens if `memberId`
 is specified for a static group?


 About the `--force` option. Currently, StreamsResetter fails with an
 error if the consumer group is not empty. You state in your KIP that:

 > without --force, we need to wait for session timeout.

 Is this an intended behavior change if `--force` is not used or is the
 KIP description incorrect?

 For my own understanding: with the `--force` option, we intend to get
 all `memberIds` and send a "remove member" request for each with
 corresponding `memberId` to remove the member from the group
 (independent is the group is static or dynamic)?

 I am really wondering, if for a static group, we should allow users to
 remove individual members? For a dynamic group this feature would not
 make much sense IMHO, because the `memberId` is not know by the user.



 - -Matthias


 On 3/5/20 7:15 AM, Bill Bejeck wrote:
 > Thanks for the KIP.  +1 (binding).
 >
 > -Bill
 >
 >
 >
 > On Wed, Mar 4, 2020 at 12:40 AM Guozhang Wang <wangg...@gmail.com>
 > wrote:
 >
 >> Thanks, +1 from me (binding).
 >>
 >> On Tue, Mar 3, 2020 at 9:39 PM feyman2009 <feyman2...@aliyun.com>
 >> wrote:
 >>
 >>> Hi, Guozhang Thanks a lot for the advice, that make sense! I
 >>> have updated the KIP page with the operational steps of
 >>> StreamsResetter.
 >>>
 >>> Thanks! Feyman
 >>>
 >>> ------------------------------------------------------------------
 >>>
 >>>
 发件人:Guozhang Wang <wangg...@gmail.com>
 >>> 发送时间:2020年3月3日(星期二) 14:22 收件人:dev <dev@kafka.apache.org>;
 >>> feyman2009 <feyman2...@aliyun.com> 主 题:Re: 回复:回复:[Vote]
 >>> KIP-571: Add option to force remove members in StreamsResetter
 >>>
 >>> Hello Feyman, thanks for the proposal!
 >>>
 >>> I read through the doc and overall it looks good to me.
 >>>
 >>> One minor thing I'd still like to point out is that, the
 >>> "removeMembersFromConsumerGroup" only sends a leave-group
 >>> request to the coordinator to let it remove the member,
 >>> however, if the member is still there alive and running then it
 >>> would soon be notified that it is no
 >> longer
 >>> a legal member of the group via heartbeats, and then
 >>> automatically tries
 >> to
 >>> re-join the group. So on the operational side, it is still
 >>> required that the following steps:
 >>>
 >>> 1) first stop the consumers (of streams instances), wait until
 >>> the shutdown is complete. 2) then use admin client in case the
 >>> stopped consumers are still registered at the broker side and
 >>> we do not want to wait for session timeout.
 >>>
 >>> Even with this KIP, people should still not skip step 1) above,
 >>> since otherwise the consumers would re-connect and re-join the
 >>> group
 >> immediately
 >>> still.
 >>>
 >>> In your doc you've already mentioned "Furthermore, users should
 >>> make sure all the stream applications are shutdown when running
 >>> StreamsResetter
 >> with
 >>> --force, otherwise it might trigger unexpected rebalance. "
 >>> What I'd want to clarify is that no matter if "--force" option
 >>> is enabled, this is
 >> always
 >>> the case that users should shutdown the streams instances
 >>> first, and then use the streams resetter :)
 >>>
 >>> As long as that is clarified in the proposal documentation, I'm
 >>> +1 on
 >> this
 >>> KIP.
 >>>
 >>>
 >>> Thanks again for the contribution, Guozhang
 >>>
 >>>
 >>> On Mon, Mar 2, 2020 at 6:31 AM feyman2009
 >>> <feyman2...@aliyun.com.invalid
 >>>
 >>> wrote: Hi, John Sorry, I have mistaken the KIP approval
 >>> standard, anyway, I will
 >> start
 >>> the PR soon and waiting for more binding approvals.
 >>>
 >>> Thanks! Feyman
 >>>
 >>>
 >>> ------------------------------------------------------------------
 >>>
 >>>
 发件人:John Roesler <vvcep...@apache.org>
 >>> 发送时间:2020年3月2日(星期一) 22:00 收件人:dev
 <dev@kafka.apache.org> 主
 >>> 题:Re: 回复:回复:[Vote] KIP-571: Add option to force remove members
 >>> in StreamsResetter
 >>>
 >>> Hi Feyman,
 >>>
 >>> Sorry, but we actually need 3 binding votes for the KIP to
 >>> pass. Please feel free to keep bumping the thread until some
 >>> more committers can take
 >> a
 >>> look.
 >>>
 >>> By the way, you can totally start a PR, but we can’t merge it
 >>> until the KIP passes the vote.
 >>>
 >>> Thanks! John
 >>>
 >>> On Mon, Mar 2, 2020, at 00:24, feyman2009 wrote:
 >>>> Hi,all Since currently we have 1 binding and two non-binding
 >>>> +1, I will update the KIP-571 as adopted and initiate a PR
 >>>> shortly
 >>>>
 >>>> Thanks! Feyman
 >>>>
 >>>>
 >>>> ------------------------------------------------------------------
 >>>>
 >>>>
 发件人:Sophie Blee-Goldman <sop...@confluent.io>
 >>>> 发送时间:2020年2月28日(星期五) 10:17 收件人:dev
 <dev@kafka.apache.org> 主
 >>>> 题:Re: 回复:[Vote] KIP-571: Add option to force remove members
 >>>> in
 >>> StreamsResetter
 >>>>
 >>>> Thanks for the KIP, +1 (non-binding)
 >>>>
 >>>> On Thu, Feb 27, 2020 at 12:40 PM Boyang Chen <
 >> reluctanthero...@gmail.com
 >>>>
 >>>> wrote:
 >>>>
 >>>>> Thanks Feyman, +1 (non-binding)
 >>>>>
 >>>>> On Thu, Feb 27, 2020 at 9:25 AM John Roesler
 >>>>> <vvcep...@apache.org>
 >>> wrote:
 >>>>>
 >>>>>> Thanks for the proposal!
 >>>>>>
 >>>>>> I'm +1 (binding) -John
 >>>>>>
 >>>>>> On Wed, Feb 26, 2020, at 19:41, feyman2009 wrote:
 >>>>>>> Updated with the KIP link:
 >>>>>>>
 >>>>>>
 >>>>>
 >>>
 >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+opti
 on+to+force+remove+members+in+StreamsResetter
 >>>>>>>
 >>>>>>>
 >>>>>>>
 >>
 >>
 - ------------------------------------------------------------------
 >>>>>>> 发件人:feyman2009 <feyman2...@aliyun.com.INVALID> 发送时
 >>>>>>> 间:2020年2月27日(星期四) 09:38 收件人:dev <dev@kafka.apache.org>
 >>>>>>> 主 题:[Vote] KIP-571: Add option to force remove members
 >>>>>>> in
 >>>>> StreamsResetter
 >>>>>>>
 >>>>>>>
 >>>>>>> Hi, all I would like to start a vote on KIP-571: Add
 >>>>>>> option to force
 >>> remove
 >>>>>>> members in StreamsResetter .
 >>>>>>>
 >>>>>>> Thanks! Feyman
 >>>>>>>
 >>>>>>>
 >>>>>>
 >>>>>
 >>>>
 >>>>
 >>>
 >>>
 >>>
 >>> -- -- Guozhang
 >>>
 >>>
 >>>
 >>
 >> -- -- Guozhang
 >>
 >
 -----BEGIN PGP SIGNATURE-----

 iQIzBAEBCgAdFiEEI8mthP+5zxXZZdDSO4miYXKq/OgFAl5hQrgACgkQO4miYXKq
 /OjDFQ/9GSMU0BIOvXjc2QeidqUBHJuhmrxr4sk6Adov2bR5CQxcXjocDibujICt
 Yybt9Ob7wWUQVAxsh2UDEN6sTjIvn2PB9gY9YwFzil2Mw66PdarSWDcImJQc07ZP
 RSbV3I3/2KvPlUJK+xPMc+M7+vaEU2ByE/EhAc6mQk5X+F0piZ/1W5O83po7i0Xu
 0l8j57CDkeKJcAN9mqr7vY3OFKr5/hAtSWCstCYiz6Xv39XcKU+VxX+PvXE8tRjc
 mHzzsYOgShhzuLayI5HRbBkUxvm6RiadqBx5LW8TUuiYYgAApJhbnf0DEkWOg3/6
 CcDh7LPzA25F0ayiMoJUhw5mxBiFnDHrqEjoR4t5Bywb/zQEG5BTq8spbad+shth
 OpGO8fBcD4zb+zfFkZdp8hROUbq/Hi8YzoTgXhOieA0l0lMoQhGi0SFaOHioHwiL
 XUoNrjeXjqJRWPe2By6lQn/uEAWynWWY4yVxym+TDqtK9heZmyBS4bY3RZ9J81dY
 zxwgDxbyPZUNdVe5vM9Bm269kJFlXOz0oY/ipaxwu6ebU8TJlmtSZQUkXtQ3p889
 ELDbOMhCZQMHoVYMgXsQG/UbLWqOtyEYauA5x50YZPf/Ux7bIyWt7IF4Bq7qDVG2
 ET6p89AY67OswUb8bAEZuNvGn6jAqgULEB/CPHbku/CIyzIvX1o=
 =hRhk
 -----END PGP SIGNATURE-----


Reply via email to