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