Re: KIP-4 Wiki Update

2016-04-04 Thread Guozhang Wang
About the "only controller" to "any broker" over time, I think I buy all these points. Since this is out of the scope of this KIP, and since as Grant said, this would be a rather general feature, but not only restricted to the added admin requests in this KIP, we can leave that discussion for later

Re: KIP-4 Wiki Update

2016-04-04 Thread Grant Henke
Thanks for the review Guozhang, see my responses below. 1. One minor thing: "Note there is a special use case - automatic topic > creation for TopicMetadataRequest, to trigger it user should set > client_id=consumer and define only topic name:" > I think the current behavior is that as long as t

Re: KIP-4 Wiki Update

2016-04-03 Thread Guozhang Wang
Hi Grant, Thanks for the write-ups. I had some comments besides the debate of metadata request principles: 1. One minor thing: "Note there is a special use case - automatic topic creation for TopicMetadataRequest, to trigger it user should set client_id=consumer and define only topic name:" I th

Re: KIP-4 Wiki Update

2016-04-03 Thread Ismael Juma
Hi Jun, Yes, that makes it a bit challenging to do this in a manner that is fully compatible. Having said that, most (all?) languages have Perl-derived regexes (including Java), so it seems like it should be possible to define a subset for the protocol. But perhaps that's a bigger change that requ

Re: KIP-4 Wiki Update

2016-04-03 Thread Jun Rao
Ismael, The reason that we only apply the regex for topics on the client side is that regex is java specific. So, for non-java clients, it would be a bit weird for them to include java specific stuff in the wire protocol. Thanks, Jun On Sun, Apr 3, 2016 at 12:36 AM, Ismael Juma wrote: > Hi Gr

Re: KIP-4 Wiki Update

2016-04-03 Thread Ismael Juma
Hi Grant, One question that occurred to me is whether we want to take the chance to make it possible to pass a regex pattern for the desired topics in the metadata request. This would potentially improve the efficiency of `KafkaConsumer.subscribe` significantly for cases where there are large numb

Re: KIP-4 Wiki Update

2016-03-31 Thread Grant Henke
I have update the wiki and patch based on the feedback on the metadata changes. Please take a look and let me know if there are any concerns or issues. - Wiki: https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandline

Re: KIP-4 Wiki Update

2016-03-31 Thread Jason Gustafson
I also prefer B. In addition to being intuitive, it seems less error-prone in the long term, though it might be a little annoying for clients maintaining backwards compatibility. -Jason On Thu, Mar 31, 2016 at 1:24 PM, Ismael Juma wrote: > I prefer B, the fact that we version the protocol means

Re: KIP-4 Wiki Update

2016-03-31 Thread Ismael Juma
I prefer B, the fact that we version the protocol means that we can fix mistakes instead of living with them forever. We should take advantage of that. Ismael On Thu, Mar 31, 2016 at 9:15 PM, Grant Henke wrote: > Looking for some resolution on the "No Topics" change. > > I am thinking that usin

Re: KIP-4 Wiki Update

2016-03-31 Thread Grant Henke
Looking for some resolution on the "No Topics" change. I am thinking that using null in the protocol isn't that complex, and it avoids various edge cases with having multiple fields. That leaves us with 2 options: - A: null = no topics, empty = all topics - B: null = all topics, empty = no

Re: KIP-4 Wiki Update

2016-03-31 Thread Grant Henke
> > I had a second look at the proposed changes to Metadata Request and > Response and it seems to me that having a `controller_id` field would be > more efficient for non-trivial cases than having a `is_controller` field > for each broker (which would be false for all but 1 case). I agree this

Re: KIP-4 Wiki Update

2016-03-31 Thread Ismael Juma
Hi Grant, I had a second look at the proposed changes to Metadata Request and Response and it seems to me that having a `controller_id` field would be more efficient for non-trivial cases than having a `is_controller` field for each broker (which would be false for all but 1 case). Similar, but

Re: KIP-4 Wiki Update

