Thanks Sophie, Sorry I didn't respond. I think your new method name sounds good.
Regarding the interface vs implementation, I agree it's confusing. It's always bothered me that the interface redirects you to an implementation JavaDocs, but never enough for me to stop what I'm doing to fix it. It's not a big deal either way, I just thought it was strange to propose a "public interface" change, but not in terms of the actual interface class. It _is_ true that KafkaConsumer is also part of the public API, but only really for the constructor. Any proposal to define a new "consumer client" API should be on the Consumer interface (which you said you plan to do anyway). I guess I brought it up because proposing an addition to Consumer implies it would be added to KafkaConsumer, but proposing an addition to KafkaConsumer does not necessarily imply it would also be added to Consumer. Does that make sense? Anyway, thanks for updating the KIP. Thanks, -John On Mon, Feb 10, 2020, at 14:38, Sophie Blee-Goldman wrote: > Since this doesn't seem too controversial, I'll probably call for a vote by > end of day. > If there any further comments/questions/concerns, please let me know! > > Thanks, > Sophie > > On Sat, Feb 8, 2020 at 12:19 AM Sophie Blee-Goldman <sop...@confluent.io> > wrote: > > > Thanks for the feedback! That's a good point about trying to prevent users > > from > > thinking they should use this API during normal processing and clarifying > > when/why > > you might need it -- regardless of the method name, we should explicitly > > call this out > > in the javadocs. > > > > As for the method name, on reflection I agree that "rejoinGroup" does not > > seem to be > > appropriate. Of course that's what the consumer will actually be doing, > > but that's just an > > implementation detail -- the name should reflect what the API is doing, > > not how it does it > > (which can always change). > > > > How about "enforceRebalance"? This is stolen from the StreamThread method > > that does > > exactly this (by unsubscribing) so it seems to fit. I'll update the KIP > > with this unless anyone > > has another suggestion. > > > > Regarding the Consumer vs KafkaConsumer matter, I included the > > KafkaConsumer method > > because that's where all the javadocs redirect to in the Consumer > > interface. Also, FWIW > > I'm pretty sure KafkaConsumer is also part of the public API -- we would > > be adding a new > > method to both. > > > > On Fri, Feb 7, 2020 at 7:42 PM John Roesler <vvcep...@apache.org> wrote: > > > >> Hi all, > >> > >> Thanks for the well motivated KIP, Sophie. I had some alternatives in > >> mind, which > >> I won't even bother to relate because I feel like the motivation made a > >> compelling > >> argument for the API as proposed. > >> > >> One very minor point you might as well fix is that the API change is > >> targeted at > >> KafkaConsumer (the implementation), but should be targeted at > >> Consumer (the interface). > >> > >> I agree with your discomfort about the name. Adding a "rejoin" method > >> seems strange > >> since there's no "join" method. Instead the way you join the group the > >> first time is just > >> by calling "subscribe". But "resubscribe" seems too indirect from what > >> we're really trying > >> to do, which is to trigger a rebalance by sending a new JoinGroup request. > >> > >> Another angle is that we don't want the method to sound like something > >> you should > >> be calling in normal circumstances, or people will be "tricked" into > >> calling it unnecessarily. > >> > >> So, I think "rejoinGroup" is fine, although a person _might_ be forgiven > >> for thinking they > >> need to call it periodically or something. Did you consider > >> "triggerRebalance", which > >> sounds pretty advanced-ish, and accurately describes what happens when > >> you call it? > >> > >> All in all, the KIP sounds good to me, and I'm in favor. > >> > >> Thanks, > >> -John > >> > >> On Fri, Feb 7, 2020, at 21:22, Anna McDonald wrote: > >> > This situation was discussed at length after a recent talk I gave. This > >> KIP > >> > would be a great step towards increased availability and in facilitating > >> > lightweight rebalances. > >> > > >> > anna > >> > > >> > > >> > > >> > On Fri, Feb 7, 2020, 9:38 PM Sophie Blee-Goldman <sop...@confluent.io> > >> > wrote: > >> > > >> > > Hi all, > >> > > > >> > > In light of some recent and upcoming rebalancing and availability > >> > > improvements, it seems we have a need for explicitly triggering a > >> consumer > >> > > group rebalance. Therefore I'd like to propose adding a new > >> > > rejoinGroup()method > >> > > to the Consumer client (better method name suggestions are very > >> welcome). > >> > > > >> > > Please take a look at the KIP and let me know what you think! > >> > > > >> > > KIP document: > >> > > > >> > > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-568%3A+Explicit+rebalance+triggering+on+the+Consumer > >> > > > >> > > JIRA: https://issues.apache.org/jira/browse/KAFKA-9525 > >> > > > >> > > Cheers, > >> > > Sophie > >> > > > >> > > >> > > >
