ableegoldman commented on code in PR #19400:
URL: https://github.com/apache/kafka/pull/19400#discussion_r2060908295
##########
streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java:
##########
@@ -1640,33 +1599,6 @@ public synchronized boolean close(final CloseOptions
options) throws IllegalArgu
return close(Optional.of(timeoutMs), options.leaveGroup);
}
- private Consumer<StreamThread> streamThreadLeaveConsumerGroup(final long
remainingTimeMs) {
Review Comment:
this method is only ever called from `KafkaStreams#close`, there's no
separate method KafkaStreams method that can be used to remove consumer from
the group via the admin client (side note: maybe there should be?). There's a
cli tool for static member removal so users who don't set `leaveGroup=true` in
the CloseOptions can do so through this.
So the new
`consumer#close(CloseOptions.groupMembershipOperation(LEAVE_GROUP))` is
essentially replacing this method entirely so in theory we should be able to
remove it. That said, it's definitely concerning if
KafkaStreamsCloseOptionsIntegrationTest is failing because we removed this.
That suggests maybe our hack/workaround for removing static members in
`consumer#close` via intentionally getting fenced doesn't work?
If that's the case then we should go back to using the admin's
`#removeMembersFromConsumerGroup` for Streams for now, and can follow up with
that KIP to do a proper fix for allowing static members to permanently leave
via LeaveGroup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
