-----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-----