Thanks John. I took out the KafkaConsumer method and moved the javadocs to the Consumer#enforceRebalance in the KIP -- hope you're happy :P
Also, I wanted to point out one minor change to the current proposal: make this a blocking call, which accepts a timeout and returns whether the rebalance completed within the timeout. It will still reduce to a nonblocking call if a "zero" timeout is supplied. I've updated the KIP accordingly. Let me know if there are any further concerns, else I'll call for a vote. Cheers! Sophie On Mon, Feb 10, 2020 at 12:47 PM John Roesler <vvcep...@apache.org> wrote: > 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 > > >> > > > > >> > > > >> > > > > > >