2016-03-30 Thread Joe Stein
If the meta data change for KIP-4 happens maybe we just update the KIP-4 page for the release, it would be great to have this available in 0.10 On Wed, Mar 30, 2016 at 3:36 PM, Grant Henke wrote: > Additionally feel free to review the PR (#1095 > ) to s

Re: KIP-4 Wiki Update

2016-03-30 Thread Grant Henke
Additionally feel free to review the PR (#1095 ) to see what the current proposal/implementation looks like. Right now its using null to signify no topics. That can be changed based on the discussion here. On Wed, Mar 30, 2016 at 2:34 PM, Grant Henke wro

Re: KIP-4 Wiki Update

2016-03-30 Thread Grant Henke
Thank you for the input everyone! I will try to address some of it below. I will stick to the metadata related discussion first and we can go from there. I would like to stay away from language/client style discussion to start. Instead I think it would be useful to focus on reviewing the wire prot

Re: KIP-4 Wiki Update

2016-03-30 Thread Ismael Juma
We can add an internal class until then (it's pretty trivial) since the request classes are internal. Ismael On Wed, Mar 30, 2016 at 7:00 PM, Gwen Shapira wrote: > I like it, but we are not on Java8 yet, and I don't think we want to block > on that :) > > On Wed, Mar 30, 2016 at 10:53 AM, Ismae

Re: KIP-4 Wiki Update

2016-03-30 Thread Gwen Shapira
I like it, but we are not on Java8 yet, and I don't think we want to block on that :) On Wed, Mar 30, 2016 at 10:53 AM, Ismael Juma wrote: > On Wed, Mar 30, 2016 at 6:21 PM, Gwen Shapira wrote: > > > Ismael, can you detail how the Optional approach would work in the wire > > protocol? It sounds

Re: KIP-4 Wiki Update

2016-03-30 Thread Ismael Juma
On Wed, Mar 30, 2016 at 6:21 PM, Gwen Shapira wrote: > Ismael, can you detail how the Optional approach would work in the wire > protocol? It sounds good, but I'm unclear on what this would look like on > the wire. > In the wire protocol, we would still use a size of -1 like we normally do for n

Re: KIP-4 Wiki Update

2016-03-30 Thread Gwen Shapira
Process comment: Since KIP-4 is voted and signed. Perhaps a small KIP detailing the suggested Metadata API changes so it will be easy to refer to what we are discussing? Gwen On Wed, Mar 30, 2016 at 10:21 AM, Gwen Shapira wrote: > I guess Scala corrupted my brain because I can't see why ()-> al

Re: KIP-4 Wiki Update

2016-03-30 Thread Gwen Shapira
I guess Scala corrupted my brain because I can't see why ()-> all and ([])-> none is intuitive :) I would much prefer: empty-> no topics, null -> exception and boolean for "all". Ismael, can you detail how the Optional approach would work in the wire protocol? It sounds good, but I'm unclear on w

Re: KIP-4 Wiki Update

2016-03-30 Thread Ismael Juma
My concern is that this approach is error-prone and a bit magical. It is possible to do that in the wire protocol while using a safer approach in the code if people prefer this option (by using a type like `Optional` in Java). Ismael On Wed, Mar 30, 2016 at 5:33 PM, Jason Gustafson wrote: > Yea

Re: KIP-4 Wiki Update

2016-03-30 Thread Jason Gustafson
Yeah, that actually is intuitive when viewed that way. -Jason On Wed, Mar 30, 2016 at 9:25 AM, Dana Powers wrote: > Perhaps python has corrupted my brain, but a null arg seems quite clean to > me: > > getTopics() -> returns all > getTopics([]) -> returns none > getTopics([foo, bar]) -> returns

Re: KIP-4 Wiki Update

2016-03-30 Thread Dana Powers
Perhaps python has corrupted my brain, but a null arg seems quite clean to me: getTopics() -> returns all getTopics([]) -> returns none getTopics([foo, bar]) -> returns foo and bar only -Dana On Wed, Mar 30, 2016 at 9:10 AM, Jason Gustafson wrote: > > > > Yes, I think empty should be "no topic

Re: KIP-4 Wiki Update

2016-03-30 Thread Jason Gustafson
> > Yes, I think empty should be "no topics" too. However, I would suggest > using a boolean to indicate "all topics" and null should not be allowed (as > it is now). I think this is a clearer API and it's similar to > how org.apache.kafka.clients.Metadata works today. +1. Having null imply all i

Re: KIP-4 Wiki Update

2016-03-30 Thread Ismael Juma
I haven't had a chance to look at the KIP in detail, but a quick comment below. On Wed, Mar 30, 2016 at 4:49 PM, Dana Powers wrote: > > MetadataRequest v1: long-term / conceptually, I think a "null" topic list > aligns better with fetching all topics. Empty list aligns better with > fetching no t

Re: KIP-4 Wiki Update

2016-03-30 Thread Dana Powers
Grant - sorry I was unable to attend. Getting API access to admin functionality has been a big ask for python client users. I like this KIP a lot. I reviewed the details quickly. Here are some comments: MetadataRequest v1: long-term / conceptually, I think a "null" topic list aligns better with f

Re: KIP-4 Wiki Update

2016-03-30 Thread Grant Henke
I didn't get anyone in attendance for this meeting. If you would like to discuss it please let me know. Thank you, Grant On Mon, Mar 28, 2016 at 9:18 AM, Grant Henke wrote: > I am hoping to get more discussion and feedback around the blocking vs > async discussion so I can start to get KIP-4 pa

Re: KIP-4 Wiki Update

2016-03-28 Thread Grant Henke
I am hoping to get more discussion and feedback around the blocking vs async discussion so I can start to get KIP-4 patches reviewed. In order to facilitate a faster discussion I will hold an open discussion on Tuesday March 29th at 12pm PST (right after the usual KIP call, if we have one). Please

Re: KIP-4 Wiki Update

2016-03-15 Thread Grant Henke
Moving the relevant wiki text here for discussion/tracking: > > Server-side Admin Request handlers > > At the highest level, admin requests will be handled on the brokers the > same way that all message types are. However, because admin messages modify > cluster metadata they should be handled by t

KIP-4 Wiki Update

2016-03-14 Thread Grant Henke
I have been updating the KIP-4 wiki page based on the last KIP call and wanted to get some review and discussion around the server side implementation for admin requests. Both the "ideal" functionality and the "intermediated" functionality. The updates are still in progress, but this section is the