Requesting review of PR 10377
Hi team, Could anyone help to review PR 10377, thanks! KAFKA-12515 ApiVersionManager should create a response based on the request https://github.com/apache/kafka/pull/10377/files Haoran Xuan
回复：[ANNOUNCE] New Kafka PMC Member: Randall Hauch
Congratulations Randall! Haoran -- 发件人：Luke Chen 发送时间：2021年4月17日(星期六) 12:05 收件人：Kafka Users 抄 送：dev 主 题：Re: [ANNOUNCE] New Kafka PMC Member: Randall Hauch Congratulations Randall! Luke Bill Bejeck 於 2021年4月17日 週六 上午11:33 寫道： > Congratulations Randall! > > -Bill > > On Fri, Apr 16, 2021 at 11:10 PM lobo xu wrote: > > > Congrats Randall > > >
回复：[ANNOUNCE] New committer: Xi Hu
Congrats, Xi! Feyman -- 发件人：Satish Duggana 发送时间：2020年6月29日(星期一) 13:37 收件人：dev 主 题：Re: [ANNOUNCE] New committer: Xi Hu Congratulations Xi! On Mon, Jun 29, 2020 at 10:41 AM Konstantine Karantasis wrote: > > Congrats Xi! > > -Konstantine > > On Sat, Jun 27, 2020 at 7:13 PM Matt Wang wrote: > > > Congratulations ~ > > > > > > -- > > > > Best, > > Matt Wang > > > > > > On 06/26/2020 23:06，David Jacot wrote： > > Congrats! > > > > On Thu, Jun 25, 2020 at 4:08 PM Hu Xi wrote: > > > > Thank you, everyone. It is my great honor to be a part of the community. > > Will make a greater contribution in the coming days. > > > > > > 发件人: Roc Marshal > > 发送时间: 2020年6月25日 10:20 > > 收件人: us...@kafka.apache.org > > 主题: Re:Re: [ANNOUNCE] New committer: Xi Hu > > > > Congratulations ! Xi Hu. > > > > > > Best, > > Roc Marshal. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > At 2020-06-25 01:30:33, "Boyang Chen" wrote: > > Congratulations Xi! Well deserved. > > > > On Wed, Jun 24, 2020 at 10:10 AM AJ Chen wrote: > > > > Congratulations, Xi. > > -aj > > > > > > > > On Wed, Jun 24, 2020 at 9:27 AM Guozhang Wang > > wrote: > > > > The PMC for Apache Kafka has invited Xi Hu as a committer and we are > > pleased to announce that he has accepted! > > > > Xi Hu has been actively contributing to Kafka since 2016, and is well > > recognized especially for his non-code contributions: he maintains a > > tech > > blog post evangelizing Kafka in the Chinese speaking community ( > > https://www.cnblogs.com/huxi2b/), and is one of the most active > > answering > > member in Zhihu (Chinese Reddit / StackOverflow) Kafka topic. He has > > presented in Kafka meetup events in the past and authored a > > book deep-diving on Kafka architecture design and operations as well ( > > https://www.amazon.cn/dp/B07JH9G2FL). Code wise, he has contributed > > 75 > > patches so far. > > > > > > Thanks for all the contributions Xi. Congratulations! > > > > -- Guozhang, on behalf of the Apache Kafka PMC > > > > > > > >
回复：回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi, team I updated the KIP-571 since we took a slightly different implementation in the PR https://github.com/apache/kafka/pull/8589, basically: In RemoveMembersFromConsumerGroupOptions, leveraging empty members rather than introducing a new field to imply the removeAll scenario. Please let me know if you have any concerns, thanks a lot! Feyman -- 发件人：feyman2009 发送时间：2020年4月13日(星期一) 08:47 收件人：dev 主 题：回复：回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks , John and Guochang! -- 发件人：Guozhang Wang 发送时间：2020年4月11日(星期六) 03:07 收件人：dev 主 题：Re: 回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks Feyman, I've looked at the update that you incorporated from Matthias and that LGTM too. I'm still +1 :) Guozhang On Fri, Apr 10, 2020 at 11:18 AM John Roesler wrote: > Hey Feyman, > > Just to remove any ambiguity, I've been casually following the discussion, > I've just looked at the KIP document again, and I'm still +1 (binding). > > Thanks, > -John > > On Fri, Apr 10, 2020, at 01:44, feyman2009 wrote: > > Hi, all > > KIP-571 has already collected 4 bind +1 (John, Guochang, Bill, > > Matthias) and 3 non-binding +1(Boyang, Sophie), I will mark it as > > approved and create a PR shortly. > > Thanks! > > > > Feyman > > -- > > 发件人：feyman2009 > > 发送时间：2020年4月8日(星期三) 14:21 > > 收件人：dev ; Boyang Chen > > 主 题：回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove > > members in StreamsResetter > > > > Hi Boyang, > > Thanks for reminding me of that! > > I'm not sure about the convention, I thought it would need to > > re-collect votes if the KIP has changed~ > > Let's leave the vote thread here for 2 days, if no objection, I > > will take it as approved and update the PR accordingly. > > > > Thanks! > > Feyman > > > > > > > > -- > > 发件人：Boyang Chen > > 发送时间：2020年4月8日(星期三) 12:42 > > 收件人：dev ; feyman2009 > > 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove > > members in StreamsResetter > > > > You should already get enough votes if I'm counting correctly > > (Guozhang, John, Matthias) > > On Tue, Apr 7, 2020 at 6:59 PM feyman2009 > > wrote: > > Hi, Boyang > > I think Matthias's proposal makes sense, but we can use the admin > > tool for this scenario as Boyang mentioned or follow up later, so I > > prefer to keep this KIP unchanged to minimize the scope. > > Calling for vote ~ > > > > Thanks! > > Feyman > > > > -- > > 发件人：Boyang Chen > > 发送时间：2020年4月8日(星期三) 02:15 > > 收件人：dev > > 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove > > members in StreamsResetter > > > > Hey Feyman, > > > > I think Matthias' suggestion is optional, and we could just use admin > tool > > to remove single static members as well. > > > > Boyang > > > > On Tue, Apr 7, 2020 at 11:00 AM Matthias J. Sax > wrote: > > > > > > Would you mind to elaborate why we still need that > > > > > > Sure. > > > > > > For static memership, the session timeout it usually set quite high. > > > This make scaling in an application tricky: if you shut down one > > > instance, no rebalance would happen until `session.timeout.ms` hits. > > > This is specific to Kafka Streams, because when a Kafka Stream > > client is > > > closed, it does _not_ send a `LeaveGroupRequest`. Hence, the > > > corresponding partitions would not be processed for a long time and > > > thus, fall back. > > > > > > Given that each instance will have a unique `instance.id` provided by > > > the user, we could allow users to remove the instance they want to > > > decommission from the consumer group without the need to wait for > > > `session.timeout.ms`. > > > > > > Hence, it's not an application reset scenario for which one wants to > > > remove all members, but a scaling-in scenario. For dynamic > > membership, > > > this issue usually does not occur because the `session.timeout.ms` is > > > set to a fairly low value and a rebalance would happen quick
回复：回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Thanks , John and Guochang! -- 发件人：Guozhang Wang 发送时间：2020年4月11日(星期六) 03:07 收件人：dev 主 题：Re: 回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks Feyman, I've looked at the update that you incorporated from Matthias and that LGTM too. I'm still +1 :) Guozhang On Fri, Apr 10, 2020 at 11:18 AM John Roesler wrote: > Hey Feyman, > > Just to remove any ambiguity, I've been casually following the discussion, > I've just looked at the KIP document again, and I'm still +1 (binding). > > Thanks, > -John > > On Fri, Apr 10, 2020, at 01:44, feyman2009 wrote: > > Hi, all > > KIP-571 has already collected 4 bind +1 (John, Guochang, Bill, > > Matthias) and 3 non-binding +1(Boyang, Sophie), I will mark it as > > approved and create a PR shortly. > > Thanks! > > > > Feyman > > -- > > 发件人：feyman2009 > > 发送时间：2020年4月8日(星期三) 14:21 > > 收件人：dev ; Boyang Chen > > 主 题：回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove > > members in StreamsResetter > > > > Hi Boyang, > > Thanks for reminding me of that! > > I'm not sure about the convention, I thought it would need to > > re-collect votes if the KIP has changed~ > > Let's leave the vote thread here for 2 days, if no objection, I > > will take it as approved and update the PR accordingly. > > > > Thanks! > > Feyman > > > > > > > > -- > > 发件人：Boyang Chen > > 发送时间：2020年4月8日(星期三) 12:42 > > 收件人：dev ; feyman2009 > > 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove > > members in StreamsResetter > > > > You should already get enough votes if I'm counting correctly > > (Guozhang, John, Matthias) > > On Tue, Apr 7, 2020 at 6:59 PM feyman2009 > > wrote: > > Hi, Boyang > > I think Matthias's proposal makes sense, but we can use the admin > > tool for this scenario as Boyang mentioned or follow up later, so I > > prefer to keep this KIP unchanged to minimize the scope. > > Calling for vote ~ > > > > Thanks! > > Feyman > > > > -- > > 发件人：Boyang Chen > > 发送时间：2020年4月8日(星期三) 02:15 > > 收件人：dev > > 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove > > members in StreamsResetter > > > > Hey Feyman, > > > > I think Matthias' suggestion is optional, and we could just use admin > tool > > to remove single static members as well. > > > > Boyang > > > > On Tue, Apr 7, 2020 at 11:00 AM Matthias J. Sax > wrote: > > > > > > Would you mind to elaborate why we still need that > > > > > > Sure. > > > > > > For static memership, the session timeout it usually set quite high. > > > This make scaling in an application tricky: if you shut down one > > > instance, no rebalance would happen until `session.timeout.ms` hits. > > > This is specific to Kafka Streams, because when a Kafka Stream > > client is > > > closed, it does _not_ send a `LeaveGroupRequest`. Hence, the > > > corresponding partitions would not be processed for a long time and > > > thus, fall back. > > > > > > Given that each instance will have a unique `instance.id` provided by > > > the user, we could allow users to remove the instance they want to > > > decommission from the consumer group without the need to wait for > > > `session.timeout.ms`. > > > > > > Hence, it's not an application reset scenario for which one wants to > > > remove all members, but a scaling-in scenario. For dynamic > > membership, > > > this issue usually does not occur because the `session.timeout.ms` is > > > set to a fairly low value and a rebalance would happen quickly after > > an > > > instance is decommissioned. > > > > > > Does this make sense? > > > > > > As said before, we may or may not include this in this KIP. It's up > > to > > > you if you want to address it or not. > > > > > > > > > -Matthias > > > > > > > > > > > > On 4/7/20 7:12 AM, feyman2009 wrote: > > > > Hi, Matthias > > > > Thanks a lot! > > > > So you do not plan so support removing a _single stat
回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi, all KIP-571 has already collected 4 bind +1 (John, Guochang, Bill, Matthias) and 3 non-binding +1(Boyang, Sophie), I will mark it as approved and create a PR shortly. Thanks! Feyman -- 发件人：feyman2009 发送时间：2020年4月8日(星期三) 14:21 收件人：dev ; Boyang Chen 主 题：回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hi Boyang, Thanks for reminding me of that! I'm not sure about the convention, I thought it would need to re-collect votes if the KIP has changed~ Let's leave the vote thread here for 2 days, if no objection, I will take it as approved and update the PR accordingly. Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年4月8日(星期三) 12:42 收件人：dev ; feyman2009 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter You should already get enough votes if I'm counting correctly (Guozhang, John, Matthias) On Tue, Apr 7, 2020 at 6:59 PM feyman2009 wrote: Hi, Boyang I think Matthias's proposal makes sense, but we can use the admin tool for this scenario as Boyang mentioned or follow up later, so I prefer to keep this KIP unchanged to minimize the scope. Calling for vote ~ Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年4月8日(星期三) 02:15 收件人：dev 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hey Feyman, I think Matthias' suggestion is optional, and we could just use admin tool to remove single static members as well. Boyang On Tue, Apr 7, 2020 at 11:00 AM Matthias J. Sax wrote: > > Would you mind to elaborate why we still need that > > Sure. > > For static memership, the session timeout it usually set quite high. > This make scaling in an application tricky: if you shut down one > instance, no rebalance would happen until `session.timeout.ms` hits. > This is specific to Kafka Streams, because when a Kafka Stream client is > closed, it does _not_ send a `LeaveGroupRequest`. Hence, the > corresponding partitions would not be processed for a long time and > thus, fall back. > > Given that each instance will have a unique `instance.id` provided by > the user, we could allow users to remove the instance they want to > decommission from the consumer group without the need to wait for > `session.timeout.ms`. > > Hence, it's not an application reset scenario for which one wants to > remove all members, but a scaling-in scenario. For dynamic membership, > this issue usually does not occur because the `session.timeout.ms` is > set to a fairly low value and a rebalance would happen quickly after an > instance is decommissioned. > > Does this make sense? > > As said before, we may or may not include this in this KIP. It's up to > you if you want to address it or not. > > > -Matthias > > > > On 4/7/20 7:12 AM, feyman2009 wrote: > > Hi, Matthias > > Thanks a lot! > > So you do not plan so support removing a _single static_ member via > `StreamsResetter`? > > => > > Would you mind to elaborate why we still need that if we are > able to batch remove active members with adminClient? > > > > Thanks! > > > > Feyman > > -- > > 发件人：Matthias J. Sax > > 发送时间：2020年4月7日(星期二) 08:25 > > 收件人：dev > > 主 题：Re: 回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members > in StreamsResetter > > > > Overall LGTM. > > > > +1 (binding) > > > > So you do not plan so support removing a _single static_ member via > > `StreamsResetter`? We can of course still add this as a follow up but it > > might be nice to just add it to this KIP right away. Up to you if you > > want to include it or not. > > > > > > -Matthias > > > > > > > > On 3/30/20 8:16 AM, feyman2009 wrote: > >> Hi, Boyang > >> Thanks a lot, that make sense, we should not expose member.id in > the MemberToRemove anymore, I have fixed it in the KIP. > >> > >> > >> Feyman > >> -- > >> 发件人：Boyang Chen > >> 发送时间：2020年3月29日(星期日) 01:45 > >> 收件人：dev ; feyman2009 > >> 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > >> > >> Hey Feyman, > >> > >> thanks for the update. I ass
回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi Boyang, Thanks for reminding me of that! I'm not sure about the convention, I thought it would need to re-collect votes if the KIP has changed~ Let's leave the vote thread here for 2 days, if no objection, I will take it as approved and update the PR accordingly. Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年4月8日(星期三) 12:42 收件人：dev ; feyman2009 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter You should already get enough votes if I'm counting correctly (Guozhang, John, Matthias) On Tue, Apr 7, 2020 at 6:59 PM feyman2009 wrote: Hi, Boyang I think Matthias's proposal makes sense, but we can use the admin tool for this scenario as Boyang mentioned or follow up later, so I prefer to keep this KIP unchanged to minimize the scope. Calling for vote ~ Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年4月8日(星期三) 02:15 收件人：dev 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hey Feyman, I think Matthias' suggestion is optional, and we could just use admin tool to remove single static members as well. Boyang On Tue, Apr 7, 2020 at 11:00 AM Matthias J. Sax wrote: > > Would you mind to elaborate why we still need that > > Sure. > > For static memership, the session timeout it usually set quite high. > This make scaling in an application tricky: if you shut down one > instance, no rebalance would happen until `session.timeout.ms` hits. > This is specific to Kafka Streams, because when a Kafka Stream client is > closed, it does _not_ send a `LeaveGroupRequest`. Hence, the > corresponding partitions would not be processed for a long time and > thus, fall back. > > Given that each instance will have a unique `instance.id` provided by > the user, we could allow users to remove the instance they want to > decommission from the consumer group without the need to wait for > `session.timeout.ms`. > > Hence, it's not an application reset scenario for which one wants to > remove all members, but a scaling-in scenario. For dynamic membership, > this issue usually does not occur because the `session.timeout.ms` is > set to a fairly low value and a rebalance would happen quickly after an > instance is decommissioned. > > Does this make sense? > > As said before, we may or may not include this in this KIP. It's up to > you if you want to address it or not. > > > -Matthias > > > > On 4/7/20 7:12 AM, feyman2009 wrote: > > Hi, Matthias > > Thanks a lot! > > So you do not plan so support removing a _single static_ member via > `StreamsResetter`? > > => > > Would you mind to elaborate why we still need that if we are > able to batch remove active members with adminClient? > > > > Thanks! > > > > Feyman > > -- > > 发件人：Matthias J. Sax > > 发送时间：2020年4月7日(星期二) 08:25 > > 收件人：dev > > 主 题：Re: 回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members > in StreamsResetter > > > > Overall LGTM. > > > > +1 (binding) > > > > So you do not plan so support removing a _single static_ member via > > `StreamsResetter`? We can of course still add this as a follow up but it > > might be nice to just add it to this KIP right away. Up to you if you > > want to include it or not. > > > > > > -Matthias > > > > > > > > On 3/30/20 8:16 AM, feyman2009 wrote: > >> Hi, Boyang > >> Thanks a lot, that make sense, we should not expose member.id in > the MemberToRemove anymore, I have fixed it in the KIP. > >> > >> > >> Feyman > >> -- > >> 发件人：Boyang Chen > >> 发送时间：2020年3月29日(星期日) 01:45 > >> 收件人：dev ; feyman2009 > >> 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > >> > >> Hey Feyman, > >> > >> thanks for the update. I assume we would rely entirely on the internal > changes for `removeMemberFromGroup` by sending a DescribeGroup request > first. With that in mind, I don't think we need to add member.id to > MemberToRemove anymore, as it is only facing public where users will only > configure group.instance.id? > >> On Fri, Mar 27, 2020 at 5:04 PM feyman2009 > wrote: > >> Bump, can anyone kindly ta
回复：回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi, Boyang I think Matthias's proposal makes sense, but we can use the admin tool for this scenario as Boyang mentioned or follow up later, so I prefer to keep this KIP unchanged to minimize the scope. Calling for vote ~ Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年4月8日(星期三) 02:15 收件人：dev 主 题：Re: 回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hey Feyman, I think Matthias' suggestion is optional, and we could just use admin tool to remove single static members as well. Boyang On Tue, Apr 7, 2020 at 11:00 AM Matthias J. Sax wrote: > > Would you mind to elaborate why we still need that > > Sure. > > For static memership, the session timeout it usually set quite high. > This make scaling in an application tricky: if you shut down one > instance, no rebalance would happen until `session.timeout.ms` hits. > This is specific to Kafka Streams, because when a Kafka Stream client is > closed, it does _not_ send a `LeaveGroupRequest`. Hence, the > corresponding partitions would not be processed for a long time and > thus, fall back. > > Given that each instance will have a unique `instance.id` provided by > the user, we could allow users to remove the instance they want to > decommission from the consumer group without the need to wait for > `session.timeout.ms`. > > Hence, it's not an application reset scenario for which one wants to > remove all members, but a scaling-in scenario. For dynamic membership, > this issue usually does not occur because the `session.timeout.ms` is > set to a fairly low value and a rebalance would happen quickly after an > instance is decommissioned. > > Does this make sense? > > As said before, we may or may not include this in this KIP. It's up to > you if you want to address it or not. > > > -Matthias > > > > On 4/7/20 7:12 AM, feyman2009 wrote: > > Hi, Matthias > > Thanks a lot! > > So you do not plan so support removing a _single static_ member via > `StreamsResetter`? > > => > > Would you mind to elaborate why we still need that if we are > able to batch remove active members with adminClient? > > > > Thanks! > > > > Feyman > > -- > > 发件人：Matthias J. Sax > > 发送时间：2020年4月7日(星期二) 08:25 > > 收件人：dev > > 主 题：Re: 回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members > in StreamsResetter > > > > Overall LGTM. > > > > +1 (binding) > > > > So you do not plan so support removing a _single static_ member via > > `StreamsResetter`? We can of course still add this as a follow up but it > > might be nice to just add it to this KIP right away. Up to you if you > > want to include it or not. > > > > > > -Matthias > > > > > > > > On 3/30/20 8:16 AM, feyman2009 wrote: > >> Hi, Boyang > >> Thanks a lot, that make sense, we should not expose member.id in > the MemberToRemove anymore, I have fixed it in the KIP. > >> > >> > >> Feyman > >> -- > >> 发件人：Boyang Chen > >> 发送时间：2020年3月29日(星期日) 01:45 > >> 收件人：dev ; feyman2009 > >> 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > >> > >> Hey Feyman, > >> > >> thanks for the update. I assume we would rely entirely on the internal > changes for `removeMemberFromGroup` by sending a DescribeGroup request > first. With that in mind, I don't think we need to add member.id to > MemberToRemove anymore, as it is only facing public where users will only > configure group.instance.id? > >> On Fri, Mar 27, 2020 at 5:04 PM feyman2009 > wrote: > >> Bump, can anyone kindly take a look at the updated KIP-571? Thanks! > >> > >> > >> -- > >> 发件人：feyman2009 > >> 发送时间：2020年3月23日(星期一) 08:51 > >> 收件人：dev > >> 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > >> > >> Hi, team > >> I have updated the KIP-571 according to our latest discussion > results, would you mind to take a look? Thanks! > >> > >> Feyman > >> > >> > >> -- > >> 发件人：Boyang Chen > >> 发送时间：2020年3月19日(星期四) 13:41 > >> 收件人：dev ; feyman2009 > >> 主 题：Re: 回复：回复：回复：[Vote] KIP-571
回复：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi, Matthias Thanks a lot! So you do not plan so support removing a _single static_ member via `StreamsResetter`? => Would you mind to elaborate why we still need that if we are able to batch remove active members with adminClient? Thanks! Feyman -- 发件人：Matthias J. Sax 发送时间：2020年4月7日(星期二) 08:25 收件人：dev 主 题：Re: 回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Overall LGTM. +1 (binding) So you do not plan so support removing a _single static_ member via `StreamsResetter`? We can of course still add this as a follow up but it might be nice to just add it to this KIP right away. Up to you if you want to include it or not. -Matthias On 3/30/20 8:16 AM, feyman2009 wrote: > Hi, Boyang > Thanks a lot, that make sense, we should not expose member.id in the > MemberToRemove anymore, I have fixed it in the KIP. > > > Feyman > -- > 发件人：Boyang Chen > 发送时间：2020年3月29日(星期日) 01:45 > 收件人：dev ; feyman2009 > 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > > Hey Feyman, > > thanks for the update. I assume we would rely entirely on the internal > changes for `removeMemberFromGroup` by sending a DescribeGroup request first. > With that in mind, I don't think we need to add member.id to MemberToRemove > anymore, as it is only facing public where users will only configure > group.instance.id? > On Fri, Mar 27, 2020 at 5:04 PM feyman2009 > wrote: > Bump, can anyone kindly take a look at the updated KIP-571? Thanks! > > > ------ > 发件人：feyman2009 > 发送时间：2020年3月23日(星期一) 08:51 > 收件人：dev > 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > > Hi, team > I have updated the KIP-571 according to our latest discussion results, > would you mind to take a look? Thanks! > > Feyman > > > ------ > 发件人：Boyang Chen > 发送时间：2020年3月19日(星期四) 13:41 > 收件人：dev ; feyman2009 > 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > > Thanks for the insight Feyman. I personally feel adding another admin client > command is redundant, so picking option 1). The memberInfos struct is > internal and just used for result reference purposes. I think it could still > work even we overload with `removeAll` option, if that makes sense. > > Boyang > On Wed, Mar 18, 2020 at 8:51 PM feyman2009 > wrote: > Hi, team > Before going too far on the KIP update, I would like to hear your > opinions about how we would change the interface of AdminClient, the two > alternatives I could think of are: > 1) Extend adminClient.removeMembersFromConsumerGroup to support remove > all > As Guochang suggested, we could add some flag param in > RemoveMembersFromConsumerGroupOptions to indicated the "remove all" logic. > 2) Add a new API like > adminClient.removeAllMembersFromConsumerGroup(groupId, options) > > I think 1) will be more compact from the API perspective, but looking > at the code, I found that the if we are going to remove all, then the > RemoveMembersFromConsumerGroupResult#memberInfos/memberResult()/all() should > be changed accordingly, and they seem not that meaningful under the "remove > all" scenario. > > A minor thought about the adminClient.removeMembersFromConsumerGroup > API is: > Looking at some other deleteXX APIs, like deleteTopics, deleteRecords, > the results contains only a Map>, I think it's enough to > describe the related results, is it make sense that we may remove memberInfos > in RemoveMembersFromConsumerGroupResult ? This KIP has no dependency on this > if we choose alternative 2) > > Could you advise? Thanks! > > Feyman > > > 送时间：2020年3月15日(星期日) 10:11 > 收件人：dev > 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in > StreamsResetter > > Hi, all > Thanks a lot for your feedback! > According to the discussion, it seems we don't have some valid use > cases for removing specific dynamic members, I think it makes sense to > encapsulate the "get and delete" logic in adminClient. I will update the KIP > shortly! > > Thanks! > > Feyman > > > -- > 发件人：Boyang Chen > 发送时间：2020年3月14日(星期六) 00:39 > 收件人：dev > 主 题：Re: 回复：回复：回复：[V
回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi, Boyang Thanks a lot, that make sense, we should not expose member.id in the MemberToRemove anymore, I have fixed it in the KIP. Feyman -- 发件人：Boyang Chen 发送时间：2020年3月29日(星期日) 01:45 收件人：dev ; feyman2009 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hey Feyman, thanks for the update. I assume we would rely entirely on the internal changes for `removeMemberFromGroup` by sending a DescribeGroup request first. With that in mind, I don't think we need to add member.id to MemberToRemove anymore, as it is only facing public where users will only configure group.instance.id? On Fri, Mar 27, 2020 at 5:04 PM feyman2009 wrote: Bump, can anyone kindly take a look at the updated KIP-571? Thanks! -- 发件人：feyman2009 发送时间：2020年3月23日(星期一) 08:51 收件人：dev 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hi, team I have updated the KIP-571 according to our latest discussion results, would you mind to take a look? Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年3月19日(星期四) 13:41 收件人：dev ; feyman2009 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks for the insight Feyman. I personally feel adding another admin client command is redundant, so picking option 1). The memberInfos struct is internal and just used for result reference purposes. I think it could still work even we overload with `removeAll` option, if that makes sense. Boyang On Wed, Mar 18, 2020 at 8:51 PM feyman2009 wrote: Hi, team Before going too far on the KIP update, I would like to hear your opinions about how we would change the interface of AdminClient, the two alternatives I could think of are: 1) Extend adminClient.removeMembersFromConsumerGroup to support remove all As Guochang suggested, we could add some flag param in RemoveMembersFromConsumerGroupOptions to indicated the "remove all" logic. 2) Add a new API like adminClient.removeAllMembersFromConsumerGroup(groupId, options) I think 1) will be more compact from the API perspective, but looking at the code, I found that the if we are going to remove all, then the RemoveMembersFromConsumerGroupResult#memberInfos/memberResult()/all() should be changed accordingly, and they seem not that meaningful under the "remove all" scenario. A minor thought about the adminClient.removeMembersFromConsumerGroup API is: Looking at some other deleteXX APIs, like deleteTopics, deleteRecords, the results contains only a Map>, I think it's enough to describe the related results, is it make sense that we may remove memberInfos in RemoveMembersFromConsumerGroupResult ? This KIP has no dependency on this if we choose alternative 2) Could you advise? Thanks! Feyman 送时间：2020年3月15日(星期日) 10:11 收件人：dev 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hi, all Thanks a lot for your feedback! According to the discussion, it seems we don't have some valid use cases for removing specific dynamic members, I think it makes sense to encapsulate the "get and delete" logic in adminClient. I will update the KIP shortly! Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年3月14日(星期六) 00:39 收件人：dev 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks Matthias and Guozhang for the feedback. I'm not worrying too much about the member.id exposure as we have done so in a couple of areas. As for the recommended admin client change, I think it makes sense in an encapsulation perspective. Maybe I'm still a bit hesitant as we are losing the flexibility of closing only a subset of `dynamic members` potentially, but we could always get back and address it if some user feels necessary to have it. My short answer would be, LGTM :) Boyang On Thu, Mar 12, 2020 at 5:26 PM Guozhang Wang wrote: > Hi Matthias, > > About the AdminClient param API: that's a great point here. I think overall > if users want to just "remove all members" they should not need to first > get all the member.ids themselves, but instead internally the admin client > can first issue a describe-group request to get all the member.ids, and > then use them in the next issued leave-group request, all abstracted away > from the users. With that in mind, maybe in > RemoveMembersFromConsumerGroupOptions we can just introduce an overloaded > flag param besides "members" that indicate "remove all"? > > Guozhang > > On Thu, M
回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Bump, can anyone kindly take a look at the updated KIP-571? Thanks! -- 发件人：feyman2009 发送时间：2020年3月23日(星期一) 08:51 收件人：dev 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hi, team I have updated the KIP-571 according to our latest discussion results, would you mind to take a look? Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年3月19日(星期四) 13:41 收件人：dev ; feyman2009 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks for the insight Feyman. I personally feel adding another admin client command is redundant, so picking option 1). The memberInfos struct is internal and just used for result reference purposes. I think it could still work even we overload with `removeAll` option, if that makes sense. Boyang On Wed, Mar 18, 2020 at 8:51 PM feyman2009 wrote: Hi, team Before going too far on the KIP update, I would like to hear your opinions about how we would change the interface of AdminClient, the two alternatives I could think of are: 1) Extend adminClient.removeMembersFromConsumerGroup to support remove all As Guochang suggested, we could add some flag param in RemoveMembersFromConsumerGroupOptions to indicated the "remove all" logic. 2) Add a new API like adminClient.removeAllMembersFromConsumerGroup(groupId, options) I think 1) will be more compact from the API perspective, but looking at the code, I found that the if we are going to remove all, then the RemoveMembersFromConsumerGroupResult#memberInfos/memberResult()/all() should be changed accordingly, and they seem not that meaningful under the "remove all" scenario. A minor thought about the adminClient.removeMembersFromConsumerGroup API is: Looking at some other deleteXX APIs, like deleteTopics, deleteRecords, the results contains only a Map>, I think it's enough to describe the related results, is it make sense that we may remove memberInfos in RemoveMembersFromConsumerGroupResult ? This KIP has no dependency on this if we choose alternative 2) Could you advise? Thanks! Feyman 送时间：2020年3月15日(星期日) 10:11 收件人：dev 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hi, all Thanks a lot for your feedback! According to the discussion, it seems we don't have some valid use cases for removing specific dynamic members, I think it makes sense to encapsulate the "get and delete" logic in adminClient. I will update the KIP shortly! Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年3月14日(星期六) 00:39 收件人：dev 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks Matthias and Guozhang for the feedback. I'm not worrying too much about the member.id exposure as we have done so in a couple of areas. As for the recommended admin client change, I think it makes sense in an encapsulation perspective. Maybe I'm still a bit hesitant as we are losing the flexibility of closing only a subset of `dynamic members` potentially, but we could always get back and address it if some user feels necessary to have it. My short answer would be, LGTM :) Boyang On Thu, Mar 12, 2020 at 5:26 PM Guozhang Wang wrote: > Hi Matthias, > > About the AdminClient param API: that's a great point here. I think overall > if users want to just "remove all members" they should not need to first > get all the member.ids themselves, but instead internally the admin client > can first issue a describe-group request to get all the member.ids, and > then use them in the next issued leave-group request, all abstracted away > from the users. With that in mind, maybe in > RemoveMembersFromConsumerGroupOptions we can just introduce an overloaded > flag param besides "members" that indicate "remove all"? > > Guozhang > > On Thu, Mar 12, 2020 at 2:59 PM Matthias J. Sax wrote: > > > Feyman, > > > > some more comments/questions: > > > > The description of `LeaveGroupRequest` is clear but it's unclear how > > `MemberToRemove` should behave. Which parameter is required? Which is > > optional? What is the relationship between both. > > > > The `LeaveGroupRequest` description clearly states that specifying a > > `memberId` is optional if the `groupInstanceId` is provided. If > > `MemberToRemove` applies the same pattern, it must be explicitly defined > > in the KIP (and explained in the JavaDocs of `MemberToRemove`) because > > we cannot expect that an admin-client users knows that internally a > > `LeaveGroupRequest` is us
回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi, team I have updated the KIP-571 according to our latest discussion results, would you mind to take a look? Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年3月19日(星期四) 13:41 收件人：dev ; feyman2009 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks for the insight Feyman. I personally feel adding another admin client command is redundant, so picking option 1). The memberInfos struct is internal and just used for result reference purposes. I think it could still work even we overload with `removeAll` option, if that makes sense. Boyang On Wed, Mar 18, 2020 at 8:51 PM feyman2009 wrote: Hi, team Before going too far on the KIP update, I would like to hear your opinions about how we would change the interface of AdminClient, the two alternatives I could think of are: 1) Extend adminClient.removeMembersFromConsumerGroup to support remove all As Guochang suggested, we could add some flag param in RemoveMembersFromConsumerGroupOptions to indicated the "remove all" logic. 2) Add a new API like adminClient.removeAllMembersFromConsumerGroup(groupId, options) I think 1) will be more compact from the API perspective, but looking at the code, I found that the if we are going to remove all, then the RemoveMembersFromConsumerGroupResult#memberInfos/memberResult()/all() should be changed accordingly, and they seem not that meaningful under the "remove all" scenario. A minor thought about the adminClient.removeMembersFromConsumerGroup API is: Looking at some other deleteXX APIs, like deleteTopics, deleteRecords, the results contains only a Map>, I think it's enough to describe the related results, is it make sense that we may remove memberInfos in RemoveMembersFromConsumerGroupResult ? This KIP has no dependency on this if we choose alternative 2) Could you advise? Thanks! Feyman 送时间：2020年3月15日(星期日) 10:11 收件人：dev 主 题：回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Hi, all Thanks a lot for your feedback! According to the discussion, it seems we don't have some valid use cases for removing specific dynamic members, I think it makes sense to encapsulate the "get and delete" logic in adminClient. I will update the KIP shortly! Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年3月14日(星期六) 00:39 收件人：dev 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks Matthias and Guozhang for the feedback. I'm not worrying too much about the member.id exposure as we have done so in a couple of areas. As for the recommended admin client change, I think it makes sense in an encapsulation perspective. Maybe I'm still a bit hesitant as we are losing the flexibility of closing only a subset of `dynamic members` potentially, but we could always get back and address it if some user feels necessary to have it. My short answer would be, LGTM :) Boyang On Thu, Mar 12, 2020 at 5:26 PM Guozhang Wang wrote: > Hi Matthias, > > About the AdminClient param API: that's a great point here. I think overall > if users want to just "remove all members" they should not need to first > get all the member.ids themselves, but instead internally the admin client > can first issue a describe-group request to get all the member.ids, and > then use them in the next issued leave-group request, all abstracted away > from the users. With that in mind, maybe in > RemoveMembersFromConsumerGroupOptions we can just introduce an overloaded > flag param besides "members" that indicate "remove all"? > > Guozhang > > On Thu, Mar 12, 2020 at 2:59 PM Matthias J. Sax wrote: > > > Feyman, > > > > some more comments/questions: > > > > The description of `LeaveGroupRequest` is clear but it's unclear how > > `MemberToRemove` should behave. Which parameter is required? Which is > > optional? What is the relationship between both. > > > > The `LeaveGroupRequest` description clearly states that specifying a > > `memberId` is optional if the `groupInstanceId` is provided. If > > `MemberToRemove` applies the same pattern, it must be explicitly defined > > in the KIP (and explained in the JavaDocs of `MemberToRemove`) because > > we cannot expect that an admin-client users knows that internally a > > `LeaveGroupRequest` is used nor what the semantics of a > > `LeaveGroupRequest` are. > > > > > > About Admin API: > > > > In general, I am also confused that we allow so specify a `memberId` at > > all, because the `memberId` is an internal id that is
回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
moving a specific member from the group seems > > not to be an important feature -- for static groups we expect a long > > `session.timeout` and a user can also identify individual clients via > > `groupInstandId`, hence the feature makes sense for this case and is > > straight forward to use. > > > > > > About StreamsResetter: > > > > For this case we just say "remove all members" and thus the > > `describeConsumerGroup` approach works. However, it seems to be a > > special case? > > > > Or, if we expected that the "remove all members" use case is the norm, > > why can't we make a change admin-client to directly accept a `group.id`? > > The admin-client can internal first do a `DescribeGroupRequest` and > > afterward corresponding `LeaveGroupRequest` -- i.e., instead of building > > this pattern in `StreamsResetter` we build it directly into > `AdminClient`. > > > > Last, for static group the main use case seems to be to remove an > > individual member from the group but this feature is not covered by the > > KIP: I think using `--force` to remove all members makes sense, but an > > important second feature to remove an individual static member would > > require it's own flag to specify a single `group.instance.id`. > > > > > > Thoughts? > > > > > > -Matthias > > > > > > > > > > > > On 3/11/20 8:43 PM, feyman2009 wrote: > > > 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 > > > 发送时间：2020年3月10日(星期二) 03:02 > > > 收件人：dev ; feyman2009 > > > 主 题：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 >
回复：回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Hi, all Thanks a lot for your feedback! According to the discussion, it seems we don't have some valid use cases for removing specific dynamic members, I think it makes sense to encapsulate the "get and delete" logic in adminClient. I will update the KIP shortly! Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年3月14日(星期六) 00:39 收件人：dev 主 题：Re: 回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter Thanks Matthias and Guozhang for the feedback. I'm not worrying too much about the member.id exposure as we have done so in a couple of areas. As for the recommended admin client change, I think it makes sense in an encapsulation perspective. Maybe I'm still a bit hesitant as we are losing the flexibility of closing only a subset of `dynamic members` potentially, but we could always get back and address it if some user feels necessary to have it. My short answer would be, LGTM :) Boyang On Thu, Mar 12, 2020 at 5:26 PM Guozhang Wang wrote: > Hi Matthias, > > About the AdminClient param API: that's a great point here. I think overall > if users want to just "remove all members" they should not need to first > get all the member.ids themselves, but instead internally the admin client > can first issue a describe-group request to get all the member.ids, and > then use them in the next issued leave-group request, all abstracted away > from the users. With that in mind, maybe in > RemoveMembersFromConsumerGroupOptions we can just introduce an overloaded > flag param besides "members" that indicate "remove all"? > > Guozhang > > On Thu, Mar 12, 2020 at 2:59 PM Matthias J. Sax wrote: > > > Feyman, > > > > some more comments/questions: > > > > The description of `LeaveGroupRequest` is clear but it's unclear how > > `MemberToRemove` should behave. Which parameter is required? Which is > > optional? What is the relationship between both. > > > > The `LeaveGroupRequest` description clearly states that specifying a > > `memberId` is optional if the `groupInstanceId` is provided. If > > `MemberToRemove` applies the same pattern, it must be explicitly defined > > in the KIP (and explained in the JavaDocs of `MemberToRemove`) because > > we cannot expect that an admin-client users knows that internally a > > `LeaveGroupRequest` is used nor what the semantics of a > > `LeaveGroupRequest` are. > > > > > > About Admin API: > > > > In general, I am also confused that we allow so specify a `memberId` at > > all, because the `memberId` is an internal id that is not really exposed > > to the user. Hence, from a AdminClient point of view, accepting a > > `memberId` as input seems questionable to me? Of course, `memberId` can > > be collected via `describeConsumerGroups()` but it will return the > > `memberId` of _all_ consumer in the group and thus how would a user know > > which member should be removed for a dynamic group (if an individual > > member should be removed)? > > > > Hence, how can any user get to know the `memberId` of an individual > > client in a programtic way? > > > > Also I am wondering in general, why the removal of single dynamic member > > is important? In general, I would expect a short `session.timeout` for > > dynamic groups and thus removing a specific member from the group seems > > not to be an important feature -- for static groups we expect a long > > `session.timeout` and a user can also identify individual clients via > > `groupInstandId`, hence the feature makes sense for this case and is > > straight forward to use. > > > > > > About StreamsResetter: > > > > For this case we just say "remove all members" and thus the > > `describeConsumerGroup` approach works. However, it seems to be a > > special case? > > > > Or, if we expected that the "remove all members" use case is the norm, > > why can't we make a change admin-client to directly accept a `group.id`? > > The admin-client can internal first do a `DescribeGroupRequest` and > > afterward corresponding `LeaveGroupRequest` -- i.e., instead of building > > this pattern in `StreamsResetter` we build it directly into > `AdminClient`. > > > > Last, for static group the main use case seems to be to remove an > > individual member from the group but this feature is not covered by the > > KIP: I think using `--force` to remove all members makes sense, but an > > important second feature to remove an individual static member would > > require it's own flag to specify a single `group.instance.id`
回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
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 发送时间：2020年3月10日(星期二) 03:02 收件人：dev ; feyman2009 主 题：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 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
回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
> > > On Wed, Mar 4, 2020 at 12:40 AM Guozhang Wang > wrote: > >> Thanks, +1 from me (binding). >> >> On Tue, Mar 3, 2020 at 9:39 PM feyman2009 >> 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 >>> 发送时间：2020年3月3日(星期二) 14:22 收件人：dev ; >>> feyman2009 主 题：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 >>> >> >>> 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 >>> 发送时间：2020年3月2日(星期一) 22:00 收件人：dev 主 >>> 题：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 >>>> 发送时间：2020年2月28日(星期五) 10:17 收件人：dev 主 >>>> 题：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 >>>>> >>> wrote: >>>>> >>>>>> Than
回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
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 发送时间：2020年3月3日(星期二) 14:22 收件人：dev ; feyman2009 主 题：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 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 发送时间：2020年3月2日(星期一) 22:00 收件人：dev 主 题：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 > 发送时间：2020年2月28日(星期五) 10:17 > 收件人：dev > 主 题：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 > wrote: > > > Thanks Feyman, +1 (non-binding) > > > > On Thu, Feb 27, 2020 at 9:25 AM John Roesler 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+option+to+force+remove+members+in+StreamsResetter > > > > > > > > > > > > -- > > > > 发件人：feyman2009 > > > > 发送时间：2020年2月27日(星期四) 09:38 > > > > 收件人：dev > > > > 主 题：[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
回复：回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
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 发送时间：2020年3月2日(星期一) 22:00 收件人：dev 主 题：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 > 发送时间：2020年2月28日(星期五) 10:17 > 收件人：dev > 主 题：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 > wrote: > > > Thanks Feyman, +1 (non-binding) > > > > On Thu, Feb 27, 2020 at 9:25 AM John Roesler 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+option+to+force+remove+members+in+StreamsResetter > > > > > > > > > > > > -- > > > > 发件人：feyman2009 > > > > 发送时间：2020年2月27日(星期四) 09:38 > > > > 收件人：dev > > > > 主 题：[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 > > > > > > > > > > > > > > >
回复：回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
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 发送时间：2020年2月28日(星期五) 10:17 收件人：dev 主 题：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 wrote: > Thanks Feyman, +1 (non-binding) > > On Thu, Feb 27, 2020 at 9:25 AM John Roesler 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+option+to+force+remove+members+in+StreamsResetter > > > > > > > > > -- > > > 发件人：feyman2009 > > > 发送时间：2020年2月27日(星期四) 09:38 > > > 收件人：dev > > > 主 题：[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 > > > > > > > > >
回复：[Vote] KIP-571: Add option to force remove members in StreamsResetter
Updated with the KIP link: https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+option+to+force+remove+members+in+StreamsResetter -- 发件人：feyman2009 发送时间：2020年2月27日(星期四) 09:38 收件人：dev 主 题：[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
回复：[Discuss] KIP-571: Add option to force remove members in StreamsResetter
Hi, Sophie Thanks a lot! I have initiated a vote Thanks! Feyman -- 发件人：Sophie Blee-Goldman 发送时间：2020年2月27日(星期四) 08:04 收件人：feyman2009 主 题：Re: [Discuss] KIP-571: Add option to force remove members in StreamsResetter Hi guys, Just to clarify, I meant a batch API on the admin not for the StreamsResetter, to avoid extra round trips and a simpler API. But I suppose it might be useful to be able to remove individual (dynamic) members and not the whole group for other use cases that could then benefit from this as well. Anyways, I'm fine with the current plan if that makes sense to you. Feel free to call for a vote if the KIP is ready Cheers, Sophie On Mon, Feb 24, 2020 at 4:16 AM feyman2009 wrote: Hi, Boyang Thanks! I have updated the KIP :) If Sophie also thinks it's ok, I will start a vote soon. Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年2月24日(星期一) 00:42 收件人：dev 主 题：Re: [Discuss] KIP-571: Add option to force remove members in StreamsResetter Hey Feyman, thanks a lot for the update, the KIP LGTM now. Will let Sophie take a look again, also a minor API change: s/setGroupInstanceId/withGroupInstanceId, and similar to setMemberId, as usually setters are not expected to return an actual object. Boyang On Sat, Feb 22, 2020 at 11:05 PM feyman2009 wrote: > Hi, Boyang > Thanks for your review, I have updated the KIP page :) > > Hi, Sophie > Thanks for your suggestions! > 1) Did you consider an API that just removes *all* remaining members > from a group? > We plan to implement the batch removal in StreamsResetter as below: > 1) adminClient#describeConsumerGroups to get members in each > group, this part needs no change. > 2) adminClient#removeMembersFromConsumerGroup to remove all the > members got from the above call (This involves API change to support the > dynamic member removal) > I think your suggestion is feasible but maybe not necessary currently > since it is a subset of the combination of the above two APIs. Looking at > the APIs in KafkaAdminClient, the adminClient.deleteXXX always takes a > collection as the input parameter and the caller does the "query and > delete" if "delete all" is needed, this leaves more burden on the caller > side but increases flexibility. Since the KafkaAdminClient's API is still > evolving, I think it would be reasonable to follow the convention and not > adding a "removal all members" API. > > 2) Thanks to Boyang's correction, broker version >= 2.4 is needed > since batch members removal is introduced since then(please check KIP-345 > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-345%3A+Introduce+static+membership+protocol+to+reduce+consumer+rebalances> > for > details). > If it is used upon the older clusters like 2.3, > *UnsupportedVersionException > *will be thrown. > > Thanks! > Haoran > > -- > 发件人：Boyang Chen > 发送时间：2020年2月19日(星期三) 11:57 > 收件人：dev > 主 题：Re: [Discuss] KIP-571: Add option to force remove members in > StreamsResetter > > Also Feyman, there is one thing I forget which is that the leave group > change was introduced in 2.4 broker instead of 2.3. Feel free to correct it > on the KIP. > > On Tue, Feb 18, 2020 at 5:44 PM Sophie Blee-Goldman > wrote: > > > Hey Feyman, > > > > Thanks for the KIP! I had two high-level questions: > > > > > It seems like, in the specific case motivating this KIP, we would only ever > > want to remove *all* the members remaining in the group (and never just a > > single member at a time). As you mention there is already an admin API to > > > remove static members, but we'd still need something new to handle dynamic > > ones. Did you consider an API that just removes *all* remaining members > > from a group, rather than requiring the caller to determine and then > > specify the > > group.id (static) or member.id (dynamic) for each one? This way we can > > just > > > have a single API exposed that will handle what we need to do regardless of > > whether static membership is used or not. > > > > > My other question is, will this new option only work for clusters that are > > on 2.3 > > or higher? Do you have any thoughts about whether it would be possible to > > implement this feature for older clusters as well, or are we dependent on > > changes only introduced in 2.3? > > > > If so, we should make it absolutely clear what will happen if this used > > with > > an older cluster. That is, will the reset tool
[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
回复：[Discuss] KIP-571: Add option to force remove members in StreamsResetter
Hi, Boyang Thanks! I have updated the KIP :) If Sophie also thinks it's ok, I will start a vote soon. Thanks! Feyman -- 发件人：Boyang Chen 发送时间：2020年2月24日(星期一) 00:42 收件人：dev 主 题：Re: [Discuss] KIP-571: Add option to force remove members in StreamsResetter Hey Feyman, thanks a lot for the update, the KIP LGTM now. Will let Sophie take a look again, also a minor API change: s/setGroupInstanceId/withGroupInstanceId, and similar to setMemberId, as usually setters are not expected to return an actual object. Boyang On Sat, Feb 22, 2020 at 11:05 PM feyman2009 wrote: > Hi, Boyang > Thanks for your review, I have updated the KIP page :) > > Hi, Sophie > Thanks for your suggestions! > 1) Did you consider an API that just removes *all* remaining members > from a group? > We plan to implement the batch removal in StreamsResetter as below: > 1) adminClient#describeConsumerGroups to get members in each > group, this part needs no change. > 2) adminClient#removeMembersFromConsumerGroup to remove all the > members got from the above call (This involves API change to support the > dynamic member removal) > I think your suggestion is feasible but maybe not necessary currently > since it is a subset of the combination of the above two APIs. Looking at > the APIs in KafkaAdminClient, the adminClient.deleteXXX always takes a > collection as the input parameter and the caller does the "query and > delete" if "delete all" is needed, this leaves more burden on the caller > side but increases flexibility. Since the KafkaAdminClient's API is still > evolving, I think it would be reasonable to follow the convention and not > adding a "removal all members" API. > > 2) Thanks to Boyang's correction, broker version >= 2.4 is needed > since batch members removal is introduced since then(please check KIP-345 > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-345%3A+Introduce+static+membership+protocol+to+reduce+consumer+rebalances> > for > details). > If it is used upon the older clusters like 2.3, > *UnsupportedVersionException > *will be thrown. > > Thanks! > Haoran > > -- > 发件人：Boyang Chen > 发送时间：2020年2月19日(星期三) 11:57 > 收件人：dev > 主 题：Re: [Discuss] KIP-571: Add option to force remove members in > StreamsResetter > > Also Feyman, there is one thing I forget which is that the leave group > change was introduced in 2.4 broker instead of 2.3. Feel free to correct it > on the KIP. > > On Tue, Feb 18, 2020 at 5:44 PM Sophie Blee-Goldman > wrote: > > > Hey Feyman, > > > > Thanks for the KIP! I had two high-level questions: > > > > > It seems like, in the specific case motivating this KIP, we would only ever > > want to remove *all* the members remaining in the group (and never just a > > single member at a time). As you mention there is already an admin API to > > > remove static members, but we'd still need something new to handle dynamic > > ones. Did you consider an API that just removes *all* remaining members > > from a group, rather than requiring the caller to determine and then > > specify the > > group.id (static) or member.id (dynamic) for each one? This way we can > > just > > > have a single API exposed that will handle what we need to do regardless of > > whether static membership is used or not. > > > > > My other question is, will this new option only work for clusters that are > > on 2.3 > > or higher? Do you have any thoughts about whether it would be possible to > > implement this feature for older clusters as well, or are we dependent on > > changes only introduced in 2.3? > > > > If so, we should make it absolutely clear what will happen if this used > > with > > an older cluster. That is, will the reset tool exit with a clear error > > message right > > away, or will it potentially leave the app in a partially reset state? > > > > Thanks! > > Sophie > > > > On Tue, Feb 18, 2020 at 4:30 PM Boyang Chen > > wrote: > > > > > > Thanks for the update Feyman. The updates look great, except one thing I > > > would like to be more specific is error cases display. In the "*2)* Add > > > > cmdline option" you mention throwing exception when request failed, does > > > that suggest partial failure or a full failure? How do we deal with > > > different scenarios? > > > > > > Also some minor syntax fix: > > > > 1. it only support remove s
回复：[Discuss] KIP-571: Add option to force remove members in StreamsResetter
Hi, Boyang Thanks for your review, I have updated the KIP page :) Hi, Sophie Thanks for your suggestions! 1) Did you consider an API that just removes *all* remaining members from a group? We plan to implement the batch removal in StreamsResetter as below: 1) adminClient#describeConsumerGroups to get members in each group, this part needs no change. 2) adminClient#removeMembersFromConsumerGroup to remove all the members got from the above call (This involves API change to support the dynamic member removal) I think your suggestion is feasible but maybe not necessary currently since it is a subset of the combination of the above two APIs. Looking at the APIs in KafkaAdminClient, the adminClient.deleteXXX always takes a collection as the input parameter and the caller does the "query and delete" if "delete all" is needed, this leaves more burden on the caller side but increases flexibility. Since the KafkaAdminClient's API is still evolving, I think it would be reasonable to follow the convention and not adding a "removal all members" API. 2) Thanks to Boyang's correction, broker version >= 2.4 is needed since batch members removal is introduced since then(please check KIP-345 for details). If it is used upon the older clusters like 2.3, UnsupportedVersionException will be thrown. Thanks! Haoran -- 发件人：Boyang Chen 发送时间：2020年2月19日(星期三) 11:57 收件人：dev 主 题：Re: [Discuss] KIP-571: Add option to force remove members in StreamsResetter Also Feyman, there is one thing I forget which is that the leave group change was introduced in 2.4 broker instead of 2.3. Feel free to correct it on the KIP. On Tue, Feb 18, 2020 at 5:44 PM Sophie Blee-Goldman wrote: > Hey Feyman, > > Thanks for the KIP! I had two high-level questions: > > It seems like, in the specific case motivating this KIP, we would only ever > want to remove *all* the members remaining in the group (and never just a > single member at a time). As you mention there is already an admin API to > remove static members, but we'd still need something new to handle dynamic > ones. Did you consider an API that just removes *all* remaining members > from a group, rather than requiring the caller to determine and then > specify the > group.id (static) or member.id (dynamic) for each one? This way we can > just > have a single API exposed that will handle what we need to do regardless of > whether static membership is used or not. > > My other question is, will this new option only work for clusters that are > on 2.3 > or higher? Do you have any thoughts about whether it would be possible to > implement this feature for older clusters as well, or are we dependent on > changes only introduced in 2.3? > > If so, we should make it absolutely clear what will happen if this used > with > an older cluster. That is, will the reset tool exit with a clear error > message right > away, or will it potentially leave the app in a partially reset state? > > Thanks! > Sophie > > On Tue, Feb 18, 2020 at 4:30 PM Boyang Chen > wrote: > > > Thanks for the update Feyman. The updates look great, except one thing I > > would like to be more specific is error cases display. In the "*2)* Add > > cmdline option" you mention throwing exception when request failed, does > > that suggest partial failure or a full failure? How do we deal with > > different scenarios? > > > > Also some minor syntax fix: > > 1. it only support remove static members -> it only supports the removal > of > > static members > > 2. "new constructor is added and the old constructor will be deprecated" > > you mean the `new helper` right? Should be `new helper is added` > > 3. users should make sure all the stream applications should be are > > shutdown > > > > Other than the above suggestions, I think the KIP is in pretty good > shape. > > > > Boyang > > > > On Fri, Feb 14, 2020 at 9:29 PM feyman2009 > wrote: > > > > > Hi, Boyang > > > You can call me Feyman :) > > > Thanks for your quick reply with great advices! > > > I have updated the KIP-571 , would you mind to see if it looks > good ? > > > Thanks ! > > > > > > -- > > > 发件人：Boyang Chen > > > 发送时间：2020年2月14日(星期五) 08:35 > > > 收件人：dev ; feyman2009 > > > 主 题：Re: [Discuss] KIP-571: Add option to force remove members in > > > StreamsResetter > > > > > > Thanks for driving this change Feyman! Hope this is a good name to call > > > you :) &
回复：[Discuss] KIP-571: Add option to force remove members in StreamsResetter
Hi, Boyang You can call me Feyman :) Thanks for your quick reply with great advices! I have updated the KIP-571 , would you mind to see if it looks good ? Thanks ! -- 发件人：Boyang Chen 发送时间：2020年2月14日(星期五) 08:35 收件人：dev ; feyman2009 主 题：Re: [Discuss] KIP-571: Add option to force remove members in StreamsResetter Thanks for driving this change Feyman! Hope this is a good name to call you :) The motivation of the KIP looks good, and I have a couple of initial thoughts: 1. I guess the reason to use setters instead of adding a new constructor to MemberToRemove class is because we have two String members. Could you point that out upfront so that people are not asking why not adding new constructor? 2. KIP discussion usually focuses on the public side changes, so you don't need to copy-paste the entire class. Just the new APIs you are adding should be suffice 3. Add the description of new flag inside Public API change, whose name could be simplified as `--force` and people would just read the description. An edge case I could think of is that some ongoing applications are not closed when the reset tool applies, which causes more unexpected rebalances. So it's important to warn users to use the flag wisely and be responsible to shutdown old applications first. 4. It would be good to mention in the Compatibility section which version of broker and admin client we need to be able to use this new feature. Also what's the expected behavior when the broker is not supporting the new API. 5. What additional feedback for users using the new flag? Are we going to include a list of successfully deleted members, and some failed members? 6. We could separate the proposed change and public API section. In the proposed change section, we could have a mention of which API we are going to use to get the members of the stream application. And some minor style advices: 1. Remove the link on `member.id` inside Motivation section; 2. Use a code block for the new MemberToRemove and avoid unnecessary coloring; 3. Please pay more attention to style, for example `ability to force removing` has double spaces. Boyang On Thu, Feb 13, 2020 at 1:48 AM feyman2009 wrote: Hi, all In order to make it possible for StreamsResetter to reset stream even when there are active members, since we currently only have the ability to remove static members, so we basically need the ability to remove dynamic members, this involves some public interfaces change in org.apache.kafka.clients.admin.MemberToRemove. KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+option+to+force+remove+members+in+StreamsResetter JIRA: https://issues.apache.org/jira/browse/KAFKA-9146 Any comments would be highly appreciated~ Thanks !
[Discuss] KIP-571: Add option to force remove members in StreamsResetter
Hi, all In order to make it possible for StreamsResetter to reset stream even when there are active members, since we currently only have the ability to remove static members, so we basically need the ability to remove dynamic members, this involves some public interfaces change in org.apache.kafka.clients.admin.MemberToRemove. KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+option+to+force+remove+members+in+StreamsResetter JIRA: https://issues.apache.org/jira/browse/KAFKA-9146 Any comments would be highly appreciated~ Thanks !
回复：Requesting permission for creating KIP
Thanks a lot, Jun Rao! -- 发件人：Jun Rao 发送时间：2020年2月5日(星期三) 10:11 收件人：dev ; feyman2009 主 题：Re: Requesting permission for creating KIP Hi, Feyman, Thanks for your interest. Just gave you the wiki permission. Jun On Tue, Feb 4, 2020 at 4:38 PM feyman2009 wrote: > > my https://cwiki.apache.org/ account is the same as this sender email, > thanks !
Requesting permission for creating KIP
my https://cwiki.apache.org/ account is the same as this sender email, thanks !
